Tweaks to the precache triggering code.

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>
 
@@ skipping 41 matching lines @@
       base::FilePath(FILE_PATH_LITERAL("PrecacheDatabase"))));
 
   BrowserThread::PostTask(
       BrowserThread::DB, FROM_HERE,
       base::Bind(base::IgnoreResult(&PrecacheDatabase::Init),
                  precache_database_, db_path));
 }
 
 PrecacheManager::~PrecacheManager() {}
 
+bool PrecacheManager::ShouldRun() const {
+  // Verify IsPrecachingAllowed() before calling IsPrecachingEnabled(). This is
+  // because field trials are only assigned when requested. This allows us to
+  // create Control and Experiment groups that are limited to users for whom
+  // IsPrecachingAllowed() is true, thus accentuating the impact of precaching.
+  return IsPrecachingAllowed() && IsPrecachingEnabled();
+}
+
+bool PrecacheManager::WouldRun() const {
+  return IsPrecachingAllowed();
+}
+
 // static
 bool PrecacheManager::IsPrecachingEnabled() {
   return base::FieldTrialList::FindFullName(kPrecacheFieldTrialName) ==
              kPrecacheFieldTrialEnabledGroup ||
          base::CommandLine::ForCurrentProcess()->HasSwitch(
              switches::kEnablePrecache);
 }
 
-bool PrecacheManager::IsPrecachingAllowed() {
+bool PrecacheManager::IsPrecachingAllowed() const {
+  // SyncService delegates to SyncPrefs, which must be called on the UI thread.
   return sync_service_ &&
          sync_service_->GetActiveDataTypes().Has(syncer::SESSIONS) &&
          !sync_service_->GetEncryptedDataTypes().Has(syncer::SESSIONS);
 }
 
 void PrecacheManager::StartPrecaching(
     const PrecacheCompletionCallback& precache_completion_callback,
     const history::HistoryService& history_service) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   if (is_precaching_) {
     DLOG(WARNING) << "Cannot start precaching because precaching is already "
                      "in progress.";
     return;
   }
   is_precaching_ = true;
 
-  BrowserThread::PostTask(
-      BrowserThread::DB, FROM_HERE,
-      base::Bind(&PrecacheDatabase::DeleteExpiredPrecacheHistory,
-                 precache_database_, base::Time::Now()));
-
   precache_completion_callback_ = precache_completion_callback;
 
-  // Request NumTopHosts() top hosts. Note that PrecacheFetcher is further bound
-  // by the value of PrecacheConfigurationSettings.top_sites_count, as retrieved
-  // from the server.
-  history_service.TopHosts(
-      NumTopHosts(),
-      base::Bind(&PrecacheManager::OnHostsReceived, AsWeakPtr()));
+  if (ShouldRun()) {

[bengr, 2015/08/07 17:21:45]

Maybe do:

if (!ShouldRun()) {
  OnDone();
  return;
}

BrowserThread::PostTask(...

[twifkak, 2015/08/07 17:57:08]

I'm not a fan of it (see
https://codereview.chromium.org/1266243003/diff/80001/components/precache/con...).
If you feel strongly, say so. Note that this if-else clause will become an
if-elseif-else clause when I introduce a Control group in a bit.

+    BrowserThread::PostTask(
+        BrowserThread::DB, FROM_HERE,
+        base::Bind(&PrecacheDatabase::DeleteExpiredPrecacheHistory,
+                   precache_database_, base::Time::Now()));
+
+    // Request NumTopHosts() top hosts. Note that PrecacheFetcher is further
+    // bound by the value of PrecacheConfigurationSettings.top_sites_count, as
+    // retrieved from the server.
+    history_service.TopHosts(
+        NumTopHosts(),
+        base::Bind(&PrecacheManager::OnHostsReceived, AsWeakPtr()));
+  } else {
+    OnDone();
+  }
 }
 
 void PrecacheManager::CancelPrecaching() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   if (!is_precaching_) {
     // Do nothing if precaching is not in progress.
     return;
   }
   is_precaching_ = false;
@@ skipping 87 matching lines @@
   // Start precaching.
   precache_fetcher_.reset(
       new PrecacheFetcher(hosts, browser_context_->GetRequestContext(),
                           variations::GetVariationParamValue(
                               kPrecacheFieldTrialName, kManifestURLPrefixParam),
                           this));
   precache_fetcher_->Start();
 }
 
 }  // namespace precache