[ContentIndex] Move components to live entirely on the UI thread.

All classes are now created/destroyed on the UI thread. The Content
Index Database internally deals with posting to/from the IO thread.

Bug: 973844
Change-Id: I05cd9bc20372f8d3c28180fd3f3b2d9402956e8a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1749191
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#688959}

content/browser/content_index/content_index_context_impl.cc

@@ -6,7 +6,6 @@
 
 #include "base/bind.h"
 #include "base/task/post_task.h"
-#include "content/browser/content_index/content_index_metrics.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
@@ -18,8 +17,8 @@ ContentIndexContextImpl::ContentIndexContextImpl(
     BrowserContext* browser_context,
     scoped_refptr<ServiceWorkerContextWrapper> service_worker_context)
     : provider_(browser_context->GetContentIndexProvider()),
-      service_worker_context_(service_worker_context),
-      content_index_database_(browser_context, service_worker_context) {
+      content_index_database_(browser_context,
+                              std::move(service_worker_context)) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 }
 
@@ -28,30 +27,14 @@ void ContentIndexContextImpl::GetIcons(int64_t service_worker_registration_id,
                                        GetIconsCallback callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  base::PostTaskWithTraits(
-      FROM_HERE, {BrowserThread::IO},
-      base::BindOnce(&ContentIndexDatabase::GetIcons,
-                     content_index_database_.GetWeakPtrForIO(),
-                     service_worker_registration_id, description_id,
-                     std::move(callback)));
+  content_index_database_.GetIcons(service_worker_registration_id,
+                                   description_id, std::move(callback));
 }
 
 void ContentIndexContextImpl::GetAllEntries(GetAllEntriesCallback callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  GetAllEntriesCallback wrapped_callback = base::BindOnce(
-      [](GetAllEntriesCallback callback, blink::mojom::ContentIndexError error,
-         std::vector<ContentIndexEntry> entries) {
-        base::PostTask(
-            FROM_HERE, {BrowserThread::UI},
-            base::BindOnce(std::move(callback), error, std::move(entries)));
-      },
-      std::move(callback));
-
-  base::PostTask(FROM_HERE, {BrowserThread::IO},
-                 base::BindOnce(&ContentIndexDatabase::GetAllEntries,
-                                content_index_database_.GetWeakPtrForIO(),
-                                std::move(wrapped_callback)));
+  content_index_database_.GetAllEntries(std::move(callback));
 }
 
 void ContentIndexContextImpl::GetEntry(int64_t service_worker_registration_id,
@@ -59,18 +42,8 @@ void ContentIndexContextImpl::GetEntry(int64_t service_worker_registration_id,
                                        GetEntryCallback callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  GetEntryCallback wrapped_callback = base::BindOnce(
-      [](GetEntryCallback callback, base::Optional<ContentIndexEntry> entry) {
-        base::PostTask(FROM_HERE, {BrowserThread::UI},
-                       base::BindOnce(std::move(callback), std::move(entry)));
-      },
-      std::move(callback));
-
-  base::PostTask(FROM_HERE, {BrowserThread::IO},
-                 base::BindOnce(&ContentIndexDatabase::GetEntry,
-                                content_index_database_.GetWeakPtrForIO(),
-                                service_worker_registration_id, description_id,
-                                std::move(wrapped_callback)));
+  content_index_database_.GetEntry(service_worker_registration_id,
+                                   description_id, std::move(callback));
 }
 
 void ContentIndexContextImpl::OnUserDeletedItem(
@@ -79,85 +52,8 @@ void ContentIndexContextImpl::OnUserDeletedItem(
     const std::string& description_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  base::PostTask(
-      FROM_HERE, {BrowserThread::IO},
-      base::BindOnce(&ContentIndexDatabase::DeleteEntry,
-                     content_index_database_.GetWeakPtrForIO(),
-                     service_worker_registration_id, origin, description_id,
-                     base::BindOnce(&ContentIndexContextImpl::DidDeleteItem,
-                                    this, service_worker_registration_id,
-                                    origin, description_id)));
-}
-
-void ContentIndexContextImpl::DidDeleteItem(
-    int64_t service_worker_registration_id,
-    const url::Origin& origin,
-    const std::string& description_id,
-    blink::mojom::ContentIndexError error) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-  if (error != blink::mojom::ContentIndexError::NONE)
-    return;
-
-  service_worker_context_->FindReadyRegistrationForId(
-      service_worker_registration_id, origin.GetURL(),
-      base::BindOnce(&ContentIndexContextImpl::StartActiveWorkerForDispatch,
-                     this, description_id));
-}
-
-void ContentIndexContextImpl::StartActiveWorkerForDispatch(
-    const std::string& description_id,
-    blink::ServiceWorkerStatusCode service_worker_status,
-    scoped_refptr<ServiceWorkerRegistration> registration) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-  content_index::RecordDisptachStatus("Find", service_worker_status);
-
-  if (service_worker_status != blink::ServiceWorkerStatusCode::kOk)
-    return;
-
-  ServiceWorkerVersion* service_worker_version = registration->active_version();
-  DCHECK(service_worker_version);
-
-  service_worker_version->RunAfterStartWorker(
-      ServiceWorkerMetrics::EventType::CONTENT_DELETE,
-      base::BindOnce(&ContentIndexContextImpl::DeliverMessageToWorker, this,
-                     base::WrapRefCounted(service_worker_version),
-                     std::move(registration), description_id));
-}
-
-void ContentIndexContextImpl::DeliverMessageToWorker(
-    scoped_refptr<ServiceWorkerVersion> service_worker,
-    scoped_refptr<ServiceWorkerRegistration> registration,
-    const std::string& description_id,
-    blink::ServiceWorkerStatusCode service_worker_status) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-  content_index::RecordDisptachStatus("Start", service_worker_status);
-
-  if (service_worker_status != blink::ServiceWorkerStatusCode::kOk)
-    return;
-
-  // Don't allow DB operations while the `contentdelete` event is firing.
-  // This is to prevent re-registering the deleted content within the event.
-  content_index_database_.BlockOrigin(service_worker->script_origin());
-
-  int request_id = service_worker->StartRequest(
-      ServiceWorkerMetrics::EventType::CONTENT_DELETE,
-      base::BindOnce(&ContentIndexContextImpl::DidDispatchEvent, this,
-                     service_worker->script_origin()));
-
-  service_worker->endpoint()->DispatchContentDeleteEvent(
-      description_id, service_worker->CreateSimpleEventCallback(request_id));
-}
-
-void ContentIndexContextImpl::DidDispatchEvent(
-    const url::Origin& origin,
-    blink::ServiceWorkerStatusCode service_worker_status) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-  content_index::RecordDisptachStatus("Dispatch", service_worker_status);
-  content_index_database_.UnblockOrigin(origin);
+  content_index_database_.DeleteItem(service_worker_registration_id, origin,
+                                     description_id);
 }
 
 void ContentIndexContextImpl::GetIconSizes(
@@ -169,8 +65,7 @@ void ContentIndexContextImpl::GetIconSizes(
   if (provider_)
     icon_sizes = provider_->GetIconSizes(category);
 
-  base::PostTask(FROM_HERE, {BrowserThread::IO},
-                 base::BindOnce(std::move(callback), std::move(icon_sizes)));
+  std::move(callback).Run(std::move(icon_sizes));
 }
 
 void ContentIndexContextImpl::Shutdown() {
@@ -181,7 +76,7 @@ void ContentIndexContextImpl::Shutdown() {
 }
 
 ContentIndexDatabase& ContentIndexContextImpl::database() {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
   return content_index_database_;
 }
 

content/browser/content_index/content_index_context_impl.h

@@ -22,8 +22,7 @@ class ServiceWorkerContextWrapper;
 // Content Index database should hold a reference to this.
 class CONTENT_EXPORT ContentIndexContextImpl
     : public ContentIndexContext,
-      public base::RefCountedThreadSafe<ContentIndexContextImpl,
-                                        BrowserThread::DeleteOnIOThread> {
+      public base::RefCountedThreadSafe<ContentIndexContextImpl> {
  public:
   ContentIndexContextImpl(
       BrowserContext* browser_context,
@@ -52,37 +51,11 @@ class CONTENT_EXPORT ContentIndexContextImpl
                          const std::string& description_id) override;
 
  private:
-  friend class base::DeleteHelper<ContentIndexContextImpl>;
-  friend class base::RefCountedThreadSafe<ContentIndexContextImpl,
-                                          BrowserThread::DeleteOnIOThread>;
-  friend struct BrowserThread::DeleteOnThread<BrowserThread::IO>;
+  friend class base::RefCountedThreadSafe<ContentIndexContextImpl>;
 
   ~ContentIndexContextImpl() override;
 
-  // Called after a user-initiated delete.
-  void DidDeleteItem(int64_t service_worker_registration_id,
-                     const url::Origin& origin,
-                     const std::string& description_id,
-                     blink::mojom::ContentIndexError error);
-
-  void StartActiveWorkerForDispatch(
-      const std::string& description_id,
-      blink::ServiceWorkerStatusCode service_worker_status,
-      scoped_refptr<ServiceWorkerRegistration> registration);
-
-  void DeliverMessageToWorker(
-      scoped_refptr<ServiceWorkerVersion> service_worker,
-      scoped_refptr<ServiceWorkerRegistration> registration,
-      const std::string& description_id,
-      blink::ServiceWorkerStatusCode service_worker_status);
-
-  void DidDispatchEvent(const url::Origin& origin,
-                        blink::ServiceWorkerStatusCode service_worker_status);
-
-  // Lives on the UI thread.
   ContentIndexProvider* provider_;
-
-  scoped_refptr<ServiceWorkerContextWrapper> service_worker_context_;
   ContentIndexDatabase content_index_database_;
 
   DISALLOW_COPY_AND_ASSIGN(ContentIndexContextImpl);

content/browser/content_index/content_index_database.h

@@ -67,18 +67,43 @@ class CONTENT_EXPORT ContentIndexDatabase {
                 const std::string& description_id,
                 ContentIndexContext::GetEntryCallback callback);
 
-  // Block/Unblock DB operations for |origin|.
-  void BlockOrigin(const url::Origin& origin);
-  void UnblockOrigin(const url::Origin& origin);
+  // Deletes the entry and dispatches an event.
+  void DeleteItem(int64_t service_worker_registration_id,
+                  const url::Origin& origin,
+                  const std::string& description_id);
 
   // Called when the storage partition is shutting down.
   void Shutdown();
 
-  base::WeakPtr<ContentIndexDatabase> GetWeakPtrForIO() {
-    return weak_ptr_factory_io_.GetWeakPtr();
-  }
-
  private:
+  FRIEND_TEST_ALL_PREFIXES(ContentIndexDatabaseTest,
+                           BlockedOriginsCannotRegisterContent);
+  FRIEND_TEST_ALL_PREFIXES(ContentIndexDatabaseTest, UmaRecorded);
+
+  // public method IO counterparts.
+  void AddEntryOnIO(int64_t service_worker_registration_id,
+                    const url::Origin& origin,
+                    blink::mojom::ContentDescriptionPtr description,
+                    const std::vector<SkBitmap>& icons,
+                    const GURL& launch_url,
+                    blink::mojom::ContentIndexService::AddCallback callback);
+  void DeleteEntryOnIO(
+      int64_t service_worker_registration_id,
+      const url::Origin& origin,
+      const std::string& entry_id,
+      blink::mojom::ContentIndexService::DeleteCallback callback);
+  void GetDescriptionsOnIO(
+      int64_t service_worker_registration_id,
+      blink::mojom::ContentIndexService::GetDescriptionsCallback callback);
+  void GetIconsOnIO(int64_t service_worker_registration_id,
+                    const std::string& description_id,
+                    ContentIndexContext::GetIconsCallback callback);
+  void GetAllEntriesOnIO(ContentIndexContext::GetAllEntriesCallback callback);
+  void GetEntryOnIO(int64_t service_worker_registration_id,
+                    const std::string& description_id,
+                    ContentIndexContext::GetEntryCallback callback);
+
+  // Add Callbacks.
   void DidSerializeIcons(
       int64_t service_worker_registration_id,
       const url::Origin& origin,
@@ -89,40 +114,72 @@ class CONTENT_EXPORT ContentIndexDatabase {
   void DidAddEntry(blink::mojom::ContentIndexService::AddCallback callback,
                    ContentIndexEntry entry,
                    blink::ServiceWorkerStatusCode status);
+
+  // Delete Callbacks.
   void DidDeleteEntry(
       int64_t service_worker_registration_id,
       const url::Origin& origin,
       const std::string& entry_id,
       blink::mojom::ContentIndexService::DeleteCallback callback,
       blink::ServiceWorkerStatusCode status);
+
+  // GetDescriptions Callbacks.
   void DidGetDescriptions(
       blink::mojom::ContentIndexService::GetDescriptionsCallback callback,
       const std::vector<std::string>& data,
       blink::ServiceWorkerStatusCode status);
+
+  // GetIcons Callbacks.
   void DidGetSerializedIcons(ContentIndexContext::GetIconsCallback callback,
                              const std::vector<std::string>& data,
                              blink::ServiceWorkerStatusCode status);
   void DidDeserializeIcons(ContentIndexContext::GetIconsCallback callback,
                            std::unique_ptr<std::vector<SkBitmap>> icons);
+
+  // GetEntries Callbacks.
   void DidGetEntries(
       ContentIndexContext::GetAllEntriesCallback callback,
       const std::vector<std::pair<int64_t, std::string>>& user_data,
       blink::ServiceWorkerStatusCode status);
+
+  // GetEntry Callbacks.
   void DidGetEntry(int64_t service_worker_registration_id,
                    ContentIndexContext::GetEntryCallback callback,
                    const std::vector<std::string>& data,
                    blink::ServiceWorkerStatusCode status);
 
+  // DeleteItem Callbacks.
+  void DidDeleteItem(int64_t service_worker_registration_id,
+                     const url::Origin& origin,
+                     const std::string& description_id,
+                     blink::mojom::ContentIndexError error);
+  void StartActiveWorkerForDispatch(
+      const std::string& description_id,
+      blink::ServiceWorkerStatusCode service_worker_status,
+      scoped_refptr<ServiceWorkerRegistration> registration);
+  void DeliverMessageToWorker(
+      scoped_refptr<ServiceWorkerVersion> service_worker,
+      scoped_refptr<ServiceWorkerRegistration> registration,
+      const std::string& description_id,
+      blink::ServiceWorkerStatusCode service_worker_status);
+  void DidDispatchEvent(const url::Origin& origin,
+                        blink::ServiceWorkerStatusCode service_worker_status);
+
   // Callbacks on the UI thread to notify |provider_| of updates.
   void NotifyProviderContentAdded(std::vector<ContentIndexEntry> entries);
   void NotifyProviderContentDeleted(int64_t service_worker_registration_id,
                                     const url::Origin& origin,
                                     const std::string& entry_id);
 
+  // Block/Unblock DB operations for |origin|.
+  void BlockOrigin(const url::Origin& origin);
+  void UnblockOrigin(const url::Origin& origin);
+
   // Lives on the UI thread.
   ContentIndexProvider* provider_;
 
   // A map from origins to how many times it's been blocked.
+  // Must be used on the IO thread.
   base::flat_map<url::Origin, int> blocked_origins_;
 
   scoped_refptr<ServiceWorkerContextWrapper> service_worker_context_;

content/browser/content_index/content_index_service_impl.cc

@@ -18,21 +18,6 @@
 
 namespace content {
 
-namespace {
-
-void CreateOnIO(
-    mojo::PendingReceiver<blink::mojom::ContentIndexService> receiver,
-    const url::Origin& origin,
-    scoped_refptr<ContentIndexContextImpl> content_index_context) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-  mojo::MakeSelfOwnedReceiver(std::make_unique<ContentIndexServiceImpl>(
-                                  origin, std::move(content_index_context)),
-                              std::move(receiver));
-}
-
-}  // namespace
-
 // static
 void ContentIndexServiceImpl::CreateForRequest(
     blink::mojom::ContentIndexServiceRequest request,
@@ -53,30 +38,28 @@ void ContentIndexServiceImpl::Create(
   auto* storage_partition = static_cast<StoragePartitionImpl*>(
       render_process_host->GetStoragePartition());
 
-  base::PostTask(
-      FROM_HERE, {BrowserThread::IO},
-      base::BindOnce(
-          &CreateOnIO, std::move(receiver), origin,
-          base::WrapRefCounted(storage_partition->GetContentIndexContext())));
+  mojo::MakeSelfOwnedReceiver(
+      std::make_unique<ContentIndexServiceImpl>(
+          origin, storage_partition->GetContentIndexContext()),
+      std::move(receiver));
 }
 
 ContentIndexServiceImpl::ContentIndexServiceImpl(
     const url::Origin& origin,
     scoped_refptr<ContentIndexContextImpl> content_index_context)
     : origin_(origin),
-      content_index_context_(std::move(content_index_context)) {}
+      content_index_context_(std::move(content_index_context)) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+}
 
 ContentIndexServiceImpl::~ContentIndexServiceImpl() = default;
 
 void ContentIndexServiceImpl::GetIconSizes(
     blink::mojom::ContentCategory category,
     GetIconSizesCallback callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  base::PostTaskWithTraits(
-      FROM_HERE, {BrowserThread::UI},
-      base::BindOnce(&ContentIndexContextImpl::GetIconSizes,
-                     content_index_context_, category, std::move(callback)));
+  content_index_context_->GetIconSizes(category, std::move(callback));
 }
 
 void ContentIndexServiceImpl::Add(
@@ -85,7 +68,7 @@ void ContentIndexServiceImpl::Add(
     const std::vector<SkBitmap>& icons,
     const GURL& launch_url,
     AddCallback callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   for (const auto& icon : icons) {
     if (icon.isNull() || icon.width() * icon.height() > kMaxIconResolution) {
@@ -111,7 +94,7 @@ void ContentIndexServiceImpl::Add(
 void ContentIndexServiceImpl::Delete(int64_t service_worker_registration_id,
                                      const std::string& content_id,
                                      DeleteCallback callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   content_index_context_->database().DeleteEntry(
       service_worker_registration_id, origin_, content_id, std::move(callback));
@@ -120,7 +103,7 @@ void ContentIndexServiceImpl::Delete(int64_t service_worker_registration_id,
 void ContentIndexServiceImpl::GetDescriptions(
     int64_t service_worker_registration_id,
     GetDescriptionsCallback callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   content_index_context_->database().GetDescriptions(
       service_worker_registration_id, std::move(callback));