[bfcache] Implement Memory Controls for BFCache

- Add kBackForwardCacheMemoryControl feature which can be controlled via
  experiment.
- Introduce "memory_threshold_for_back_forward_cache_in_mb" to control the
  factor via experiment.

Bug: 1017090
Change-Id: I44d95da16f4023ea5b8ad890dd539784c74006c6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1875091Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Commit-Queue: Sreeja Kamishetty <sreejakshetty@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718605}

content/browser/back_forward_cache_browsertest.cc

@@ -3,8 +3,10 @@
 // found in the LICENSE file.
 
 #include "base/command_line.h"
+#include "base/hash/hash.h"
 #include "base/metrics/metrics_hashes.h"
 #include "base/optional.h"
+#include "base/system/sys_info.h"
 #include "base/task/post_task.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/scoped_feature_list.h"
@@ -47,6 +49,7 @@
 #include "services/service_manager/public/cpp/service_binding.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "third_party/blink/public/common/scheduler/web_scheduler_tracked_feature.h"
+#include "unordered_map"
 
 using testing::Each;
 using testing::ElementsAre;
@@ -58,6 +61,20 @@ namespace {
 
 const char* kDisabledReasonForTest = "DisabledByBackForwardCacheBrowserTest";
 
+// hash for std::unordered_map.
+struct FeatureHash {
+  size_t operator()(base::Feature feature) const {
+    return base::FastHash(feature.name);
+  }
+};
+
+// compare operator for std::unordered_map.
+struct FeatureEqualOperator {
+  bool operator()(base::Feature feature1, base::Feature feature2) const {
+    return std::strcmp(feature1.name, feature2.name) == 0;
+  }
+};
+
 // Test about the BackForwardCache.
 class BackForwardCacheBrowserTest : public ContentBrowserTest {
  public:
@@ -71,18 +88,32 @@ class BackForwardCacheBrowserTest : public ContentBrowserTest {
         switches::kIgnoreCertificateErrors);
     base::CommandLine::ForCurrentProcess()->AppendSwitch(
         switches::kEnableExperimentalWebPlatformFeatures);
-    feature_list_.InitWithFeaturesAndParameters(
-        {{features::kBackForwardCache, {GetFeatureParams()}},
-         {features::kServiceWorkerOnUI, {}}},
-        {});
+    // TODO(sreejakshetty): Initialize ScopedFeatureLists from test constructor.
+    EnableFeatureAndSetParams(features::kBackForwardCache,
+                              "TimeToLiveInBackForwardCacheInSeconds", "3600");
+    EnableFeatureAndSetParams(features::kServiceWorkerOnUI, "", "");
+    SetupFeaturesAndParameters();
 
     ContentBrowserTest::SetUpCommandLine(command_line);
   }
 
-  virtual base::FieldTrialParams GetFeatureParams() {
-    // Set a very long TTL before expiration (longer than the test timeout) so
-    // tests that are expecting deletion don't pass when they shouldn't.
-    return {{"TimeToLiveInBackForwardCacheInSeconds", "3600"}};
+  void SetupFeaturesAndParameters() {
+    std::vector<base::test::ScopedFeatureList::FeatureAndParams>
+        enabled_features;
+
+    for (auto feature_param = features_with_params_.begin();
+         feature_param != features_with_params_.end(); feature_param++) {
+      enabled_features.push_back({feature_param->first, feature_param->second});
+    }
+
+    feature_list_.InitWithFeaturesAndParameters(enabled_features,
+                                                /*disabled_features*/ {});
+  }
+
+  void EnableFeatureAndSetParams(base::Feature feature,
+                                 std::string param_name,
+                                 std::string param_value) {
+    features_with_params_[feature][param_name] = param_value;
   }
 
   void SetUpOnMainThread() override {
@@ -256,8 +287,12 @@ class BackForwardCacheBrowserTest : public ContentBrowserTest {
   std::vector<base::Bucket> expected_blocklisted_features_;
   std::vector<base::Bucket> expected_disabled_reasons_;
   std::vector<base::Bucket> expected_eviction_after_committing_;
-
   std::unique_ptr<net::EmbeddedTestServer> https_server_;
+  std::unordered_map<base::Feature,
+                     std::map<std::string, std::string>,
+                     FeatureHash,
+                     FeatureEqualOperator>
+      features_with_params_;
 };
 
 // Match RenderFrameHostImpl* that are in the BackForwardCache.
@@ -2593,8 +2628,11 @@ class BackForwardCacheBrowserTestWithServiceWorkerEnabled
   ~BackForwardCacheBrowserTestWithServiceWorkerEnabled() override {}
 
  protected:
-  base::FieldTrialParams GetFeatureParams() override {
-    return {{"service_worker_supported", "true"}};
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    EnableFeatureAndSetParams(features::kBackForwardCache,
+                              "service_worker_supported", "true");
+
+    BackForwardCacheBrowserTest::SetUpCommandLine(command_line);
   }
 };
 
@@ -2915,8 +2953,11 @@ class GeolocationBackForwardCacheBrowserTest
  protected:
   GeolocationBackForwardCacheBrowserTest() : geo_override_(0.0, 0.0) {}
 
-  base::FieldTrialParams GetFeatureParams() override {
-    return {{"geolocation_supported", "true"}};
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    EnableFeatureAndSetParams(features::kBackForwardCache,
+                              "geolocation_supported", "true");
+
+    BackForwardCacheBrowserTest::SetUpCommandLine(command_line);
   }
 
   device::ScopedGeolocationOverrider geo_override_;
@@ -3058,7 +3099,7 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, TimedEviction) {
 
   base::TimeDelta time_to_live_in_back_forward_cache =
       BackForwardCacheImpl::GetTimeToLiveInBackForwardCache();
-  // This should match the value we set in GetFeatureParams.
+  // This should match the value we set in EnableFeatureAndSetParams.
   EXPECT_EQ(time_to_live_in_back_forward_cache,
             base::TimeDelta::FromSeconds(3600));
 
@@ -3286,18 +3327,16 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, MetricsNotRecorded) {
 class BackForwardCacheBrowserTestWithDomainControlEnabled
     : public BackForwardCacheBrowserTest {
  protected:
-  base::FieldTrialParams GetFeatureParams() override {
+  void SetUpCommandLine(base::CommandLine* command_line) override {
     // Sets the allowed websites for testing, additionally adding the params
     // used by BackForwardCacheBrowserTest.
-    std::map<std::string, std::string> domain_control_params = {
-        {"allowed_websites",
-         "https://a.allowed/back_forward_cache/, "
-         "https://b.allowed/back_forward_cache/allowed_path.html"}};
-    std::map<std::string, std::string> browser_test_params =
-        BackForwardCacheBrowserTest::GetFeatureParams();
-    domain_control_params.insert(browser_test_params.begin(),
-                                 browser_test_params.end());
-    return domain_control_params;
+    std::string allowed_websites =
+        "https://a.allowed/back_forward_cache/, "
+        "https://b.allowed/back_forward_cache/allowed_path.html";
+    EnableFeatureAndSetParams(features::kBackForwardCache, "allowed_websites",
+                              allowed_websites);
+
+    BackForwardCacheBrowserTest::SetUpCommandLine(command_line);
   }
 };
 
@@ -3986,4 +4025,83 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
                 FROM_HERE);
 }
 
+// Test for functionality of memory controls in back-forward cache for low
+// memory devices.
+class BackForwardCacheBrowserTestForLowMemoryDevices
+    : public BackForwardCacheBrowserTest {
+ protected:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    // Set the value of memory threshold more than the physical memory and check
+    // if back-forward cache is disabled or not.
+    std::string memory_threshold =
+        base::NumberToString(base::SysInfo::AmountOfPhysicalMemoryMB() + 1);
+    EnableFeatureAndSetParams(features::kBackForwardCacheMemoryControl,
+                              "memory_threshold_for_back_forward_cache_in_mb",
+                              memory_threshold);
+
+    BackForwardCacheBrowserTest::SetUpCommandLine(command_line);
+  }
+};
+
+// Navigate from A to B and go back.
+IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTestForLowMemoryDevices,
+                       DisableBFCacheForLowEndDevices) {
+  EXPECT_FALSE(IsBackForwardCacheEnabled());
+  ASSERT_TRUE(embedded_test_server()->Start());
+  const GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html"));
+  const GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html"));
+
+  // 1) Navigate to A.
+  EXPECT_TRUE(NavigateToURL(shell(), url_a));
+  RenderFrameHostImpl* rfh_a = current_frame_host();
+  RenderFrameDeletedObserver delete_observer_rfh_a(rfh_a);
+
+  // 2) Navigate to B.
+  EXPECT_TRUE(NavigateToURL(shell(), url_b));
+
+  // 3) A shouldn't be stored in back-forward cache because the physical
+  // memory is less than the memory threshold.
+  delete_observer_rfh_a.WaitUntilDeleted();
+
+  // Nothing is recorded when the memory is less than the threshold value.
+  ExpectOutcomeDidNotChange(FROM_HERE);
+  ExpectNotRestoredDidNotChange(FROM_HERE);
+}
+
+// Test for functionality of memory controls in back-forward cache for high
+// memory devices.
+class BackForwardCacheBrowserTestForHighMemoryDevices
+    : public BackForwardCacheBrowserTest {
+ protected:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    // Set the value of memory threshold less than the physical memory and check
+    // if back-forward cache is enabled or not.
+    std::string memory_threshold =
+        base::NumberToString(base::SysInfo::AmountOfPhysicalMemoryMB() - 1);
+    EnableFeatureAndSetParams(features::kBackForwardCacheMemoryControl,
+                              "memory_threshold_for_back_forward_cache_in_mb",
+                              memory_threshold);
+
+    BackForwardCacheBrowserTest::SetUpCommandLine(command_line);
+  }
+};
+
+// Navigate from A to B and go back.
+IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTestForHighMemoryDevices,
+                       EnableBFCacheForHighMemoryDevices) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  const GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html"));
+  const GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html"));
+
+  // 1) Navigate to A.
+  EXPECT_TRUE(NavigateToURL(shell(), url_a));
+  RenderFrameHostImpl* rfh_a = current_frame_host();
+
+  // 2) Navigate to B.
+  EXPECT_TRUE(NavigateToURL(shell(), url_b));
+
+  // 3) A should be stored in back-forward cache because the physical memory is
+  // greater than the memory threshold.
+  EXPECT_TRUE(rfh_a->is_in_back_forward_cache());
+}
 }  // namespace content

