Change how the quota system computes the total poolsize for temporary storage

storage/browser/quota/quota_manager.cc

Index: storage/browser/quota/quota_manager.cc
diff --git a/storage/browser/quota/quota_manager.cc b/storage/browser/quota/quota_manager.cc
index 26e469a9264543b0008399cc9b11b0e4f2920860..31786daba0aa1d4a2aa0cb5d0d7f029c72c449bf 100644
--- a/storage/browser/quota/quota_manager.cc
+++ b/storage/browser/quota/quota_manager.cc
@@ -58,9 +58,7 @@ namespace {
 
 const int64_t kMBytes = 1024 * 1024;
 const int kMinutesInMilliSeconds = 60 * 1000;
-
 const int64_t kReportHistogramInterval = 60 * 60 * 1000;  // 1 hour
-const double kTemporaryQuotaRatioToAvail = 1.0 / 3.0;  // 33%
 
 }  // namespace
 
@@ -245,36 +243,6 @@ bool UpdateModifiedTimeOnDBThread(const GURL& origin,
   return database->SetOriginLastModifiedTime(origin, type, modified_time);
 }
 
-int64_t CalculateTemporaryGlobalQuota(int64_t global_limited_usage,
-                                      int64_t available_space) {
-  DCHECK_GE(global_limited_usage, 0);
-  int64_t avail_space = available_space;
-  if (avail_space <
-      std::numeric_limits<int64_t>::max() - global_limited_usage) {
-    // We basically calculate the temporary quota by
-    // [available_space + space_used_for_temp] * kTempQuotaRatio,
-    // but make sure we'll have no overflow.
-    avail_space += global_limited_usage;
-  }
-  int64_t pool_size = avail_space * kTemporaryQuotaRatioToAvail;
-  UMA_HISTOGRAM_MBYTES("Quota.GlobalTemporaryPoolSize", pool_size);
-  return pool_size;
-}
-
-void DispatchTemporaryGlobalQuotaCallback(
-    const QuotaCallback& callback,
-    QuotaStatusCode status,
-    const UsageAndQuota& usage_and_quota) {
-  if (status != kQuotaStatusOk) {
-    callback.Run(status, 0);
-    return;
-  }
-
-  callback.Run(status, CalculateTemporaryGlobalQuota(
-      usage_and_quota.global_limited_usage,
-      usage_and_quota.available_disk_space));
-}
-
 int64_t CalculateQuotaWithDiskSpace(int64_t available_disk_space,
                                     int64_t usage,
                                     int64_t quota) {
@@ -299,12 +267,20 @@ int64_t CalculateQuotaWithDiskSpace(int64_t available_disk_space,
 
 int64_t CalculateTemporaryHostQuota(int64_t host_usage,
                                     int64_t global_quota,
-                                    int64_t global_limited_usage) {
-  DCHECK_GE(global_limited_usage, 0);
-  int64_t host_quota = global_quota / QuotaManager::kPerHostTemporaryPortion;
-  if (global_limited_usage > global_quota)
-    host_quota = std::min(host_quota, host_usage);
-  return host_quota;
+                                    int64_t available_disk_space) {
+  const int64_t desired_quota = global_quota /
+                                    QuotaManager::kPerHostTemporaryPortion;
+
+  // We define "too low" as not enough room for another hosts
+  // nominal desired amount of data. (TODO: what should this be)

[jsbell, 2016/04/07 19:33:41]

This seems fine to me. (i.e. remove the TODO)

[michaeln, 2016/04/07 20:54:48]

Done.

+  const int64_t too_low_threshold = desired_quota;
+
+  // If disk space is too low, cap usage at current levels.
+  // If its close to being too low, cap growth to avoid it getting too low.

[jsbell, 2016/04/07 19:33:41]

nit: it's

[michaeln, 2016/04/07 20:54:47]

Done.

+  return std::min(
+      desired_quota,
+      host_usage +
+          std::max(INT64_C(0), available_disk_space - too_low_threshold));
 }
 
 void DispatchUsageAndQuotaForWebApps(
@@ -320,17 +296,19 @@ void DispatchUsageAndQuotaForWebApps(
     return;
   }
 
-  int64_t usage = usage_and_quota.usage;
-  int64_t quota = usage_and_quota.quota;
+  int64_t host_usage = usage_and_quota.usage;
+  int64_t global_quota = usage_and_quota.quota;
+  int64_t host_quota = global_quota;
 
   if (type == kStorageTypeTemporary && !is_unlimited) {
-    quota = CalculateTemporaryHostQuota(
-        usage, quota, usage_and_quota.global_limited_usage);
+    host_quota = CalculateTemporaryHostQuota(
+        host_usage, global_quota, usage_and_quota.available_disk_space);
   }
 
   if (is_incognito) {
-    quota = std::min(quota, QuotaManager::kIncognitoDefaultQuotaLimit);
-    callback.Run(status, usage, quota);
+    host_quota = std::min(host_quota,
+                          QuotaManager::kIncognitoDefaultQuotaLimit);
+    callback.Run(status, host_usage, host_quota);
     return;
   }
 
@@ -340,14 +318,14 @@ void DispatchUsageAndQuotaForWebApps(
   // the available disk space.
   if (is_unlimited || can_query_disk_size) {
     callback.Run(
-        status, usage,
+        status, host_usage,
         CalculateQuotaWithDiskSpace(
             usage_and_quota.available_disk_space,
-            usage, quota));
+            host_usage, host_quota));
     return;
   }
 
-  callback.Run(status, usage, quota);
+  callback.Run(status, host_usage, host_quota);
 }
 
 }  // namespace
