Revert "[TaskScheduler] Migrate some extension installer code to TaskScheduler."

This reverts commit 05dabf99.

Reason for revert: Data race https://crbug.com/750382 for details

Original change's description:
> [TaskScheduler] Migrate some extension installer code to TaskScheduler.
> 
> Bug: 689520
> Change-Id: I8743112968e261459d8aee5491e2280da1461661
> Reviewed-on: https://chromium-review.googlesource.com/585386
> Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org>
> Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#489747}

TBR=lazyboy@chromium.org,rdevlin.cronin@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

NOPRESUBMIT=True

Bug: 689520, 750382 
Change-Id: Ic7065c7ebbae93997f5008164d8c555d032d3150
Reviewed-on: https://chromium-review.googlesource.com/591935
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490607}

chrome/browser/extensions/unpacked_installer.cc

@@ -11,7 +11,6 @@
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
-#include "base/task_scheduler/post_task.h"
 #include "base/threading/thread_restrictions.h"
 #include "chrome/browser/extensions/extension_error_reporter.h"
 #include "chrome/browser/extensions/extension_install_prompt.h"
@@ -52,11 +51,6 @@ const char kImportMinVersionNewer[] =
 const char kImportMissing[] = "'import' extension is not installed.";
 const char kImportNotSharedModule[] = "'import' is not a shared module.";
 
