Quota: Enable StaticHostQuota by default.

Change the default behavior to enabled, update unit tests to match new
default behavior, and remove fieldtrial testing config.

Bug: 960305
Change-Id: I8f045648c73ab8787dddc2a75d576f23c13dac1d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2090451
Commit-Queue: Jarryd Goodman <jarrydg@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#747480}

storage/browser/quota/quota_features.cc

@@ -22,7 +22,7 @@ constexpr base::FeatureParam<double> kPerHostRatio{&kQuotaExpandPoolSize,
 // quota computation takes into account free disk space, in addition to the
 // disk's total capacity.
 const base::Feature kStaticHostQuota{"StaticHostQuota",
-                                     base::FEATURE_DISABLED_BY_DEFAULT};
+                                     base::FEATURE_ENABLED_BY_DEFAULT};
 
 // QuotaUnlimitedPoolSize removes limitations around disk space consumption with
 // respect to client-side storage web platform APIs. When enabled, quota will

storage/browser/quota/quota_manager_unittest.cc

@@ -1219,13 +1219,10 @@ TEST_F(QuotaManagerTest, GetQuotaLowAvailableDiskSpace) {
   const int kPoolSize = 10000000;
   const int kPerHostQuota = kPoolSize / 5;
 
-  // Simulating a low available disk space scenario by making
-  // kMustRemainAvailable 64KB less than GetAvailableDiskSpaceForTest(), which
-  // means there is 64KB of storage quota that can be used before triggering
-  // the low available space logic branch in quota_manager.cc. From the
-  // perspective of QuotaManager, there are 64KB of free space in the temporary
-  // pool, so it should return (64KB + usage) as quota since the sum is less
-  // than the default host quota.
+  // In here, we expect the low available space logic branch
+  // to be ignored. Doing so should have QuotaManager return the same per-host
+  // quota as what is set in QuotaSettings, despite being in a state of low
+  // available space.
   const int kMustRemainAvailable =
       static_cast<int>(GetAvailableDiskSpaceForTest() - 65536);
   SetQuotaSettings(kPoolSize, kPerHostQuota, kMustRemainAvailable);
@@ -1234,18 +1231,21 @@ TEST_F(QuotaManagerTest, GetQuotaLowAvailableDiskSpace) {
   task_environment_.RunUntilIdle();
   EXPECT_EQ(QuotaStatusCode::kOk, status());
   EXPECT_EQ(100000, usage());
-  EXPECT_GT(kPerHostQuota, quota());
-  EXPECT_EQ(65536 + usage(), quota());
+  EXPECT_EQ(kPerHostQuota, quota());
 }
 
-TEST_F(QuotaManagerTest, GetStaticQuotaLowAvailableDiskSpace) {
+TEST_F(QuotaManagerTest,
+       GetQuotaLowAvailableDiskSpace_StaticHostQuotaDisabled) {
   // This test is the same as the previous but with the kStaticHostQuota Finch
-  // feature enabled. In here, we expect the low available space logic branch
-  // to be ignored. Doing so should have QuotaManager return the same per host
-  // quota as what is set in QuotaSettings, despite being in a state of low
-  // available space. Notice the different expectation in the last line of
-  // each test.
-  scoped_feature_list_.InitAndEnableFeature(features::kStaticHostQuota);
+  // feature disabled.
+  // Simulating a low available disk space scenario by making
+  // kMustRemainAvailable 64KB less than GetAvailableDiskSpaceForTest(), which
+  // means there is 64KB of storage quota that can be used before triggering
+  // the low available space logic branch in quota_manager.cc. From the
+  // perspective of QuotaManager, there are 64KB of free space in the temporary
+  // pool, so it should return (64KB + usage) as quota since the sum is less
+  // than the default host quota.
+  scoped_feature_list_.InitAndDisableFeature(features::kStaticHostQuota);
   static const MockOriginData kData[] = {
       {"http://foo.com/", kTemp, 100000},
       {"http://unlimited/", kTemp, 4000000},
@@ -1265,7 +1265,8 @@ TEST_F(QuotaManagerTest, GetStaticQuotaLowAvailableDiskSpace) {
   task_environment_.RunUntilIdle();
   EXPECT_EQ(QuotaStatusCode::kOk, status());
   EXPECT_EQ(100000, usage());
-  EXPECT_EQ(kPerHostQuota, quota());
+  EXPECT_GT(kPerHostQuota, quota());
+  EXPECT_EQ(65536 + usage(), quota());
 }
 
 TEST_F(QuotaManagerTest, GetSyncableQuota) {
@@ -1276,14 +1277,18 @@ TEST_F(QuotaManagerTest, GetSyncableQuota) {
   EXPECT_LE(kAvailableSpaceForApp,
             QuotaManager::kSyncableStorageDefaultHostQuota);
 
-  // For unlimited origins the quota manager should return
-  // kAvailableSpaceForApp as syncable quota (because of the pre-condition).
+  // The quota manager should return
+  // QuotaManager::kSyncableStorageDefaultHostQuota as syncable quota,
+  // despite available space being less than the desired quota. Only
+  // origins with unlimited storage, which is never the case for syncable
+  // storage, shall have their quota calculation take into account the amount of
+  // available disk space.
   mock_special_storage_policy()->AddUnlimited(GURL("http://unlimited/"));
   GetUsageAndQuotaForWebApps(ToOrigin("http://unlimited/"), kSync);
   task_environment_.RunUntilIdle();
   EXPECT_EQ(QuotaStatusCode::kOk, status());
   EXPECT_EQ(0, usage());
-  EXPECT_EQ(kAvailableSpaceForApp, quota());
+  EXPECT_EQ(QuotaManager::kSyncableStorageDefaultHostQuota, quota());
 }
 
 TEST_F(QuotaManagerTest, GetPersistentUsageAndQuota_MultiOrigins) {

testing/variations/fieldtrial_testing_config.json

@@ -5158,26 +5158,6 @@
             ]
         }
     ],
-    "StaticHostQuota": [
-        {
-            "platforms": [
-                "android",
-                "android_webview",
-                "chromeos",
-                "linux",
-                "mac",
-                "windows"
-            ],
-            "experiments": [
-                {
-                    "name": "Enabled",
-                    "enable_features": [
-                        "StaticHostQuota"
-                    ]
-                }
-            ]
-        }
-    ],
     "SyncButterWallet": [
         {
             "platforms": [