Remove usage of SequencedWorkerPool::GetNamedSequenceToken from kiosk_app_data.cc.

chrome/browser/chromeos/app_mode/kiosk_app_data.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/app_mode/kiosk_app_data.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/files/file_util.h"
 #include "base/json/json_writer.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted_memory.h"
 #include "base/task_scheduler/post_task.h"
-#include "base/threading/sequenced_worker_pool.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/app_mode/kiosk_app_data_delegate.h"
 #include "chrome/browser/chromeos/app_mode/kiosk_app_manager.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/extensions/webstore_data_fetcher.h"
 #include "chrome/browser/extensions/webstore_install_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "components/prefs/pref_service.h"
@@ skipping 45 matching lines @@
 ////////////////////////////////////////////////////////////////////////////////
 // KioskAppData::CrxLoader
 // Loads meta data from crx file.
 
 class KioskAppData::CrxLoader : public extensions::SandboxedUnpackerClient {
  public:
   CrxLoader(const base::WeakPtr<KioskAppData>& client,
             const base::FilePath& crx_file)
       : client_(client),
         crx_file_(crx_file),
-        success_(false) {
-  }
+        success_(false),
+        task_runner_(base::CreateSequencedTaskRunnerWithTraits(
+            {base::MayBlock(), base::TaskPriority::BACKGROUND,
+             base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN})) {}
 
   void Start() {
-    base::SequencedWorkerPool* pool = BrowserThread::GetBlockingPool();
-    base::SequencedWorkerPool::SequenceToken token =
-        pool->GetNamedSequenceToken("KioskAppData.CrxLoaderWorker");

[fdoray, 2017/05/16 17:58:56]

Before my CL, the named sequence token provided mutual exclusion between tasks
posted from different KioskAppData::CrxLoader instances.

With my CL, mutual exclusion is only between tasks posted from the same
KioskAppData::CrxLoader instance.

Is that ok?

[xiyuan, 2017/05/16 19:41:45]

I would like to keep the existing behavior. Crx unpacking is heavy. If a kiosk
device gets a few kiosk apps pushed down via policy, we could be running crx
unpacking in parallel, which slows down the device.

We have a KioskAppManager::GetBackgroundTaskRunner, which always creates a new
task runner ATM. It is currently used in two places (for ExternalCache and USB
update monitoring). We probably can change it to only create one task runner and
use it everywhere (including for CrxLoader).

[fdoray, 2017/05/19 19:06:52]

The TaskScheduler is already limiting the rate at which
base::TaskPriority::BACKGROUND tasks run. We would prefer if tasks ran on the
same sequence only when there is a data dependency between them. Sharing a
sequence to limit load prevents the TaskScheduler from being smart (e.g. run
more BACKGROUND tasks when the browser is idle).

What do you think?

-    task_runner_ = pool->GetSequencedTaskRunnerWithShutdownBehavior(
-        token,
-        base::SequencedWorkerPool::SKIP_ON_SHUTDOWN);
     task_runner_->PostTask(FROM_HERE,
-                           base::Bind(&CrxLoader::StartOnBlockingPool, this));
+                           base::BindOnce(&CrxLoader::StartInThreadPool, this));
   }
 
   bool success() const { return success_; }
   const base::FilePath& crx_file() const { return crx_file_; }
   const std::string& name() const { return name_; }
   const SkBitmap& icon() const { return icon_; }
   const std::string& required_platform_version() const {
     return required_platform_version_;
   }
 
@@ skipping 12 matching lines @@
         extensions::KioskModeInfo::Get(extension);
     if (info == nullptr) {
       LOG(ERROR) << extension->id() << " is not a valid kiosk app.";
       success_ = false;
     } else {
       success_ = true;
       name_ = extension->name();
       icon_ = install_icon;
       required_platform_version_ = info->required_platform_version;
     }
-    NotifyFinishedOnBlockingPool();
+    NotifyFinishedInThreadPool();
   }
   void OnUnpackFailure(const extensions::CrxInstallError& error) override {
     DCHECK(task_runner_->RunsTasksOnCurrentThread());
 
     success_ = false;
-    NotifyFinishedOnBlockingPool();
+    NotifyFinishedInThreadPool();
   }
 
-  void StartOnBlockingPool() {
+  void StartInThreadPool() {
     DCHECK(task_runner_->RunsTasksOnCurrentThread());
 
     if (!temp_dir_.CreateUniqueTempDir()) {
       success_ = false;
-      NotifyFinishedOnBlockingPool();
+      NotifyFinishedInThreadPool();
       return;
     }
 
     scoped_refptr<extensions::SandboxedUnpacker> unpacker(
         new extensions::SandboxedUnpacker(
             extensions::Manifest::INTERNAL, extensions::Extension::NO_FLAGS,
             temp_dir_.GetPath(), task_runner_.get(), this));
     unpacker->StartWithCrx(extensions::CRXFileInfo(crx_file_));
   }
 
-  void NotifyFinishedOnBlockingPool() {
+  void NotifyFinishedInThreadPool() {
     DCHECK(task_runner_->RunsTasksOnCurrentThread());
 
     if (!temp_dir_.Delete()) {
       LOG(WARNING) << "Can not delete temp directory at "
                    << temp_dir_.GetPath().value();
     }
 
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
         base::Bind(&CrxLoader::NotifyFinishedOnUIThread, this));
   }
 
   void NotifyFinishedOnUIThread() {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
     if (client_)
       client_->OnCrxLoadFinished(this);
   }
 
   base::WeakPtr<KioskAppData> client_;
   base::FilePath crx_file_;
   bool success_;
 
-  scoped_refptr<base::SequencedTaskRunner> task_runner_;
+  const scoped_refptr<base::SequencedTaskRunner> task_runner_;
   base::ScopedTempDir temp_dir_;
 
   // Extracted meta data.
   std::string name_;
   SkBitmap icon_;
   std::string required_platform_version_;
 
   DISALLOW_COPY_AND_ASSIGN(CrxLoader);
 };
 
@@ skipping 353 matching lines @@
 
   SkBitmap icon = crx_loader->icon();
   if (icon.empty())
     icon = *extensions::util::GetDefaultAppIcon().bitmap();
   SetCache(crx_loader->name(), icon, crx_loader->required_platform_version());
 
   SetStatus(STATUS_LOADED);
 }
 
 }  // namespace chromeos