content/public/common/content_features.cc

@@ -63,6 +63,16 @@ const base::Feature kBackgroundFetch{"BackgroundFetch",
 const base::Feature kBackForwardCache{"BackForwardCache",
                                       base::FEATURE_DISABLED_BY_DEFAULT};
 
+// BackForwardCache is disabled on low memory devices. The threshold is defined
+// via a field trial param: "memory_threshold_for_back_forward_cache_in_mb"
+// It is compared against base::SysInfo::AmountOfPhysicalMemoryMB().
+
+// "BackForwardCacheMemoryControls" is checked before "BackForwardCache". It
+// means the low memory devices will activate neither the control group nor the
+// experimental group of the BackForwardCache field trial.
+const base::Feature kBackForwardCacheMemoryControl{
+    "BackForwardCacheMemoryControls", base::FEATURE_DISABLED_BY_DEFAULT};
+
 // Allows swipe left/right from touchpad change browser navigation. Currently
 // only enabled by default on CrOS.
 const base::Feature kTouchpadOverscrollHistoryNavigation {

content/public/common/content_features.h

@@ -26,6 +26,7 @@ CONTENT_EXPORT extern const base::Feature kAudioServiceLaunchOnStartup;
 CONTENT_EXPORT extern const base::Feature kAudioServiceOutOfProcess;
 CONTENT_EXPORT extern const base::Feature kBackgroundFetch;
 CONTENT_EXPORT extern const base::Feature kBackForwardCache;
+CONTENT_EXPORT extern const base::Feature kBackForwardCacheMemoryControl;
 CONTENT_EXPORT extern const base::Feature kBlockCredentialedSubresources;
 CONTENT_EXPORT extern const base::Feature kBrowserVerifiedUserActivation;
 CONTENT_EXPORT extern const base::Feature kCacheInlineScriptCode;

content/public/common/navigation_policy.cc

@@ -6,6 +6,7 @@
 
 #include "base/command_line.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/system/sys_info.h"
 #include "content/public/common/content_features.h"
 #include "content/public/common/content_switches.h"
 #include "services/network/public/cpp/features.h"
@@ -24,9 +25,32 @@ void LogArbitraryPolicyPerDownload(NavigationDownloadType type) {
       "Navigation.DownloadPolicy.LogArbitraryPolicyPerDownload", type);
 }
 
+bool DeviceHasEnoughMemoryForBackForwardCache() {
+  // This method make sure that the physical memory of device is greater than
+  // the allowed threshold and enables back-forward cache if the feature
+  // kBackForwardCacheMemoryControl is enabled.
+  // It is important to check the base::FeatureList to avoid activating any
+  // field trial groups if BFCache is disabled due to memory threshold.
+  if (base::FeatureList::IsEnabled(features::kBackForwardCacheMemoryControl)) {
+    int memory_threshold_mb = base::GetFieldTrialParamByFeatureAsInt(
+        features::kBackForwardCacheMemoryControl,
+        "memory_threshold_for_back_forward_cache_in_mb", 0);
+    return base::SysInfo::AmountOfPhysicalMemoryMB() > memory_threshold_mb;
+  }
+
+  // If the feature kBackForwardCacheMemoryControl is not enabled, all the
+  // devices are included by default.
+  return true;
+}
+
 }  // namespace
 
 bool IsBackForwardCacheEnabled() {
+  if (!DeviceHasEnoughMemoryForBackForwardCache())
+    return false;
+  // The feature needs to be checked last, because checking the feature
+  // activates the field trial and assigns the client either to a control or an
+  // experiment group - such assignment should be final.
   return base::FeatureList::IsEnabled(features::kBackForwardCache);
 }