Payments: Prepare for service worker core on the UI thread.

The thread ServiceWorkerContextCore lives on (the "core thread") will
move from the IO thread to the UI thread.

This CL makes PaymentAppBrowserTest tests pass when run with
ServiceWorkerOnUI enabled. It is fairly mechanical. One tricky
part is whether the callbacks here need to be called asynchronously.
In some cases we may already be on the thread the callback needs
to run on, the CL still uses PostTask() to run the callback in case
sync callbacks are not ok.

Bug: 824858
Change-Id: I195d4ce93d267d9417a1e671d64b3d6ae4a8acc2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1763551
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#689181}

content/browser/payments/payment_app_context_impl.cc

@@ -22,79 +22,81 @@ void PaymentAppContextImpl::Init(
     scoped_refptr<ServiceWorkerContextWrapper> service_worker_context) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 #if DCHECK_IS_ON()
-  DCHECK(!did_shutdown_on_io_.IsSet());
+  DCHECK(!did_shutdown_on_core_.IsSet());
 #endif
 
-  base::PostTask(
-      FROM_HERE, {BrowserThread::IO},
-      base::BindOnce(&PaymentAppContextImpl::CreatePaymentAppDatabaseOnIO, this,
-                     service_worker_context));
+  RunOrPostTaskOnThread(
+      FROM_HERE, ServiceWorkerContextWrapper::GetCoreThreadId(),
+      base::BindOnce(
+          &PaymentAppContextImpl::CreatePaymentAppDatabaseOnCoreThread, this,
+          service_worker_context));
 }
 
 void PaymentAppContextImpl::Shutdown() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  // Schedule a ShutdownOnIO() callback that holds a reference to |this| on the
-  // IO thread. When the last reference to |this| is released, |this| is
-  // automatically scheduled for deletion on the UI thread (see
+  // Schedule a ShutdownOnCoreThread() callback that holds a reference to |this|
+  // on the core thread. When the last reference to |this| is released, |this|
+  // is automatically scheduled for deletion on the UI thread (see
   // content::BrowserThread::DeleteOnUIThread in the header file).
-  base::PostTask(FROM_HERE, {BrowserThread::IO},
-                 base::BindOnce(&PaymentAppContextImpl::ShutdownOnIO, this));
+  RunOrPostTaskOnThread(
+      FROM_HERE, ServiceWorkerContextWrapper::GetCoreThreadId(),
+      base::BindOnce(&PaymentAppContextImpl::ShutdownOnCoreThread, this));
 }
 
 void PaymentAppContextImpl::CreatePaymentManager(
     payments::mojom::PaymentManagerRequest request) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  base::PostTask(
-      FROM_HERE, {BrowserThread::IO},
-      base::BindOnce(&PaymentAppContextImpl::CreatePaymentManagerOnIO, this,
-                     std::move(request)));
+  RunOrPostTaskOnThread(
+      FROM_HERE, ServiceWorkerContextWrapper::GetCoreThreadId(),
+      base::BindOnce(&PaymentAppContextImpl::CreatePaymentManagerOnCoreThread,
+                     this, std::move(request)));
 }
 
 void PaymentAppContextImpl::PaymentManagerHadConnectionError(
     PaymentManager* payment_manager) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   DCHECK(base::Contains(payment_managers_, payment_manager));
 
   payment_managers_.erase(payment_manager);
 }
 
 PaymentAppDatabase* PaymentAppContextImpl::payment_app_database() const {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   return payment_app_database_.get();
 }
 
 PaymentAppContextImpl::~PaymentAppContextImpl() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 #if DCHECK_IS_ON()
-  DCHECK(did_shutdown_on_io_.IsSet());
+  DCHECK(did_shutdown_on_core_.IsSet());
 #endif
 }
 