-constexpr base::TaskTraits kTraits = {
-    base::MayBlock(),                   // Needs file access.
-    base::TaskPriority::USER_BLOCKING,  // Install is triggered by UI.
-    base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN};
-
 // Manages an ExtensionInstallPrompt for a particular extension.
 class SimpleExtensionLoadPrompt {
  public:
@@ -135,8 +129,8 @@ UnpackedInstaller::~UnpackedInstaller() {
 void UnpackedInstaller::Load(const base::FilePath& path_in) {
   DCHECK(extension_path_.empty());
   extension_path_ = path_in;
-  base::PostTaskWithTraits(
-      FROM_HERE, kTraits,
+  BrowserThread::PostTask(
+      BrowserThread::FILE, FROM_HERE,
       base::BindOnce(&UnpackedInstaller::GetAbsolutePath, this));
 }
 
@@ -307,7 +301,7 @@ bool UnpackedInstaller::IsLoadingUnpackedAllowed() const {
 }
 
 void UnpackedInstaller::GetAbsolutePath() {
-  base::ThreadRestrictions::AssertIOAllowed();
+  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
   extension_path_ = base::MakeAbsoluteFilePath(extension_path_);
 
@@ -334,13 +328,13 @@ void UnpackedInstaller::CheckExtensionFileAccess() {
     return;
   }
 
-  base::PostTaskWithTraits(
-      FROM_HERE, kTraits,
+  BrowserThread::PostTask(
+      BrowserThread::FILE, FROM_HERE,
       base::BindOnce(&UnpackedInstaller::LoadWithFileAccess, this, GetFlags()));
 }
 
 void UnpackedInstaller::LoadWithFileAccess(int flags) {
-  base::ThreadRestrictions::AssertIOAllowed();
+  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
   std::string error;
   extension_ = file_util::LoadExtension(extension_path_, Manifest::UNPACKED,

chrome/browser/extensions/user_script_listener_unittest.cc

@@ -24,7 +24,6 @@
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/resource_throttle.h"
 #include "content/public/test/test_browser_thread_bundle.h"
-#include "content/public/test/test_utils.h"
 #include "extensions/browser/extension_registry.h"
 #include "net/base/request_priority.h"
 #include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
@@ -208,7 +207,6 @@ class UserScriptListenerTest : public testing::Test {
         .AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj")
         .AppendASCII("1.0.0.0");
     UnpackedInstaller::Create(service_)->Load(extension_path);
-    content::RunAllBlockingPoolTasksUntilIdle();
   }
 
   void UnloadTestExtension() {
@@ -233,6 +231,7 @@ namespace {
 
 TEST_F(UserScriptListenerTest, DelayAndUpdate) {
   LoadTestExtension();
+  base::RunLoop().RunUntilIdle();
 
   net::TestDelegate delegate;
   net::TestURLRequestContext context;
@@ -250,6 +249,7 @@ TEST_F(UserScriptListenerTest, DelayAndUpdate) {
 
 TEST_F(UserScriptListenerTest, DelayAndUnload) {
   LoadTestExtension();
+  base::RunLoop().RunUntilIdle();
 
   net::TestDelegate delegate;
   net::TestURLRequestContext context;
@@ -287,6 +287,7 @@ TEST_F(UserScriptListenerTest, NoDelayNoExtension) {
 
 TEST_F(UserScriptListenerTest, NoDelayNotMatching) {
   LoadTestExtension();
+  base::RunLoop().RunUntilIdle();
 
   net::TestDelegate delegate;
   net::TestURLRequestContext context;
@@ -302,6 +303,7 @@ TEST_F(UserScriptListenerTest, NoDelayNotMatching) {
 
 TEST_F(UserScriptListenerTest, MultiProfile) {
   LoadTestExtension();
+  base::RunLoop().RunUntilIdle();
 
   // Fire up a second profile and have it load an extension with a content
   // script.
@@ -347,6 +349,7 @@ TEST_F(UserScriptListenerTest, MultiProfile) {
 // throttles.
 TEST_F(UserScriptListenerTest, ResumeBeforeStart) {
   LoadTestExtension();
+  base::RunLoop().RunUntilIdle();
   net::TestDelegate delegate;
   net::TestURLRequestContext context;
   GURL url(kMatchingUrl);

chrome/browser/extensions/webstore_installer.cc

@@ -24,8 +24,6 @@
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
-#include "base/task_scheduler/post_task.h"
-#include "base/threading/thread_restrictions.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "chrome/browser/chrome_notification_types.h"
@@ -111,14 +109,19 @@ const base::FilePath::CharType kWebstoreDownloadFolder[] =
 
 base::FilePath* g_download_directory_for_tests = NULL;
 
-base::FilePath GetDownloadFilePath(const base::FilePath& download_directory,
-                                   const std::string& id) {
-  base::ThreadRestrictions::AssertIOAllowed();
+// Must be executed on the FILE thread.
+void GetDownloadFilePath(
+    const base::FilePath& download_directory,
+    const std::string& id,
+    const base::Callback<void(const base::FilePath&)>& callback) {
   // Ensure the download directory exists. TODO(asargent) - make this use
   // common code from the downloads system.
-  if (!base::DirectoryExists(download_directory) &&
-      !base::CreateDirectory(download_directory)) {
-    return base::FilePath();
+  if (!base::DirectoryExists(download_directory)) {
+    if (!base::CreateDirectory(download_directory)) {
+      BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                              base::BindOnce(callback, base::FilePath()));
+      return;
+    }
   }
 
   // This is to help avoid a race condition between when we generate this
@@ -138,7 +141,8 @@ base::FilePath GetDownloadFilePath(const base::FilePath& download_directory,
         base::StringPrintf(" (%d)", uniquifier));
   }
 
-  return file;
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                          base::BindOnce(callback, file));
 }
 
 void MaybeAppendAuthUserParameter(const std::string& authuser, GURL* url) {
@@ -597,14 +601,11 @@ void WebstoreInstaller::DownloadCrx(
   }
 #endif
 
-  constexpr base::TaskTraits kTraits = {
-      base::MayBlock(),                   // Needs file access.
-      base::TaskPriority::USER_BLOCKING,  // Install is triggered by UI.
-      base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN};
-  base::PostTaskWithTraitsAndReplyWithResult(
-      FROM_HERE, kTraits,
-      base::BindOnce(&GetDownloadFilePath, download_directory, extension_id),
-      base::BindOnce(&WebstoreInstaller::StartDownload, this, extension_id));
+  BrowserThread::PostTask(
+      BrowserThread::FILE, FROM_HERE,
+      base::BindOnce(
+          &GetDownloadFilePath, download_directory, extension_id,
+          base::Bind(&WebstoreInstaller::StartDownload, this, extension_id)));
 }
 
 // http://crbug.com/165634

chrome/browser/extensions/zipfile_installer.cc

@@ -6,7 +6,6 @@
 
 #include "base/files/file_util.h"
 #include "base/path_service.h"
-#include "base/task_scheduler/post_task.h"
 #include "chrome/browser/extensions/extension_error_reporter.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/unpacked_installer.h"
@@ -23,23 +22,6 @@ const char kExtensionHandlerTempDirError[] =
 const char kExtensionHandlerFileUnzipError[] =
     "Could not unzip extension for install.";
 
-base::Optional<base::FilePath> PrepareAndGetUnzipDir(
-    const base::FilePath& zip_file) {
-  base::ThreadRestrictions::AssertIOAllowed();
-
-  base::FilePath dir_temp;
-  base::PathService::Get(base::DIR_TEMP, &dir_temp);
-
-  base::FilePath::StringType dir_name =
-      zip_file.RemoveExtension().BaseName().value() + FILE_PATH_LITERAL("_");
-
-  base::FilePath unzip_dir;
-  if (!base::CreateTemporaryDirInDir(dir_temp, dir_name, &unzip_dir))
-    return base::Optional<base::FilePath>();
-
-  return unzip_dir;
-}
-
 }  // namespace
 
 namespace extensions {
@@ -57,13 +39,9 @@ void ZipFileInstaller::LoadFromZipFile(const base::FilePath& zip_file) {
 
   zip_file_ = zip_file;
 
-  constexpr base::TaskTraits kTraits = {
-      base::MayBlock(),                   // Needs file access.
-      base::TaskPriority::USER_BLOCKING,  // Install is triggered by UI.
-      base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN};
-  base::PostTaskWithTraitsAndReplyWithResult(
-      FROM_HERE, kTraits, base::BindOnce(&PrepareAndGetUnzipDir, zip_file),
-      base::BindOnce(&ZipFileInstaller::Unzip, this));
+  content::BrowserThread::PostTask(
+      content::BrowserThread::FILE, FROM_HERE,
+      base::BindOnce(&ZipFileInstaller::PrepareUnzipDir, this, zip_file));
 }
 
 ZipFileInstaller::ZipFileInstaller(ExtensionService* service)
@@ -72,27 +50,46 @@ ZipFileInstaller::ZipFileInstaller(ExtensionService* service)
 
 ZipFileInstaller::~ZipFileInstaller() = default;
 
-void ZipFileInstaller::Unzip(base::Optional<base::FilePath> unzip_dir) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  if (!unzip_dir) {
-    ReportFailure(std::string(kExtensionHandlerTempDirError));
+void ZipFileInstaller::PrepareUnzipDir(const base::FilePath& zip_file) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::FILE);
+
+  base::FilePath dir_temp;
+  base::PathService::Get(base::DIR_TEMP, &dir_temp);
+
+  base::FilePath::StringType dir_name =
+      zip_file.RemoveExtension().BaseName().value() + FILE_PATH_LITERAL("_");
+
+  base::FilePath unzip_dir;
+  if (!base::CreateTemporaryDirInDir(dir_temp, dir_name, &unzip_dir)) {
+    content::BrowserThread::PostTask(
+        content::BrowserThread::UI, FROM_HERE,
+        base::BindOnce(&ZipFileInstaller::ReportFailure, this,
+                       std::string(kExtensionHandlerTempDirError)));
     return;
   }
+
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::BindOnce(&ZipFileInstaller::Unzip, this, unzip_dir));
+}
+
+void ZipFileInstaller::Unzip(const base::FilePath& unzip_dir) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(!utility_process_mojo_client_);
 
   utility_process_mojo_client_ = base::MakeUnique<
       content::UtilityProcessMojoClient<mojom::ExtensionUnpacker>>(
       l10n_util::GetStringUTF16(IDS_UTILITY_PROCESS_ZIP_FILE_INSTALLER_NAME));
   utility_process_mojo_client_->set_error_callback(
-      base::Bind(&ZipFileInstaller::UnzipDone, this, *unzip_dir, false));
+      base::Bind(&ZipFileInstaller::UnzipDone, this, unzip_dir, false));
 
-  utility_process_mojo_client_->set_exposed_directory(*unzip_dir);
+  utility_process_mojo_client_->set_exposed_directory(unzip_dir);
 
   utility_process_mojo_client_->Start();
 
   utility_process_mojo_client_->service()->Unzip(
-      zip_file_, *unzip_dir,
-      base::Bind(&ZipFileInstaller::UnzipDone, this, *unzip_dir));
+      zip_file_, unzip_dir,
+      base::Bind(&ZipFileInstaller::UnzipDone, this, unzip_dir));
 }
 
 void ZipFileInstaller::UnzipDone(const base::FilePath& unzip_dir,

chrome/browser/extensions/zipfile_installer.h

@@ -40,7 +40,8 @@ class ZipFileInstaller : public base::RefCountedThreadSafe<ZipFileInstaller> {
   ~ZipFileInstaller();
 
   // Unzip an extension into |unzip_dir| and load it with an UnpackedInstaller.
-  void Unzip(base::Optional<base::FilePath> unzip_dir);
+  void PrepareUnzipDir(const base::FilePath& zip_file);
+  void Unzip(const base::FilePath& unzip_dir);
   void UnzipDone(const base::FilePath& unzip_dir, bool success);
 
   // On failure, report the |error| reason.