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

storage/browser/quota/quota_manager.cc

 // Copyright 2013 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 "storage/browser/quota/quota_manager.h"
 
 #include <stddef.h>
 #include <stdint.h>
 #include <algorithm>
 #include <functional>
 #include <limits>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/files/file_util.h"
 #include "base/macros.h"
 #include "base/metrics/histogram.h"
 #include "base/profiler/scoped_tracker.h"
 #include "base/sequenced_task_runner.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/sys_info.h"
 #include "base/task_runner_util.h"
+#include "base/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "base/trace_event/trace_event.h"
 #include "net/base/url_util.h"
 #include "storage/browser/quota/client_usage_tracker.h"
 #include "storage/browser/quota/quota_manager_proxy.h"
 #include "storage/browser/quota/quota_temporary_storage_evictor.h"
 #include "storage/browser/quota/storage_monitor.h"
 #include "storage/browser/quota/usage_tracker.h"
 #include "storage/common/quota/quota_types.h"
 
@@ skipping 15 matching lines @@
       1, 10 * 1024 * 1024 /* 10TB */, 100)
 
 namespace storage {
 
 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%
+const double kTemporaryQuotaRatio = 1.0 / 3.0;  // 33%

[jsbell, 2016/03/23 18:08:28]

While you're here, can you comment on what these are used for?

[michaeln, 2016/04/07 00:32:34]

Done, also moved these values to where they're used so it's more obvious how
they're used

+
+#if defined(OS_ANDROID)
+const uint64_t kOsAccomodation = 250 * kMBytes;

[michaeln, 2016/03/22 22:23:55]

these specific numbers are just placeholders, this should probably be bigger for
android and we probably need something specific for chromecast too, and blimp???

how can we tell if the build target is chromecast

+#elif defined(OS_CHROMEOS)
+const uint64_t kOsAccomodation = 1000 * kMBytes;
+#else
+const uint64_t kOsAccomodation = 10000 * kMBytes;
+#endif
 
 }  // namespace
 
 // Arbitrary for now, but must be reasonably small so that
 // in-memory databases can fit.
 // TODO(kinuko): Refer SysInfo::AmountOfPhysicalMemory() to determine this.
 const int64_t QuotaManager::kIncognitoDefaultQuotaLimit = 100 * kMBytes;
 
 const int64_t QuotaManager::kNoLimit = INT64_MAX;
 
@@ skipping 164 matching lines @@
 }
 
 bool UpdateModifiedTimeOnDBThread(const GURL& origin,
                                   StorageType type,
                                   base::Time modified_time,
                                   QuotaDatabase* database) {
   DCHECK(database);
   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) {
   if (available_disk_space < QuotaManager::kMinimumPreserveForSystem) {
     LOG(WARNING)
         << "Running out of disk space for profile."
         << " QuotaManager starts forbidding further quota consumption.";
     return usage;
   }
 
   if (quota < usage) {
     // No more space; cap the quota to the current usage.
     return usage;
   }
 
   available_disk_space -= QuotaManager::kMinimumPreserveForSystem;
   if (available_disk_space < quota - usage)
     return available_disk_space + usage;
 
   return quota;
 }
 
 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)

[michaeln, 2016/03/22 21:40:15]

This used to defend against filling the device.

-    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)

[michaeln, 2016/03/22 21:40:15]

~66 MBytes for small andoid devcices and ~66GB for PCs with terrabyte drives,
maybe add clamp this to no more than 10G?

too_low = std::min(computed_too_low, 10GB)

[jsbell, 2016/03/23 18:08:29]

Would the "save a bit of room for non-Chrome usage" calc go in here as well?

[michaeln, 2016/04/07 00:32:34]

This is one of the two places where logic to "save a bit for others" comes into
play. The other place is around |must_remain_available_space| in the
QuotaTemporaryStorageEvictor, it uses 1/10 the volume size for that value.

If chrome is run on an approaching full disk, this check will prevent chrome
from writing data to disk.