-void PaymentAppContextImpl::CreatePaymentAppDatabaseOnIO(
+void PaymentAppContextImpl::CreatePaymentAppDatabaseOnCoreThread(
     scoped_refptr<ServiceWorkerContextWrapper> service_worker_context) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   payment_app_database_ =
       std::make_unique<PaymentAppDatabase>(service_worker_context);
 }
 
-void PaymentAppContextImpl::CreatePaymentManagerOnIO(
+void PaymentAppContextImpl::CreatePaymentManagerOnCoreThread(
     mojo::InterfaceRequest<payments::mojom::PaymentManager> request) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   auto payment_manager =
       std::make_unique<PaymentManager>(this, std::move(request));
   payment_managers_[payment_manager.get()] = std::move(payment_manager);
 }
 
-void PaymentAppContextImpl::ShutdownOnIO() {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+void PaymentAppContextImpl::ShutdownOnCoreThread() {
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
 
   payment_managers_.clear();
   payment_app_database_.reset();
 
 #if DCHECK_IS_ON()
-  did_shutdown_on_io_.Set();
+  did_shutdown_on_core_.Set();
 #endif
 }
 

content/browser/payments/payment_app_context_impl.h

@@ -25,11 +25,13 @@ class ServiceWorkerContextWrapper;
 
 // One instance of this exists per StoragePartition, and services multiple child
 // processes/origins. Most logic is delegated to the owned PaymentAppDatabase
-// instance, which is only accessed on the IO thread.
+// instance, which is only accessed on the thread identified by
+// ServiceWorkerContextWrapper::GetCoreThreadId() (the service worker "core"
+// thread).
 //
 // This class is created/destructed by StoragePartitionImpl on UI thread.
 // However, the PaymentAppDatabase that this class has internally should work on
-// IO thread. So, this class has Init() and Shutdown() methods in addition to
+// core thread. So, this class has Init() and Shutdown() methods in addition to
 // constructor and destructor. They should be called explicitly when creating
 // and destroying StoragePartitionImpl.
 //
@@ -39,8 +41,8 @@ class ServiceWorkerContextWrapper;
 //   3) Can now call other public methods in this class in any order.
 //     - Can call CreatePaymentManager() on UI thread.
 //     - Can call GetAllPaymentApps() on UI thread.
-//     - Can call PaymentManagerHadConnectionError() on IO thread.
-//     - Can call payment_app_database() on IO thread.
+//     - Can call PaymentManagerHadConnectionError() on core thread.
+//     - Can call payment_app_database() on the core thread.
 //   4) Shutdown()
 //   5) Destructor
 class CONTENT_EXPORT PaymentAppContextImpl
@@ -62,10 +64,10 @@ class CONTENT_EXPORT PaymentAppContextImpl
   void CreatePaymentManager(payments::mojom::PaymentManagerRequest request);
 
   // Called by PaymentManager objects so that they can
-  // be deleted. Call on the IO thread.
+  // be deleted. Call on the core thread.
   void PaymentManagerHadConnectionError(PaymentManager* service);
 
-  // Should be accessed only on the IO thread.
+  // Should be accessed only on the core thread.
   PaymentAppDatabase* payment_app_database() const;
 
  private:
@@ -75,26 +77,26 @@ class CONTENT_EXPORT PaymentAppContextImpl
   friend class base::DeleteHelper<PaymentAppContextImpl>;
   ~PaymentAppContextImpl();
 
-  void CreatePaymentAppDatabaseOnIO(
+  void CreatePaymentAppDatabaseOnCoreThread(
       scoped_refptr<ServiceWorkerContextWrapper> service_worker_context);
 
-  void CreatePaymentManagerOnIO(
+  void CreatePaymentManagerOnCoreThread(
       mojo::InterfaceRequest<payments::mojom::PaymentManager> request);
 
-  void ShutdownOnIO();
+  void ShutdownOnCoreThread();
 
-  // Only accessed on the IO thread.
+  // Only accessed on the core thread.
   std::unique_ptr<PaymentAppDatabase> payment_app_database_;
 
   // The PaymentManagers are owned by this. They're either deleted during
-  // ShutdownOnIO or when the channel is closed via
-  // PaymentManagerHadConnectionError. Only accessed on the IO thread.
+  // ShutdownOnCoreThread or when the channel is closed via
+  // PaymentManagerHadConnectionError. Only accessed on the core thread.
   std::map<PaymentManager*, std::unique_ptr<PaymentManager>> payment_managers_;
 
 #if DCHECK_IS_ON()
-  // Set after ShutdownOnIO() has run on the IO thread. |this| shouldn't be
-  // deleted before this is set.
-  base::AtomicFlag did_shutdown_on_io_;
+  // Set after ShutdownOnCoreThread() has run on the core thread. |this|
+  // shouldn't be deleted before this is set.
+  base::AtomicFlag did_shutdown_on_core_;
 #endif
 
   DISALLOW_COPY_AND_ASSIGN(PaymentAppContextImpl);

content/browser/payments/payment_app_info_fetcher.cc

@@ -33,13 +33,13 @@ void PaymentAppInfoFetcher::Start(
     const GURL& context_url,
     scoped_refptr<ServiceWorkerContextWrapper> service_worker_context,
     PaymentAppInfoFetchCallback callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
 
   std::unique_ptr<std::vector<GlobalFrameRoutingId>> provider_hosts =
       service_worker_context->GetProviderHostIds(context_url.GetOrigin());
 
-  base::PostTask(
-      FROM_HERE, {BrowserThread::UI},
+  RunOrPostTaskOnThread(
+      FROM_HERE, BrowserThread::UI,
       base::BindOnce(&PaymentAppInfoFetcher::StartOnUI, context_url,
                      std::move(provider_hosts), std::move(callback)));
 }
@@ -166,7 +166,7 @@ void PaymentAppInfoFetcher::SelfDeleteFetcher::Start(
 void PaymentAppInfoFetcher::SelfDeleteFetcher::RunCallbackAndDestroy() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  base::PostTask(FROM_HERE, {BrowserThread::IO},
+  base::PostTask(FROM_HERE, {ServiceWorkerContextWrapper::GetCoreThreadId()},
                  base::BindOnce(std::move(callback_),
                                 std::move(fetched_payment_app_info_)));
   delete this;

content/browser/payments/payment_app_info_fetcher.h

@@ -32,7 +32,7 @@ class PaymentAppInfoFetcher {
   using PaymentAppInfoFetchCallback =
       base::OnceCallback<void(std::unique_ptr<PaymentAppInfo> app_info)>;
 
-  // Only accessed on the IO thread.
+  // Only accessed on the service worker core thread.
   static void Start(
       const GURL& context_url,
       scoped_refptr<ServiceWorkerContextWrapper> service_worker_context,

content/browser/payments/payment_app_installer.cc

@@ -138,21 +138,21 @@ class SelfDeleteInstaller
     scoped_refptr<PaymentAppContextImpl> payment_app_context =
         partition->GetPaymentAppContext();
 
-    base::PostTask(
-        FROM_HERE, {BrowserThread::IO},
-        base::BindOnce(&SelfDeleteInstaller::SetPaymentAppInfoOnIO, this,
-                       payment_app_context, registration_id_, scope_.spec(),
-                       app_name_, app_icon_, method_));
+    RunOrPostTaskOnThread(
+        FROM_HERE, ServiceWorkerContextWrapper::GetCoreThreadId(),
+        base::BindOnce(&SelfDeleteInstaller::SetPaymentAppInfoOnCoreThread,
+                       this, payment_app_context, registration_id_,
+                       scope_.spec(), app_name_, app_icon_, method_));
   }
 
-  void SetPaymentAppInfoOnIO(
+  void SetPaymentAppInfoOnCoreThread(
       scoped_refptr<PaymentAppContextImpl> payment_app_context,
       int64_t registration_id,
       const std::string& instrument_key,
       const std::string& name,
       const std::string& app_icon,
       const std::string& method) {
-    DCHECK_CURRENTLY_ON(BrowserThread::IO);
+    DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
 
     payment_app_context->payment_app_database()
         ->SetPaymentAppInfoForRegisteredServiceWorker(
@@ -161,10 +161,10 @@ class SelfDeleteInstaller
   }
 
   void OnSetPaymentAppInfo(payments::mojom::PaymentHandlerStatus status) {
-    DCHECK_CURRENTLY_ON(BrowserThread::IO);
+    DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
 
-    base::PostTask(
-        FROM_HERE, {BrowserThread::UI},
+    RunOrPostTaskOnThread(
+        FROM_HERE, BrowserThread::UI,
         base::BindOnce(&SelfDeleteInstaller::FinishInstallation, this,
                        status == payments::mojom::PaymentHandlerStatus::SUCCESS
                            ? true

content/browser/payments/payment_instrument_icon_fetcher.cc

@@ -39,7 +39,8 @@ void OnIconFetched(
 
   if (bitmap.drawsNothing()) {
     if (icons.empty()) {
-      base::PostTask(FROM_HERE, {BrowserThread::IO},
+      base::PostTask(FROM_HERE,
+                     {ServiceWorkerContextWrapper::GetCoreThreadId()},
                      base::BindOnce(std::move(callback), std::string()));
     } else {
       // If could not download or decode the chosen image(e.g. not supported,
@@ -57,7 +58,7 @@ void OnIconFetched(
       base::StringPiece(reinterpret_cast<const char*>(&bitmap_data[0]),
                         bitmap_data.size()),
       &encoded_data);
-  base::PostTask(FROM_HERE, {BrowserThread::IO},
+  base::PostTask(FROM_HERE, {ServiceWorkerContextWrapper::GetCoreThreadId()},
                  base::BindOnce(std::move(callback), encoded_data));
 }
 
@@ -69,7 +70,7 @@ void DownloadBestMatchingIcon(
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   if (web_contents == nullptr) {
-    base::PostTask(FROM_HERE, {BrowserThread::IO},
+    base::PostTask(FROM_HERE, {ServiceWorkerContextWrapper::GetCoreThreadId()},
                    base::BindOnce(std::move(callback), std::string()));
     return;
   }
@@ -85,7 +86,7 @@ void DownloadBestMatchingIcon(
     // developers in advance unlike when fetching or decoding fails. We already
     // checked whether they are valid in renderer side. So, if the icon url is
     // invalid, it's something wrong.
-    base::PostTask(FROM_HERE, {BrowserThread::IO},
+    base::PostTask(FROM_HERE, {ServiceWorkerContextWrapper::GetCoreThreadId()},
                    base::BindOnce(std::move(callback), std::string()));
     return;
   }
@@ -151,11 +152,12 @@ void PaymentInstrumentIconFetcher::Start(
     std::unique_ptr<std::vector<GlobalFrameRoutingId>> provider_hosts,
     const std::vector<blink::Manifest::ImageResource>& icons,
     PaymentInstrumentIconFetcherCallback callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
 
-  base::PostTask(FROM_HERE, {BrowserThread::UI},
-                 base::BindOnce(&StartOnUI, scope, std::move(provider_hosts),
-                                icons, std::move(callback)));
+  RunOrPostTaskOnThread(
+      FROM_HERE, BrowserThread::UI,
+      base::BindOnce(&StartOnUI, scope, std::move(provider_hosts), icons,
+                     std::move(callback)));
 }
 
 }  // namespace content

content/browser/payments/payment_instrument_icon_fetcher.h

@@ -23,7 +23,7 @@ class PaymentInstrumentIconFetcher {
   using PaymentInstrumentIconFetcherCallback =
       base::OnceCallback<void(const std::string&)>;
 
-  // Should be called on IO thread.
+  // Should be called on the service worker core thread.
   static void Start(
       const GURL& scope,
       std::unique_ptr<std::vector<GlobalFrameRoutingId>> provider_hosts,

content/browser/payments/payment_manager.cc

@@ -19,7 +19,7 @@
 namespace content {
 
 PaymentManager::~PaymentManager() {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
 }
 
 PaymentManager::PaymentManager(
@@ -27,7 +27,7 @@ PaymentManager::PaymentManager(
     mojo::InterfaceRequest<payments::mojom::PaymentManager> request)
     : payment_app_context_(payment_app_context),
       binding_(this, std::move(request)) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   DCHECK(payment_app_context);
 
   binding_.set_connection_error_handler(base::BindOnce(
@@ -35,7 +35,7 @@ PaymentManager::PaymentManager(
 }
 
 void PaymentManager::Init(const GURL& context_url, const std::string& scope) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
 
   should_set_payment_app_info_ = true;
   context_url_ = context_url;
@@ -59,7 +59,7 @@ void PaymentManager::Init(const GURL& context_url, const std::string& scope) {
 void PaymentManager::DeletePaymentInstrument(
     const std::string& instrument_key,
     PaymentManager::DeletePaymentInstrumentCallback callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
 
   payment_app_context_->payment_app_database()->DeletePaymentInstrument(
       scope_, instrument_key, std::move(callback));
@@ -68,7 +68,7 @@ void PaymentManager::DeletePaymentInstrument(
 void PaymentManager::GetPaymentInstrument(
     const std::string& instrument_key,
     PaymentManager::GetPaymentInstrumentCallback callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
 
   payment_app_context_->payment_app_database()->ReadPaymentInstrument(
       scope_, instrument_key, std::move(callback));
@@ -76,7 +76,7 @@ void PaymentManager::GetPaymentInstrument(
 
 void PaymentManager::KeysOfPaymentInstruments(
     PaymentManager::KeysOfPaymentInstrumentsCallback callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
 
   payment_app_context_->payment_app_database()->KeysOfPaymentInstruments(
       scope_, std::move(callback));
@@ -85,7 +85,7 @@ void PaymentManager::KeysOfPaymentInstruments(
 void PaymentManager::HasPaymentInstrument(
     const std::string& instrument_key,
     PaymentManager::HasPaymentInstrumentCallback callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
 
   payment_app_context_->payment_app_database()->HasPaymentInstrument(
       scope_, instrument_key, std::move(callback));
@@ -95,7 +95,7 @@ void PaymentManager::SetPaymentInstrument(
     const std::string& instrument_key,
     payments::mojom::PaymentInstrumentPtr details,
     PaymentManager::SetPaymentInstrumentCallback callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
 
   if (should_set_payment_app_info_) {
     payment_app_context_->payment_app_database()->WritePaymentInstrument(
@@ -112,7 +112,7 @@ void PaymentManager::SetPaymentInstrument(
 void PaymentManager::SetPaymentInstrumentIntermediateCallback(
     PaymentManager::SetPaymentInstrumentCallback callback,
     payments::mojom::PaymentHandlerStatus status) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
 
   if (status != payments::mojom::PaymentHandlerStatus::SUCCESS ||
       !should_set_payment_app_info_) {
@@ -127,7 +127,7 @@ void PaymentManager::SetPaymentInstrumentIntermediateCallback(
 
 void PaymentManager::ClearPaymentInstruments(
     ClearPaymentInstrumentsCallback callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
 
   payment_app_context_->payment_app_database()->ClearPaymentInstruments(
       scope_, std::move(callback));
@@ -139,7 +139,7 @@ void PaymentManager::SetUserHint(const std::string& user_hint) {
 }
 
 void PaymentManager::OnConnectionError() {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(ServiceWorkerContextWrapper::GetCoreThreadId());
   payment_app_context_->PaymentManagerHadConnectionError(this);
 }