Avoid touching SafeBrowsingClientImpl's ref count before it's fully constructed

SafeBrowsingContextImpl is ref counted, and its first reference used to
be made in its constructor through base::Bind. The reference is passed
to the IO thread, and released when the task is destroyed.
However, if PostTask failed or the posted task ran soon even before the
constructor has finished, the SafeBrowsingContextImpl instance may be
destroyed before the construction has completed.

Though the SafeBrowsingContextImpl case is safe, even if
`new SafeBrowsingContextImpl` returns a stale pointer, this hits an
sanity check that is being added to base::Bind.

This CL moves the PostTask() out of the constructor, and holds an
reference to the instance while base::Bind is called to avoid the
check failure.

Bug: 866456
Change-Id: Iee58de1e4cd807ff858985f9831e48f4a903bbd4
Reviewed-on: https://chromium-review.googlesource.com/1156332Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579433}

chrome/browser/extensions/blacklist.cc

@@ -79,19 +79,26 @@ class SafeBrowsingClientImpl
 
   // Constructs a client to query the database manager for |extension_ids| and
   // run |callback| with the IDs of those which have been blacklisted.
-  SafeBrowsingClientImpl(
+  static void Start(
       const std::set<std::string>& extension_ids,
-      const OnResultCallback& callback)
-      : callback_task_runner_(base::ThreadTaskRunnerHandle::Get()),
-        callback_(callback) {
+      const OnResultCallback& callback) {
+    auto safe_browsing_client = base::WrapRefCounted(
+        new SafeBrowsingClientImpl(extension_ids, callback));
     BrowserThread::PostTask(
         BrowserThread::IO, FROM_HERE,
-        base::BindOnce(&SafeBrowsingClientImpl::StartCheck, this,
-                       g_database_manager.Get().get(), extension_ids));
+        base::BindOnce(&SafeBrowsingClientImpl::StartCheck,
+                       safe_browsing_client, g_database_manager.Get().get(),
+                       extension_ids));
   }
 
  private:
   friend class base::RefCountedThreadSafe<SafeBrowsingClientImpl>;
+
+  SafeBrowsingClientImpl(const std::set<std::string>& extension_ids,
+                         const OnResultCallback& callback)
+      : callback_task_runner_(base::ThreadTaskRunnerHandle::Get()),
+        callback_(callback) {}
+
   ~SafeBrowsingClientImpl() override {}
 
   // Pass |database_manager| as a parameter to avoid touching
@@ -197,9 +204,9 @@ void Blacklist::GetBlacklistedIDs(const std::set<std::string>& ids,
   // safebrowsing for the blacklisted extensions. The set of blacklisted
   // extensions returned by SafeBrowsing will then be passed to
   // GetBlacklistStateIDs to get the particular BlacklistState for each id.
-  new SafeBrowsingClientImpl(
-      ids, base::Bind(&Blacklist::GetBlacklistStateForIDs, AsWeakPtr(),
-                      callback));
+  SafeBrowsingClientImpl::Start(
+      ids,
+      base::Bind(&Blacklist::GetBlacklistStateForIDs, AsWeakPtr(), callback));
 }
 
 void Blacklist::GetMalwareIDs(const std::set<std::string>& ids,