@@ -927,8 +905,6 @@ void QuotaManager::GetUsageAndQuotaForWebApps(
     dispatcher->set_quota(kNoLimit);
   } else {
     if (type == kStorageTypeTemporary) {
-      GetUsageTracker(type)->GetGlobalLimitedUsage(
-          dispatcher->GetGlobalLimitedUsageCallback());
       GetTemporaryGlobalQuota(dispatcher->GetQuotaCallback());
     } else if (type == kStorageTypePersistent) {
       GetPersistentHostQuota(net::GetHostOrSpecFromURL(origin),
@@ -942,8 +918,7 @@ void QuotaManager::GetUsageAndQuotaForWebApps(
   GetUsageTracker(type)->GetHostUsage(net::GetHostOrSpecFromURL(origin),
                                       dispatcher->GetHostUsageCallback());
 
-  if (!is_incognito_ && (unlimited || can_query_disk_size))
-    GetAvailableSpace(dispatcher->GetAvailableSpaceCallback());
+  GetAvailableSpace(dispatcher->GetAvailableSpaceCallback());
 
   dispatcher->WaitForResults(base::Bind(
       &DispatchUsageAndQuotaForWebApps,
@@ -1036,7 +1011,7 @@ void QuotaManager::GetAvailableSpace(const AvailableSpaceCallback& callback) {
   // crbug.com/349708
   TRACE_EVENT0("io", "QuotaManager::GetAvailableSpace");
 
-  PostTaskAndReplyWithResult(
+  base::PostTaskAndReplyWithResult(
       db_thread_.get(),
       FROM_HERE,
       base::Bind(&QuotaManager::CallGetAmountOfFreeDiskSpace,
@@ -1059,13 +1034,14 @@ void QuotaManager::GetTemporaryGlobalQuota(const QuotaCallback& callback) {
     return;
   }
 
-  UsageAndQuotaCallbackDispatcher* dispatcher =
-      new UsageAndQuotaCallbackDispatcher(this);
-  GetUsageTracker(kStorageTypeTemporary)->
-      GetGlobalLimitedUsage(dispatcher->GetGlobalLimitedUsageCallback());
-  GetAvailableSpace(dispatcher->GetAvailableSpaceCallback());
-  dispatcher->WaitForResults(
-      base::Bind(&DispatchTemporaryGlobalQuotaCallback, callback));
+  base::PostTaskAndReplyWithResult(
+      db_thread_.get(),
+      FROM_HERE,
+      base::Bind(&QuotaManager::CalculateTemporaryPoolSize,
+                 get_volume_info_fn_, profile_path_),
+      base::Bind(&QuotaManager::DidCalculateTemporaryPoolSize,
+                 weak_factory_.GetWeakPtr(),
+                 callback));
 }
 
 void QuotaManager::SetTemporaryGlobalOverrideQuota(
@@ -1633,7 +1609,7 @@ void QuotaManager::AsyncGetVolumeInfo(
   DCHECK(io_thread_->BelongsToCurrentThread());
   uint64_t* available_space = new uint64_t(0);
   uint64_t* total_space = new uint64_t(0);
-  PostTaskAndReplyWithResult(
+  base::PostTaskAndReplyWithResult(
       db_thread_.get(),
       FROM_HERE,
       base::Bind(get_volume_info_fn_,
@@ -1772,6 +1748,12 @@ void QuotaManager::DidGetAvailableSpace(int64_t space) {
   available_space_callbacks_.Run(kQuotaStatusOk, space);
 }
 
+void QuotaManager::DidCalculateTemporaryPoolSize(
+    const QuotaCallback& callback, int64_t pool_size) {
+  QuotaStatusCode status = pool_size ? kQuotaStatusUnknown : kQuotaStatusOk;
+  callback.Run(status, pool_size);
+}
+
 void QuotaManager::DidDatabaseWork(bool success) {
   db_disabled_ = !success;
 }
@@ -1799,6 +1781,38 @@ void QuotaManager::PostTaskAndReplyWithResultForDBThread(
 }
 
 // static
+int64_t QuotaManager::CalculateTemporaryPoolSize(
+    GetVolumeInfoFn get_volume_info_fn,
+    const base::FilePath& profile_path) {
+  // A rough estimate of how much storage is consumed by the base OS and
+  // essential application code outside of the browser.
+#if defined(OS_ANDROID)
+  const uint64_t kOsAccomodation = 250 * kMBytes;
+#elif defined(OS_CHROMEOS)
+  const uint64_t kOsAccomodation = 1000 * kMBytes;
+#elif defined(OS_WIN) || defined(OS_LINUX) || defined(OS_MACOSX)
+  const uint64_t kOsAccomodation = 10000 * kMBytes;
+#else
+#error "Port: Need to define kOsAccomodation for unkown os."

[jsbell, 2016/04/07 19:33:41]

Typo: unknown

[michaeln, 2016/04/07 20:54:48]

Done.

+#endif
+
+  // The fraction of the device's storage the browser is willing to
+  // use for temporary storage, this is applied after having taken
+  // the OSAccomodation into account.

[jsbell, 2016/04/07 19:33:41]

nit: kOsAccomodation (or otherwise be consistent in naming)

[michaeln, 2016/04/07 20:54:47]

Done.

+  const double kTemporaryPoolSizeRatio = 1.0 / 3.0;  // 33%
+
+  int64_t pool_size = 0;
+  uint64_t available, total;
+  if (get_volume_info_fn(profile_path, &available, &total)) {
+    if (total > kOsAccomodation)
+      total -= kOsAccomodation; // todo: what if total is super small???

[jsbell, 2016/04/07 19:33:41]

To make the calculation clearer, can we leave `total` alone and use a different
variable for this intermediate size?

...

With this, if (kOsAccomodation / total) > 1 then we just ignore kOsAccomodation
(assumption: our numbers are "whack"). But if 1 > (kOsAccomodation / total) >
0.95 then we will get a ridiculously small pool size, but our numbers are also
probably whack.

I chatted w/ dknox offline, and we have two suggestions:

#1: if (kOsAccomodation / total) > 0.80, pool = (total * 0.2) / 3

i.e. "if we're pretty sure our kOsAccomodation number is whack, just make an
educated guess"

#2: if (kOsAccomodation / total) > 0.80, pool = (available) / 3

i.e. "if we're pretty sure our kOsAccomodation number is whack, base the
calculation on what's actually available" - this is basically falling back to
our old logic, and gets weird if we're not subtracting chrome's existing
footprint (which is what we're trying to avoid in this patch!!)

(in both cases the ratios are just made up)

The ideal option, of course, would be to ask the device how big the OS is.
Android knows this, but I don't think it's exposed.

[michaeln, 2016/04/07 20:54:47]

thnx for the suggestions, i had forgotten about adjusting this one in the last
snapshot. i'll try something along the lines of #1.

[michaeln, 2016/07/27 19:57:47]

Done in chrome_content_browser_client.cc

+    pool_size = static_cast<int64_t>(total * kTemporaryPoolSizeRatio);
+    UMA_HISTOGRAM_MBYTES("Quota.GlobalTemporaryPoolSize", pool_size);
+  }
+  return pool_size;
+}
+
+// static
 int64_t QuotaManager::CallGetAmountOfFreeDiskSpace(
     GetVolumeInfoFn get_volume_info_fn,
     const base::FilePath& profile_path) {