Make InstallableManager PostTask to continue processing after calling callback.

Removes the assumption that InstallableManager currently has: that it will
continue to exist after calling the callback. This makes sense if considering
the callback as a return-like statement for the async GetData call.

In prep for refactoring in http://crrev.com/c/1873817

Bug: 1007860
Change-Id: I18f751b32c88a9d3cc9d6c3f3a083fd3952ebe9f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1884409
Commit-Queue: Glen Robertson <glenrob@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710242}

chrome/browser/installable/installable_manager.cc

@@ -11,6 +11,8 @@
 #include "base/callback.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
+#include "base/task/post_task.h"
+#include "base/task/task_traits.h"
 #include "build/build_config.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ssl/security_state_tab_helper.h"
@@ -422,6 +424,17 @@ void InstallableManager::SetManifestDependentTasksComplete() {
   SetIconFetched(IconPurpose::MASKABLE);
 }
 
+void InstallableManager::CleanupAndStartNextTask() {
+  // Sites can always register a service worker after we finish checking, so
+  // don't cache a missing service worker error to ensure we always check
+  // again.
+  if (worker_error() == NO_MATCHING_SERVICE_WORKER)
+    worker_ = std::make_unique<ServiceWorkerProperty>();
+
+  task_queue_.Next();
+  WorkOnTask();
+}
+
 void InstallableManager::RunCallback(
     InstallableTask task,
     std::vector<InstallableStatusCode> errors) {
@@ -458,17 +471,14 @@ void InstallableManager::WorkOnTask() {
   auto errors = GetErrors(params);
   bool check_passed = errors.empty();
   if ((!check_passed && !params.is_debug_mode) || IsComplete(params)) {
+    // Yield the UI thread before processing the next task. If this object is
+    // deleted in the meantime, the next task naturally won't run.
+    base::PostTask(FROM_HERE, {base::CurrentThread()},
+                   base::BindOnce(&InstallableManager::CleanupAndStartNextTask,
+                                  weak_factory_.GetWeakPtr()));
+
     auto task = std::move(task_queue_.Current());
     RunCallback(std::move(task), std::move(errors));
-
-    // Sites can always register a service worker after we finish checking, so
-    // don't cache a missing service worker error to ensure we always check
-    // again.
-    if (worker_error() == NO_MATCHING_SERVICE_WORKER)
-      worker_ = std::make_unique<ServiceWorkerProperty>();
-
-    task_queue_.Next();
-    WorkOnTask();
     return;
   }
 

chrome/browser/installable/installable_manager.h

@@ -179,6 +179,7 @@ class InstallableManager
   void SetManifestDependentTasksComplete();
 
   // Methods coordinating and dispatching work for the current task.
+  void CleanupAndStartNextTask();
   void RunCallback(InstallableTask task,
                    std::vector<InstallableStatusCode> errors);
   void WorkOnTask();

chrome/browser/installable/installable_manager_browsertest.cc

@@ -8,6 +8,8 @@
 #include "base/command_line.h"
 #include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/task/post_task.h"
+#include "base/task/task_traits.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "chrome/browser/banners/app_banner_manager_desktop.h"
@@ -130,7 +132,7 @@ class CallbackTester {
       badge_icon_.reset(new SkBitmap(*data.badge_icon));
     valid_manifest_ = data.valid_manifest;
     has_worker_ = data.has_worker;
-    quit_closure_.Run();
+    base::PostTask(FROM_HERE, {base::CurrentThread()}, quit_closure_);
   }
 
   const std::vector<InstallableStatusCode>& errors() const { return errors_; }

chrome/browser/web_applications/components/install_manager.cc

@@ -17,14 +17,7 @@ void OnInstallabilityCheckCompleted(
     InstallManager::WebAppInstallabilityCheckCallback callback,
     std::unique_ptr<content::WebContents> web_contents,
     const InstallableData& data) {
-  // This task is posted. This is because this function will be called by the
-  // InstallableManager associated with |web_contents|, and |web_contents| might
-  // be freed in the callback. If that happens, and this isn't posted, the
-  // InstallableManager will be freed, and then when this function returns to
-  // the InstallManager calling function, there will be a crash.
-  base::SequencedTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::BindOnce(std::move(callback), std::move(web_contents),
-                                data.errors.empty()));
+  std::move(callback).Run(std::move(web_contents), data.errors.empty());
 }
 
 void OnWebAppUrlLoaded(