Enable Infinite Session Restore by default

Bug: 863441
Change-Id: Icd11498ed8f396c5506ae995da34d375af8f7655
Reviewed-on: https://chromium-review.googlesource.com/c/1355989
Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613281}

chrome/browser/BUILD.gn

@@ -5255,6 +5255,8 @@ static_library("test_support") {
       "profile_resetter/profile_resetter_test_base.h",
       "sessions/session_restore_test_helper.cc",
       "sessions/session_restore_test_helper.h",
+      "sessions/session_restore_test_utils.cc",
+      "sessions/session_restore_test_utils.h",
       "sessions/session_service_test_helper.cc",
       "sessions/session_service_test_helper.h",
       "sessions/tab_loader_tester.cc",

chrome/browser/resource_coordinator/tab_manager_features.cc

@@ -26,7 +26,7 @@ const base::Feature kCustomizedTabLoadTimeout{
 // Enables TabLoader improvements for reducing the overhead of session restores
 // involving many many tabs.
 const base::Feature kInfiniteSessionRestore{"InfiniteSessionRestore",
-                                            base::FEATURE_DISABLED_BY_DEFAULT};
+                                            base::FEATURE_ENABLED_BY_DEFAULT};
 
 // Enables proactive tab freezing and discarding.
 const base::Feature kProactiveTabFreezeAndDiscard{

chrome/browser/resource_coordinator/tab_manager_features.h

@@ -259,10 +259,11 @@ struct InfiniteSessionRestoreParams {
   // This is the 75th percentile of Memory.Renderer.PrivateMemoryFootprint.
   static constexpr base::FeatureParam<int> kMbFreeMemoryPerTabToRestore{
       &features::kInfiniteSessionRestore, "MbFreeMemoryPerTabToRestore", 150};
-  // This is the 75th percentile of SessionRestore.RestoredTab.TimeSinceActive.
+  // This value has been determined by a Finch experiment, it reduces user pain
+  // without impacting the gains from this feature.
   static constexpr base::FeatureParam<int> kMaxTimeSinceLastUseToRestore{
       &features::kInfiniteSessionRestore, "MaxTimeSinceLastUseToRestore",
-      6 * base::Time::kSecondsPerHour};
+      30 * base::Time::kHoursPerDay* base::Time::kSecondsPerHour};
   // Taken from an informal survey of Googlers on min engagement of things they
   // think *must* load. Note that about 25% of session-restore tabs fall above
   // this threshold (see SessionRestore.RestoredTab.SiteEngagementScore).

chrome/browser/sessions/session_restore_browsertest.cc

@@ -27,6 +27,7 @@
 #include "chrome/browser/resource_coordinator/session_restore_policy.h"
 #include "chrome/browser/resource_coordinator/tab_manager_features.h"
 #include "chrome/browser/sessions/session_restore_test_helper.h"
+#include "chrome/browser/sessions/session_restore_test_utils.h"
 #include "chrome/browser/sessions/session_service.h"
 #include "chrome/browser/sessions/session_service_factory.h"
 #include "chrome/browser/sessions/session_service_test_helper.h"
@@ -213,17 +214,6 @@ class SessionRestoreTest : public InProcessBrowserTest {
   base::test::FakeMemoryPressureMonitor fake_memory_pressure_monitor_;
 };
 
-// SessionRestorePolicy that always allow tabs to load.
-class TestSessionRestorePolicy
-    : public resource_coordinator::SessionRestorePolicy {
- public:
-  // Always allow tabs to load so we can test the behavior of SessionRestore
-  // independently from the policy logic.
-  bool ShouldLoad(content::WebContents* contents) const override {
-    return true;
-  }
-};
-
 // Activates the smart restore behaviour and tracks the loading of tabs.
 class SmartSessionRestoreTest : public SessionRestoreTest,
                                       public content::NotificationObserver {
@@ -231,12 +221,10 @@ class SmartSessionRestoreTest : public SessionRestoreTest,
   SmartSessionRestoreTest() {}
 
   void SetUp() override {
-    TabLoaderDelegate::SetSessionRestorePolicyForTesting(&test_policy_);
     SessionRestoreTest::SetUp();
   }
 
   void TearDown() override {
-    TabLoaderDelegate::SetSessionRestorePolicyForTesting(nullptr);
     SessionRestoreTest::TearDown();
   }
 
@@ -281,7 +269,7 @@ class SmartSessionRestoreTest : public SessionRestoreTest,
   std::vector<content::WebContents*> web_contents_;
   scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
   size_t num_tabs_;
-  TestSessionRestorePolicy test_policy_;
+  testing::ScopedAlwaysLoadSessionRestoreTestPolicy test_policy_;
 
   DISALLOW_COPY_AND_ASSIGN(SmartSessionRestoreTest);
 };

chrome/browser/sessions/session_restore_test_utils.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/sessions/session_restore_test_utils.h"
+
+#include "chrome/browser/sessions/tab_loader_delegate.h"
+
+namespace testing {
+
+bool AlwayLoadSessionRestorePolicy::ShouldLoad(
+    content::WebContents* contents) const {
+  return true;
+}
+
+ScopedAlwaysLoadSessionRestoreTestPolicy::
+    ScopedAlwaysLoadSessionRestoreTestPolicy() {
+  TabLoaderDelegate::SetSessionRestorePolicyForTesting(&policy_);
+}
+
+ScopedAlwaysLoadSessionRestoreTestPolicy::
+    ~ScopedAlwaysLoadSessionRestoreTestPolicy() {
+  TabLoaderDelegate::SetSessionRestorePolicyForTesting(nullptr);
+}
+
+}  // namespace testing

chrome/browser/sessions/session_restore_test_utils.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_SESSIONS_SESSION_RESTORE_TEST_UTILS_H_
+#define CHROME_BROWSER_SESSIONS_SESSION_RESTORE_TEST_UTILS_H_
+
+#include <memory>
+
+#include "base/macros.h"
+#include "chrome/browser/resource_coordinator/session_restore_policy.h"
+
+namespace testing {
+
+// SessionRestorePolicy that always allow tabs to load.
+class AlwayLoadSessionRestorePolicy
+    : public resource_coordinator::SessionRestorePolicy {
+ public:
+  AlwayLoadSessionRestorePolicy() = default;
+  ~AlwayLoadSessionRestorePolicy() override = default;
+
+  // Always allow tabs to load so we can test the behavior of SessionRestore
+  // independently from the policy logic.
+  bool ShouldLoad(content::WebContents* contents) const override;
+};
+
+class ScopedAlwaysLoadSessionRestoreTestPolicy {
+ public:
+  ScopedAlwaysLoadSessionRestoreTestPolicy();
+  ~ScopedAlwaysLoadSessionRestoreTestPolicy();
+
+ private:
+  AlwayLoadSessionRestorePolicy policy_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScopedAlwaysLoadSessionRestoreTestPolicy);
+};
+
+}  // namespace testing
+
+#endif  // CHROME_BROWSER_SESSIONS_SESSION_RESTORE_TEST_UTILS_H_

chrome/browser/sessions/tab_loader_unittest.cc

@@ -12,6 +12,7 @@
 #include "base/time/time.h"
 #include "chrome/browser/resource_coordinator/tab_helper.h"
 #include "chrome/browser/resource_coordinator/tab_manager_features.h"
+#include "chrome/browser/sessions/session_restore_test_utils.h"
 #include "chrome/browser/sessions/tab_loader_tester.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/variations/variations_params_manager.h"
@@ -45,6 +46,7 @@ class TabLoaderTest : public testing::Test {
         &TabLoaderTest::OnTabLoaderCreated, base::Unretained(this));
     TabLoaderTester::SetConstructionCallbackForTesting(&construction_callback_);
     test_web_contents_factory_.reset(new content::TestWebContentsFactory);
+    test_policy_.reset(new testing::ScopedAlwaysLoadSessionRestoreTestPolicy());
   }
 
   void TearDown() override {
@@ -57,6 +59,7 @@ class TabLoaderTest : public testing::Test {
     TabLoaderTester::SetConstructionCallbackForTesting(nullptr);
     test_web_contents_factory_.reset();
     thread_bundle_.RunUntilIdle();
+    test_policy_.reset();
   }
 
   void SimulateLoadTimeout() {
@@ -140,6 +143,8 @@ class TabLoaderTest : public testing::Test {
   std::unique_ptr<content::TestWebContentsFactory> test_web_contents_factory_;
   content::TestBrowserThreadBundle thread_bundle_;
   TestingProfile testing_profile_;
+  std::unique_ptr<testing::ScopedAlwaysLoadSessionRestoreTestPolicy>
+      test_policy_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(TabLoaderTest);
@@ -325,6 +330,8 @@ TEST_F(TabLoaderTest, TimeoutCanExceedLoadingSlots) {
 TEST_F(TabLoaderTest, DelegatePolicyIsApplied) {
   namespace rc = resource_coordinator;
 
+  test_policy_.reset();
+
   std::set<std::string> features;
   features.insert(features::kInfiniteSessionRestore.name);
 

testing/variations/fieldtrial_testing_config.json

@@ -2407,24 +2407,6 @@
             ]
         }
     ],
-    "InfiniteSessionRestore": [
-        {
-            "platforms": [
-                "chromeos",
-                "linux",
-                "mac",
-                "windows"
-            ],
-            "experiments": [
-                {
-                    "name": "Enabled",
-                    "enable_features": [
-                        "InfiniteSessionRestore"
-                    ]
-                }
-            ]
-        }
-    ],
     "InstanceID": [
         {
             "platforms": [