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/metrics/field_trial.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) {
+  if (net_error_code == net::OK) {

[twifkak, 2016/11/18 00:18:00]

Add a DCHECK_CURRENTLY_ON here? Also, are you sure the callback is called on the
same thread? I can't find evidence to convince myself one way or the other.

[jamartin, 2016/11/18 03:55:31]

Discussed offline. It seems that most of this needs to happen on the IO thread.
Fixed.

+    DCHECK(cache_backend_);
+    int result = cache_backend_->CalculateSizeOfAllEntries(base::Bind(
+        &PrecacheManager::OnCacheSizeReceived, base::Unretained(this)));
+    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
+    // Assume there is no cache.
+    OnCacheSizeReceived(0);
+  }
+}
+
+void PrecacheManager::OnCacheSizeReceived(int cache_size_bytes) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (cache_size_bytes < min_cache_size_bytes_) {
+    OnDone();

[twifkak, 2016/11/17 21:52:04]

Add an UMA for the cache_size_bytes when this happens. (Ideally we'd log all the
error conditions (for which this is currently 0) in an enum, but maybe wait
until we discover that's a problem?)

[jamartin, 2016/11/18 03:55:32]

Done.

+  } else {
+    BrowserThread::PostTaskAndReplyWithResult(
+        BrowserThread::DB, FROM_HERE,
+        base::Bind(&PrecacheDatabase::GetUnfinishedWork,
+                   base::Unretained(precache_database_.get())),
+        base::Bind(&PrecacheManager::OnGetUnfinishedWorkDone, AsWeakPtr()));
+  }
+}
+
 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()));
+
+  // Continue with OnGetUnfinishedWorkDone only if the size of the cache is

[twifkak, 2016/11/17 21:52:04]

Perhaps this check should be inside OnGetUnfinishedWorkDone only if the prefetch
is new, rather than before it. If there is unfinished work, then the client had
a big enough cache at one point, so it is safe to proceed without checking
again.

[twifkak, 2016/11/18 01:26:41]

Actually, my next CL will want to pass the cache backend into PrecacheFetcher
(even for resumes), so it doesn't matter.

[jamartin, 2016/11/18 03:55:32]

Acknowledged.

+  // 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::HttpCache* cache =
+      content::BrowserContext::GetDefaultStoragePartition(browser_context_)
+          ->GetURLRequestContext()

[twifkak, 2016/11/17 21:52:04]

May return null?
https://cs.chromium.org/chromium/src/content/browser/storage_partition_impl.c...

[jamartin, 2016/11/18 03:55:32]

Done.

+          ->GetURLRequestContext()

[twifkak, 2016/11/17 21:52:04]

May return null:
https://cs.chromium.org/chromium/src/net/url_request/url_request_context_gett...

[jamartin, 2016/11/18 03:55:32]

Done.

+          ->http_transaction_factory()

[twifkak, 2016/11/17 21:52:04]

May return null?
https://cs.chromium.org/chromium/src/net/url_request/url_request_context.h?rc...

[jamartin, 2016/11/18 03:55:32]

Done.

+          ->GetCache();
+  if (cache) {
+    const int net_error_code = cache->GetBackend(
+        &cache_backend_, base::Bind(&PrecacheManager::OnCacheBackendReceived,
+                                    base::Unretained(this)));

[twifkak, 2016/11/17 23:15:19]

I think you should use AsWeakPtr() instead?

[jamartin, 2016/11/18 03:55:31]

Done.

+    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.
+    // TODO(jamartin): Verify that it is not only that it lacks initialization.
+    OnCacheSizeReceived(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