Fix a DCHECK in UpdateClientImpl destructor

UpdateClientImpl DCHECKs that it's destructed only if list of pending
tasks is empty. Tasks have a callback bound to a update client
reference, and the UpdateClient keeps a references to pending update
tasks, so this should generally be true, but TaskUpdate invokes the
callback by posting it to a task runner, which moves the callback (and
a UpdateClientImpl reference) ownership to the task runner itself. If
Chrome shutsdown while the callback task is scheduled on the task
runner, the UpdateClientImpl will be deleted with the callback during
task runner tear-down, before the callback is run.

Also, fixes a task instance was referencing a callback bound to
unretained UpdateClientImpl, thus not keeping UpdateClient alive.

BUG=1079241

Change-Id: I6f6c81c9011ede03806362fd5cf52a3f36b36cf8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2205326Reviewed-by: Sorin Jianu <sorin@chromium.org>
Commit-Queue: Toni Baržić <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#769944}

components/update_client/task_update.cc

@@ -57,9 +57,16 @@ std::vector<std::string> TaskUpdate::GetIds() const {
 void TaskUpdate::TaskComplete(Error error) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
+  // NOTE: Do not post the callback_ directly to the task thread to ensure the
+  // UpdateClient reference (to which the callback is bound) does not get
+  // released before the callback is run during shutdown, when the task runner
+  // gets destroyed.
   base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::BindOnce(std::move(callback_), base::WrapRefCounted(this), error));
+      FROM_HERE, base::BindOnce(&TaskUpdate::RunCallback, this, error));
+}
+
+void TaskUpdate::RunCallback(Error error) {
+  std::move(callback_).Run(this, error);
 }
 
 }  // namespace update_client

components/update_client/task_update.h

@@ -53,6 +53,9 @@ class TaskUpdate : public Task {
   // it has been canceled.
   void TaskComplete(Error error);
 
+  // Runs the task update callback.
+  void RunCallback(Error error);
+
   base::ThreadChecker thread_checker_;
   scoped_refptr<UpdateEngine> update_engine_;
   const bool is_foreground_;

components/update_client/update_client.cc

@@ -238,7 +238,7 @@ void UpdateClientImpl::SendUninstallPing(const std::string& id,
 
   RunTask(base::MakeRefCounted<TaskSendUninstallPing>(
       update_engine_.get(), id, version, reason,
-      base::BindOnce(&UpdateClientImpl::OnTaskComplete, base::Unretained(this),
+      base::BindOnce(&UpdateClientImpl::OnTaskComplete, this,
                      std::move(callback))));
 }