Do not precache when the cache size is small

components/precache/content/precache_manager.cc

Index: components/precache/content/precache_manager.cc
diff --git a/components/precache/content/precache_manager.cc b/components/precache/content/precache_manager.cc
index f4333c12d64e9b02779ca6a8de304a99f2e8bcf5..f658c19919dffb250ed8f1ab97f854611f255521 100644
--- a/components/precache/content/precache_manager.cc
+++ b/components/precache/content/precache_manager.cc
@@ -11,7 +11,9 @@
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/logging.h"
+#include "base/memory/ref_counted.h"
 #include "base/metrics/field_trial.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/strings/string_util.h"
 #include "base/time/time.h"
 #include "components/history/core/browser/history_service.h"
@@ -26,6 +28,9 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/storage_partition.h"
 #include "net/base/network_change_notifier.h"
+#include "net/http/http_cache.h"
+#include "net/url_request/url_request_context.h"
+#include "net/url_request/url_request_context_getter.h"
 
 using content::BrowserThread;
 
@@ -111,6 +116,90 @@ PrecacheManager::AllowedType PrecacheManager::PrecachingAllowed() const {
   return AllowedType::DISALLOWED;
 }
 
+void PrecacheManager::OnCacheBackendReceived(int net_error_code) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  if (net_error_code == net::OK) {
+    DCHECK(cache_backend_);
+    int result = cache_backend_->CalculateSizeOfAllEntries(
+        base::Bind(&PrecacheManager::OnCacheSizeReceived, AsWeakPtr()));
+    if (result == net::ERR_IO_PENDING) {
+      // Wait for the callback.
+    } else if (result >= 0) {
+      // The result is the expected bytes already.
+      OnCacheSizeReceived(result);
+    } else {
+      // Error occurred. Couldn't get the size. Assume there is no cache.
+      OnCacheSizeReceived(0);
+    }
+  } else {  // net_error_code != net::OK

[gavinp, 2016/12/08 17:33:46]

Readability nit: couldn't this be moved up to the top

if (net_error_code != net::OK) {
  OnCacheSizeReceived(0);
  return;
}

// rest of function here.

Avoids nesting things; I think conforms better to style. wdyt?

[jamartin, 2016/12/08 22:43:58]

Done.

I don't have a strong opinion. Devin (twifkak) kind of convinced me that early
returns can make difficult control flows. Besides, one can argue that if you
avoid early-returns, and you end up having several nested levels, chances are
that you should refactorize the code into more functions.

+    // Assume there is no cache.
+    OnCacheSizeReceived(0);
+  }
+  cache_backend_ = nullptr;
+}
+
+void PrecacheManager::OnCacheSizeReceived(int cache_size_bytes) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&PrecacheManager::OnCacheSizeReceivedInUIThread, AsWeakPtr(),
+                 cache_size_bytes));
+}
+
+void PrecacheManager::OnCacheSizeReceivedInUIThread(int cache_size_bytes) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  UMA_HISTOGRAM_MEMORY_KB("Precache.CacheSize.AllEntries",
+                          cache_size_bytes / 1024);
+  if (cache_size_bytes < min_cache_size_bytes_) {
+    OnDone();  // Do not continue.
+  } else {
+    BrowserThread::PostTaskAndReplyWithResult(
+        BrowserThread::DB, FROM_HERE,
+        base::Bind(&PrecacheDatabase::GetUnfinishedWork,
+                   base::Unretained(precache_database_.get())),
+        base::Bind(&PrecacheManager::OnGetUnfinishedWorkDone, AsWeakPtr()));
+  }
+}
+
+void PrecacheManager::PrecacheIfCacheIsBigEnough(
+    scoped_refptr<net::URLRequestContextGetter> url_request_context_getter) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  CHECK(url_request_context_getter);
+
+  // Continue with OnGetUnfinishedWorkDone only if the size of the cache is
+  // at least min_cache_size_bytes_.
+  // Class disk_cache::Backend does not expose its maximum size. However, caches
+  // are usually full, so we can use the size of all the entries stored in the
+  // cache (via CalculateSizeOfAllEntries) as a proxy of its maximum size.
+  net::URLRequestContext* context =
+      url_request_context_getter->GetURLRequestContext();
+  if (!context) {
+    OnCacheSizeReceived(0);
+    return;
+  }
+  net::HttpTransactionFactory* factory = context->http_transaction_factory();
+  if (!factory) {
+    OnCacheSizeReceived(0);
+    return;
+  }
+  net::HttpCache* cache = factory->GetCache();
+  if (cache) {

[gavinp, 2016/12/08 17:33:46]

Is this test more of an assert? Could you ever really have an HttpNetworkLayer
here? If not, suggest removing the test and error handling, and just crashing.
But if it's possible that your context doesn't include an Http caching layer,
the test must stay.

[jamartin, 2016/12/08 22:43:57]

I don't know. I don't know enough about Chromium network layers and their
default settings. My gut feeling is that this is an unlikely scenario but it may
happen in forks, experiments and rare devices with so little memory that may
disable the cache.

Anyway, I'd rather not to risk crashing the browser. I believe the safest course
of action is to disable precaching in these situations.

+    const int net_error_code = cache->GetBackend(
+        &cache_backend_,
+        base::Bind(&PrecacheManager::OnCacheBackendReceived, AsWeakPtr()));
+    if (net_error_code != net::ERR_IO_PENDING) {
+      // No need to wait for the callback. The callback hasn't been called with
+      // the appropriate code, so we call it directly.
+      OnCacheBackendReceived(net_error_code);
+    }
+  } else {  // !cache.
+    // There is no known cache. Assume that there is no cache.
+    OnCacheSizeReceived(0);

[gavinp, 2016/12/08 17:33:46]

If keeping this test (see above), for readability I suggest moving this to just
after initializing the cache, like this:

if (!cache) {
  OnCacheSizeReceived(0);
  return;
}

[jamartin, 2016/12/08 22:43:58]

Done.

+  }
+}
+
 void PrecacheManager::StartPrecaching(
     const PrecacheCompletionCallback& precache_completion_callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -128,12 +217,18 @@ void PrecacheManager::StartPrecaching(
       base::Bind(&PrecacheDatabase::SetLastPrecacheTimestamp,
                  base::Unretained(precache_database_.get()),
                  base::Time::Now()));
-  BrowserThread::PostTaskAndReplyWithResult(
-      BrowserThread::DB,
-      FROM_HERE,
-      base::Bind(&PrecacheDatabase::GetUnfinishedWork,
-                 base::Unretained(precache_database_.get())),
-      base::Bind(&PrecacheManager::OnGetUnfinishedWorkDone, AsWeakPtr()));
+
+  scoped_refptr<net::URLRequestContextGetter> url_request_context_getter(
+      content::BrowserContext::GetDefaultStoragePartition(browser_context_)
+          ->GetURLRequestContext());
+  if (url_request_context_getter) {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&PrecacheManager::PrecacheIfCacheIsBigEnough, AsWeakPtr(),
+                   std::move(url_request_context_getter)));
+  } else {  // !url_request_context_getter_.
+    OnCacheSizeReceivedInUIThread(0);
+  }
 }
 
 void PrecacheManager::OnGetUnfinishedWorkDone(