+  const int64_t too_low_threshold = desired_quota;
+
+  // If disk space is too low, cap usage at current levels.
+  if (available_disk_space < too_low_threshold)

[michaeln, 2016/03/22 21:40:15]

That defense is replaced with this.

+    return std::min(desired_quota, host_usage);
+
+  // If its close to being too low, cap growth to avoid it getting too low.
+  int64_t remaining_quota = std::min(INT64_C(0), desired_quota - host_usage);

[jsbell, 2016/03/23 18:08:28]

I think this might be clearer as:

return std::min(
  desired_quota,
  host_usage + (available_disk_space - too_low_threshold));

(sanity check that this is the same?)

[michaeln, 2016/04/07 00:32:34]

I think your right. I had thought a variable named 'remaining_quota' might help
explain what its doing but smushing it into a single line helps more. Your
equation looks right, I added the inner std::max to prevent returning a negative
number.

return std::min(
    desired_quota,
    host_usage + std::max(0, available_disk_space - too_low_threshold));

+  if (available_disk_space < remaining_quota + too_low_threshold) {
+    remaining_quota = available_disk_space - too_low_threshold;
+    return host_usage + remaining_quota;
+  }
+
+  return desired_quota;
 }
 
 void DispatchUsageAndQuotaForWebApps(
     StorageType type,
     bool is_incognito,
     bool is_unlimited,
     bool can_query_disk_size,
     const QuotaManager::GetUsageAndQuotaCallback& callback,
     QuotaStatusCode status,
     const UsageAndQuota& usage_and_quota) {
   if (status != kQuotaStatusOk) {
     callback.Run(status, 0, 0);
     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;
   }
 
   // For apps with unlimited permission or can_query_disk_size is true (and not
   // in incognito mode).
   // We assume we can expose the actual disk size for them and cap the quota by
   // 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
 
 UsageAndQuota::UsageAndQuota()
     : usage(0),
       global_limited_usage(0),
       quota(0),
       available_disk_space(0) {
 }
@@ skipping 559 matching lines @@
   bool unlimited = IsStorageUnlimited(origin, type);
   bool can_query_disk_size = CanQueryDiskSize(origin);
 
   UsageAndQuotaCallbackDispatcher* dispatcher =
       new UsageAndQuotaCallbackDispatcher(this);
 
   if (unlimited) {
     dispatcher->set_quota(kNoLimit);
   } else {
     if (type == kStorageTypeTemporary) {
-      GetUsageTracker(type)->GetGlobalLimitedUsage(
-          dispatcher->GetGlobalLimitedUsageCallback());
       GetTemporaryGlobalQuota(dispatcher->GetQuotaCallback());
     } else if (type == kStorageTypePersistent) {
       GetPersistentHostQuota(net::GetHostOrSpecFromURL(origin),
                              dispatcher->GetQuotaCallback());
     } else {
       dispatcher->set_quota(kSyncableStorageDefaultHostQuota);
     }
   }
 
   DCHECK(GetUsageTracker(type));
   GetUsageTracker(type)->GetHostUsage(net::GetHostOrSpecFromURL(origin),
                                       dispatcher->GetHostUsageCallback());
 
-  if (!is_incognito_ && (unlimited || can_query_disk_size))
-    GetAvailableSpace(dispatcher->GetAvailableSpaceCallback());
+  GetAvailableSpace(dispatcher->GetAvailableSpaceCallback());

[jsbell, 2016/03/23 18:08:29]

Can we avoid this if is_incognito_ like we used to?

[michaeln, 2016/04/07 00:32:34]

that's a good question, the system is currently using GetAvailableSpace() for
the incognito case for some things, i'll have to dig some more? maybe we can use
constants for volume info in the incog case?

 
   dispatcher->WaitForResults(base::Bind(
       &DispatchUsageAndQuotaForWebApps,
       type, is_incognito_, unlimited, can_query_disk_size,
       callback));
 }
 
 void QuotaManager::GetUsageAndQuota(
     const GURL& origin, StorageType type,
     const GetUsageAndQuotaCallback& callback) {
@@ skipping 95 matching lines @@
         &QuotaManager::GetTemporaryGlobalQuota,
         weak_factory_.GetWeakPtr(), callback));
     return;
   }
 
   if (temporary_quota_override_ > 0) {
     callback.Run(kQuotaStatusOk, temporary_quota_override_);
     return;
   }
 
