Do not precache when the cache size is small

components/precache/content/precache_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 "components/precache/content/precache_manager.h"
 
 #include <string>
 #include <utility>
 #include <vector>
 
 #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"
 #include "components/precache/core/precache_database.h"
 #include "components/precache/core/precache_switches.h"
 #include "components/precache/core/proto/unfinished_work.pb.h"
 #include "components/prefs/pref_service.h"
 #include "components/sync/driver/sync_service.h"
 #include "components/variations/metrics_util.h"
 #include "components/variations/variations_associated_data.h"
 #include "content/public/browser/browser_context.h"
 #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;
 
 namespace {
 
 const char kPrecacheFieldTrialName[] = "Precache";
 const char kPrecacheFieldTrialEnabledGroup[] = "Enabled";
 const char kPrecacheFieldTrialControlGroup[] = "Control";
 const char kConfigURLParam[] = "config_url";
 const char kManifestURLPrefixParam[] = "manifest_url_prefix";
@@ skipping 65 matching lines @@
 
   // SyncService delegates to SyncPrefs, which must be called on the UI thread.
   if (history_service_ &&
       sync_service_->GetActiveDataTypes().Has(syncer::SESSIONS) &&
       !sync_service_->GetEncryptedDataTypes().Has(syncer::SESSIONS))
     return AllowedType::ALLOWED;
 
   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);
 
   if (is_precaching_) {
     DLOG(WARNING) << "Cannot start precaching because precaching is already "
                      "in progress.";
     return;
   }
   precache_completion_callback_ = precache_completion_callback;
 
   is_precaching_ = true;
   BrowserThread::PostTask(
       BrowserThread::DB, FROM_HERE,
       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(
     std::unique_ptr<PrecacheUnfinishedWork> unfinished_work) {
   // Reset progress on a prefetch that has taken too long to complete.
   if (unfinished_work->has_start_time() &&
       base::Time::Now() -
               base::Time::FromInternalValue(unfinished_work->start_time()) >
           base::TimeDelta::FromHours(6)) {
     PrecacheFetcher::RecordCompletionStatistics(
@@ skipping 208 matching lines @@
       this));
   precache_fetcher_->Start();
 }
 
 void PrecacheManager::OnHostsReceivedThenDone(
     const history::TopHostsList& host_counts) {
   OnDone();
 }
 
 }  // namespace precache