Cleanup InfiniteSessionRestore feature and SessionRestorePolicy.

BUG=749785

Change-Id: Id9e7bf25b421199bdb0f6b0dc41d617792a64bbb
Reviewed-on: https://chromium-review.googlesource.com/1093019Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org>
Commit-Queue: Chris Hamilton <chrisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565660}

chrome/browser/resource_coordinator/session_restore_policy.cc

@@ -63,7 +63,8 @@ SessionRestorePolicy::SessionRestorePolicy()
     : policy_enabled_(
           base::FeatureList::IsEnabled(features::kInfiniteSessionRestore)),
       delegate_(SysInfoDelegate::Get()),
-      params_(&GetStaticInfiniteSessionRestoreParams()),
+      parsed_params_(GetInfiniteSessionRestoreParams()),
+      params_(&parsed_params_),
       simultaneous_tab_loads_(CalculateSimultaneousTabLoadsFromParams()) {}
 
 SessionRestorePolicy::~SessionRestorePolicy() = default;
@@ -110,10 +111,10 @@ void SessionRestorePolicy::NotifyTabLoadStarted() {
 }
 
 SessionRestorePolicy::SessionRestorePolicy(
+    bool policy_enabled,
     const Delegate* delegate,
     const InfiniteSessionRestoreParams* params)
-    : policy_enabled_(
-          base::FeatureList::IsEnabled(features::kInfiniteSessionRestore)),
+    : policy_enabled_(policy_enabled),
       delegate_(delegate),
       params_(params),
       simultaneous_tab_loads_(CalculateSimultaneousTabLoadsFromParams()) {}

chrome/browser/resource_coordinator/session_restore_policy.h

@@ -45,7 +45,8 @@ class SessionRestorePolicy {
   // Protected so can be exposed for unittesting.
 
   // Full constructor for testing.
-  SessionRestorePolicy(const Delegate* delegate,
+  SessionRestorePolicy(bool policy_enabled,
+                       const Delegate* delegate,
                        const InfiniteSessionRestoreParams* params);
 
   // Helper function for computing the number of loading slots to use. All
@@ -69,9 +70,13 @@ class SessionRestorePolicy {
   // the logic in this class.
   const Delegate* const delegate_;
 
-  // The parameters being used by this policy engine. These default to the
-  // static parameters from tab_manager_features.h, but can be injected for
-  // testing.
+  // A container for storing parsed parameters. Unless parameters are injected
+  // externally this will be populated with parsed parameters.
+  const InfiniteSessionRestoreParams parsed_params_;
+
+  // The parameters being used by this policy engine. By default this is simply
+  // a pointer to |parsed_params_|, but it can also point to externally
+  // injected parameters for testing.
   const InfiniteSessionRestoreParams* const params_;
 
   // The number of simultaneous tab loads that are permitted by policy. This

chrome/browser/resource_coordinator/session_restore_policy_unittest.cc

@@ -6,7 +6,6 @@
 
 #include "base/test/simple_test_tick_clock.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
-#include "components/variations/variations_params_manager.h"
 #include "content/public/browser/web_contents.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -56,9 +55,10 @@ class TestSessionRestorePolicy : public SessionRestorePolicy {
  public:
   using SessionRestorePolicy::CalculateSimultaneousTabLoads;
   using SessionRestorePolicy::SetTabLoadsStartedForTesting;
-  TestSessionRestorePolicy(const Delegate* delegate,
+  TestSessionRestorePolicy(bool policy_enabled,
+                           const Delegate* delegate,
                            const InfiniteSessionRestoreParams* params)
-      : SessionRestorePolicy(delegate, params) {}
+      : SessionRestorePolicy(policy_enabled, delegate, params) {}
 
   ~TestSessionRestorePolicy() override {}
 
@@ -117,8 +117,9 @@ class SessionRestorePolicyTest : public ChromeRenderViewHostTestHarness {
     ChromeRenderViewHostTestHarness::TearDown();
   }
 
-  void CreatePolicy() {
-    policy_ = std::make_unique<TestSessionRestorePolicy>(&delegate_, &params_);
+  void CreatePolicy(bool policy_enabled) {
+    policy_ = std::make_unique<TestSessionRestorePolicy>(policy_enabled,
+                                                         &delegate_, &params_);
   }
 
  protected:
@@ -170,16 +171,7 @@ TEST_F(SessionRestorePolicyTest, CalculateSimultaneousTabLoads) {
 }
 
 TEST_F(SessionRestorePolicyTest, ShouldLoadFeatureEnabled) {
-  // Enable the InfiniteSessionRestore feature so that the policy logic is
-  // actually enabled.
-  std::set<std::string> features;
-  std::map<std::string, std::string> variations_params;
-  variations::testing::VariationParamsManager variations_manager;
-  features.insert(features::kInfiniteSessionRestore.name);
-  variations_manager.SetVariationParamsWithFeatureAssociations(
-      "DummyTrial", variations_params, features);
-
-  CreatePolicy();
+  CreatePolicy(true);
   EXPECT_TRUE(policy_->policy_enabled());
   EXPECT_EQ(2u, policy_->simultaneous_tab_loads());
 
@@ -247,7 +239,7 @@ TEST_F(SessionRestorePolicyTest, ShouldLoadFeatureEnabled) {
 }
 
 TEST_F(SessionRestorePolicyTest, ShouldLoadFeatureDisabled) {
-  CreatePolicy();
+  CreatePolicy(false);
   EXPECT_FALSE(policy_->policy_enabled());
   EXPECT_EQ(std::numeric_limits<size_t>::max(),
             policy_->simultaneous_tab_loads());

chrome/browser/resource_coordinator/tab_manager_features.cc

@@ -357,10 +357,4 @@ InfiniteSessionRestoreParams GetInfiniteSessionRestoreParams() {
   return params;
 }
 
-const InfiniteSessionRestoreParams& GetStaticInfiniteSessionRestoreParams() {
-  static base::NoDestructor<InfiniteSessionRestoreParams> params(
-      GetInfiniteSessionRestoreParams());
-  return *params;
-}
-
 }  // namespace resource_coordinator

chrome/browser/resource_coordinator/tab_manager_features.h

@@ -254,10 +254,6 @@ GetStaticSiteCharacteristicsDatabaseParams();
 // Gets parameters for the infinite session restore feature.
 InfiniteSessionRestoreParams GetInfiniteSessionRestoreParams();
 
-// Returns a static InfiniteSessionRestoreParams object that can be used by all
-// instances of TabLoader.
-const InfiniteSessionRestoreParams& GetStaticInfiniteSessionRestoreParams();
-
 }  // namespace resource_coordinator
 
 #endif  // CHROME_BROWSER_RESOURCE_COORDINATOR_TAB_MANAGER_FEATURES_H_

chrome/browser/resource_coordinator/tab_manager_features_unittest.cc

@@ -110,6 +110,8 @@ class TabManagerFeaturesTest : public testing::Test {
     EXPECT_EQ(max_tabs_to_restore, params.max_tabs_to_restore);
     EXPECT_EQ(mb_free_memory_per_tab_to_restore,
               params.mb_free_memory_per_tab_to_restore);
+    EXPECT_EQ(max_time_since_last_use_to_restore,
+              params.max_time_since_last_use_to_restore);
     EXPECT_EQ(min_site_engagement_to_restore,
               params.min_site_engagement_to_restore);
   }