-  UsageAndQuotaCallbackDispatcher* dispatcher =
-      new UsageAndQuotaCallbackDispatcher(this);
-  GetUsageTracker(kStorageTypeTemporary)->
-      GetGlobalLimitedUsage(dispatcher->GetGlobalLimitedUsageCallback());
-  GetAvailableSpace(dispatcher->GetAvailableSpaceCallback());
-  dispatcher->WaitForResults(
-      base::Bind(&DispatchTemporaryGlobalQuotaCallback, callback));
+  db_thread_->PostTask(
+      FROM_HERE,
+      base::Bind(&QuotaManager::CalculateTemporaryPoolSize,
+                 get_volume_info_fn_, profile_path_, callback,
+                 base::ThreadTaskRunnerHandle::Get()));
 }
 
 void QuotaManager::SetTemporaryGlobalOverrideQuota(
     int64_t new_quota,
     const QuotaCallback& callback) {
   LazyInitialize();
 
   if (new_quota < 0) {
     if (!callback.is_null())
       callback.Run(kQuotaErrorInvalidModification, -1);
@@ skipping 712 matching lines @@
   // |database_|, therefore we can be sure that database_ is alive when this
   // task runs.
   base::PostTaskAndReplyWithResult(
       db_thread_.get(),
       from_here,
       base::Bind(task, base::Unretained(database_.get())),
       reply);
 }
 
 // static
+void QuotaManager::CalculateTemporaryPoolSize(
+    GetVolumeInfoFn get_volume_info_fn,
+    const base::FilePath& profile_path,
+    const QuotaCallback& callback,
+    const scoped_refptr<base::TaskRunner>& reply_runner) {
+  QuotaStatusCode status_code = kQuotaStatusUnknown;
+  int64_t pool_size = 0;
+  uint64_t available, total;
+  if (get_volume_info_fn(profile_path, &available, &total)) {
+    status_code = kQuotaStatusOk;
+    if (total > kOsAccomodation)
+      total -= kOsAccomodation; // todo: what if total is super small???

[jsbell, 2016/03/23 18:08:28]

Maybe we want a threshold, e.g. os_reserve = min(kOsAccomodation, total * 0.75)
so that on a newfangled device that e.g. we treat as desktop we assume there's
*some* storage?

Also, maybe we should have a related DCHECK so we catch new configurations
during development, e.g. if you're running on a Windows VM with only 8GB disk ?
Then we'd know we need to adjust the calcs/variables.

[michaeln, 2016/04/07 00:32:34]

Done

+    pool_size = static_cast<int64_t>(total * kTemporaryQuotaRatio);
+    UMA_HISTOGRAM_MBYTES("Quota.GlobalTemporaryPoolSize", pool_size);
+  }
+  reply_runner->PostTask(
+      FROM_HERE, base::Bind(callback, status_code, pool_size));
+}
+
+// static
 int64_t QuotaManager::CallGetAmountOfFreeDiskSpace(
     GetVolumeInfoFn get_volume_info_fn,
     const base::FilePath& profile_path) {
   // crbug.com/349708
   TRACE_EVENT0("io", "CallSystemGetAmountOfFreeDiskSpace");
   if (!base::CreateDirectory(profile_path)) {
     LOG(WARNING) << "Create directory failed for path" << profile_path.value();
     return 0;
   }
   uint64_t available, total;
@@ skipping 23 matching lines @@
     return false;
   *available_space = static_cast<uint64_t>(stats.f_bavail) * stats.f_frsize;
   *total_size = static_cast<uint64_t>(stats.f_blocks) * stats.f_frsize;
 #else
 #error Not implemented
 #endif
   return true;
 }
 
 }  // namespace storage