Fix crash in PrecacheManager::OnDone().

OnDone() should check precache_completion_callback_ for null before
calling it, as it may have been nulled out by a call to
CancelPrecaching().

BUG=541319

Review URL: https://codereview.chromium.org/1437743002

Cr-Commit-Position: refs/heads/master@{#359130}

2 files changed, 38 insertions, 6 deletions

diff --git a/components/precache/content/precache_manager.cc b/components/precache/content/precache_manager.cc
index 3e91b920..3d317ee 100644
--- a/components/precache/content/precache_manager.cc
+++ b/components/precache/content/precache_manager.cc
@@ -242,9 +242,14 @@ void PrecacheManager::OnDone() {
 
   precache_fetcher_.reset();
 
-  precache_completion_callback_.Run(!is_precaching_);
-  // Uninitialize the callback so that any scoped_refptrs in it are released.
-  precache_completion_callback_.Reset();
+  // Run completion callback if not null. It's null if the client is in the
+  // Control group and CancelPrecaching is called before TopHosts computation
+  // finishes.
+  if (!precache_completion_callback_.is_null()) {
+    precache_completion_callback_.Run(!is_precaching_);
+    // Uninitialize the callback so that any scoped_refptrs in it are released.
+    precache_completion_callback_.Reset();
+  }
 
   is_precaching_ = false;
 }
diff --git a/components/precache/content/precache_manager_unittest.cc b/components/precache/content/precache_manager_unittest.cc
index bccbbda..674883b 100644
--- a/components/precache/content/precache_manager_unittest.cc
+++ b/components/precache/content/precache_manager_unittest.cc
@@ -117,10 +117,17 @@ class PrecacheManagerUnderTest : public PrecacheManager {
   PrecacheManagerUnderTest(content::BrowserContext* browser_context,
                            const sync_driver::SyncService* const sync_service,
                            const history::HistoryService* const history_service)
-      : PrecacheManager(browser_context, sync_service, history_service) {}
-  bool IsInExperimentGroup() const override { return true; }
-  bool IsInControlGroup() const override { return false; }
+      : PrecacheManager(browser_context, sync_service, history_service),
+        control_group_(false) {}
+  bool IsInExperimentGroup() const override { return !control_group_; }
+  bool IsInControlGroup() const override { return control_group_; }
   bool IsPrecachingAllowed() const override { return true; }
+  void SetInControlGroup(bool in_control_group) {
+    control_group_ = in_control_group;
+  }
+
+ private:
+  bool control_group_;
 };
 
 class PrecacheManagerTest : public testing::Test {
@@ -184,6 +191,26 @@ TEST_F(PrecacheManagerTest, StartAndFinishPrecaching) {
   EXPECT_EQ(expected_requested_urls, url_callback_.requested_urls());
 }
 
+TEST_F(PrecacheManagerTest, StartAndCancelPrecachingBeforeTopHostsCompleted) {
+  EXPECT_FALSE(precache_manager_.IsPrecaching());
+
+  MockHistoryService::TopHostsCallback top_hosts_callback;
+  EXPECT_CALL(history_service_, TopHosts(NumTopHosts(), _))
+      .WillOnce(SaveArg<1>(&top_hosts_callback));
+
+  precache_manager_.SetInControlGroup(true);
+  precache_manager_.StartPrecaching(precache_callback_.GetCallback());
+  EXPECT_TRUE(precache_manager_.IsPrecaching());
+
+  precache_manager_.CancelPrecaching();
+  EXPECT_FALSE(precache_manager_.IsPrecaching());
+
+  top_hosts_callback.Run(
+      history::TopHostsList(1, std::make_pair("starting-url.com", 1)));
+  EXPECT_FALSE(precache_manager_.IsPrecaching());
+  EXPECT_FALSE(precache_callback_.was_on_done_called());
+}
+
 TEST_F(PrecacheManagerTest, StartAndCancelPrecachingBeforeURLsReceived) {
   EXPECT_FALSE(precache_manager_.IsPrecaching());