[Extensions] Merge AddPendingTaskToDispatchEvent with AddPendingTask.

This CL integrates previous refactoring changes to have a single add task API for the task queues. Bug: 915814 Change-Id: I1098d0785df50c025bff6c74bc28515d86b88ed8 Reviewed-on: https://chromium-review.googlesource.com/c/1380822 Commit-Queue: David Bertoni <dbertoni@chromium.org> Reviewed-by: Stuart Langley <slangley@chromium.org> Reviewed-by: Ben Wells <benwells@chromium.org> Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org> Cr-Commit-Position: refs/heads/master@{#617843}
marcosholgado · Dec 19, 2018 · 8269a09 · 8269a09
1 parent 1f8c821
commit 8269a09
Show file tree

Hide file tree

Showing 15 changed files with 45 additions and 54 deletions.
diff --git a/apps/launcher.cc b/apps/launcher.cc
@@ -35,6 +35,7 @@
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/granted_file_entry.h"
 #include "extensions/browser/lazy_background_task_queue.h"
+#include "extensions/browser/lazy_context_id.h"
 #include "extensions/browser/process_manager.h"
 #include "extensions/common/api/app_runtime.h"
 #include "extensions/common/extension.h"
@@ -293,7 +294,7 @@ class PlatformAppPathLauncher
         extensions::LazyBackgroundTaskQueue::Get(context_);
     if (queue->ShouldEnqueueTask(context_, app)) {
       queue->AddPendingTask(
-          context_, extension_id,
+          extensions::LazyContextId(context_, extension_id),
           base::Bind(&PlatformAppPathLauncher::GrantAccessToFilesAndLaunch,
                      this));
       return;

diff --git a/chrome/browser/chromeos/file_manager/file_browser_handlers.cc b/chrome/browser/chromeos/file_manager/file_browser_handlers.cc
@@ -38,6 +38,7 @@
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_util.h"
 #include "extensions/browser/lazy_background_task_queue.h"
+#include "extensions/browser/lazy_context_id.h"
 #include "extensions/common/extension_set.h"
 #include "extensions/common/manifest_handlers/background_info.h"
 #include "extensions/common/url_pattern.h"
@@ -356,7 +357,7 @@ void FileBrowserHandlerExecutor::ExecuteFileActionsOnUIThread(
       return;
     }
     queue->AddPendingTask(
-        profile_, extension_->id(),
+        extensions::LazyContextId(profile_, extension_->id()),
         base::BindOnce(
             &FileBrowserHandlerExecutor::SetupPermissionsAndDispatchEvent,
             weak_ptr_factory_.GetWeakPtr(),

diff --git a/chrome/browser/extensions/devtools_util.cc b/chrome/browser/extensions/devtools_util.cc
@@ -8,6 +8,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "extensions/browser/extension_host.h"
 #include "extensions/browser/lazy_background_task_queue.h"
+#include "extensions/browser/lazy_context_id.h"
 #include "extensions/browser/process_manager.h"
 #include "extensions/common/extension.h"
 
@@ -34,7 +35,8 @@ void InspectBackgroundPage(const Extension* extension, Profile* profile) {
         std::make_unique<LazyContextTaskQueue::ContextInfo>(host));
   } else {
     LazyBackgroundTaskQueue::Get(profile)->AddPendingTask(
-        profile, extension->id(), base::BindOnce(&InspectExtensionHost));
+        LazyContextId(profile, extension->id()),
+        base::BindOnce(&InspectExtensionHost));
   }
 }
 

diff --git a/extensions/browser/api/messaging/message_service.cc b/extensions/browser/api/messaging/message_service.cc
@@ -39,6 +39,7 @@
 #include "extensions/browser/extensions_browser_client.h"
 #include "extensions/browser/guest_view/web_view/web_view_guest.h"
 #include "extensions/browser/lazy_background_task_queue.h"
+#include "extensions/browser/lazy_context_id.h"
 #include "extensions/browser/process_manager.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/manifest_constants.h"
@@ -638,7 +639,7 @@ void MessageService::ClosePortImpl(const PortId& port_id,
     auto pending = pending_lazy_background_page_channels_.find(channel_id);
     if (pending != pending_lazy_background_page_channels_.end()) {
       lazy_background_task_queue_->AddPendingTask(
-          pending->second.first, pending->second.second,
+          LazyContextId(pending->second.first, pending->second.second),
           base::BindOnce(&MessageService::PendingLazyBackgroundPageClosePort,
                          weak_factory_.GetWeakPtr(), port_id, process_id,
                          routing_id, force_close, error_message));
@@ -738,7 +739,7 @@ void MessageService::EnqueuePendingMessageForLazyBackgroundLoad(
   auto pending = pending_lazy_background_page_channels_.find(channel_id);
   if (pending != pending_lazy_background_page_channels_.end()) {
     lazy_background_task_queue_->AddPendingTask(
-        pending->second.first, pending->second.second,
+        LazyContextId(pending->second.first, pending->second.second),
         base::BindOnce(&MessageService::PendingLazyBackgroundPagePostMessage,
                        weak_factory_.GetWeakPtr(), source_port_id, message));
   }
@@ -780,7 +781,7 @@ bool MessageService::MaybeAddPendingLazyBackgroundPageOpenChannelTask(
       PendingLazyBackgroundPageChannel(context, extension->id());
   int source_id = (*params)->source_process_id;
   lazy_background_task_queue_->AddPendingTask(
-      context, extension->id(),
+      LazyContextId(context, extension->id()),
       base::BindOnce(&MessageService::PendingLazyBackgroundPageOpenChannel,
                      weak_factory_.GetWeakPtr(), base::Passed(params),
                      source_id));

diff --git a/extensions/browser/api/runtime/runtime_api.cc b/extensions/browser/api/runtime/runtime_api.cc
@@ -32,6 +32,7 @@
 #include "extensions/browser/extension_util.h"
 #include "extensions/browser/extensions_browser_client.h"
 #include "extensions/browser/lazy_background_task_queue.h"
+#include "extensions/browser/lazy_context_id.h"
 #include "extensions/browser/process_manager_factory.h"
 #include "extensions/common/api/runtime.h"
 #include "extensions/common/error_utils.h"
@@ -130,7 +131,7 @@ void DispatchOnStartupEventImpl(
       LazyBackgroundTaskQueue::Get(browser_context)
           ->ShouldEnqueueTask(browser_context, extension)) {
     LazyBackgroundTaskQueue::Get(browser_context)
-        ->AddPendingTask(browser_context, extension_id,
+        ->AddPendingTask(LazyContextId(browser_context, extension_id),
                          base::BindOnce(&DispatchOnStartupEventImpl,
                                         browser_context, extension_id, false));
     return;
@@ -599,7 +600,7 @@ ExtensionFunction::ResponseAction RuntimeGetBackgroundPageFunction::Run() {
           ->ShouldEnqueueTask(browser_context(), extension())) {
     LazyBackgroundTaskQueue::Get(browser_context())
         ->AddPendingTask(
-            browser_context(), extension_id(),
+            LazyContextId(browser_context(), extension_id()),
             base::BindOnce(&RuntimeGetBackgroundPageFunction::OnPageLoaded,
                            this));
   } else if (host) {

diff --git a/extensions/browser/events/lazy_event_dispatcher.cc b/extensions/browser/events/lazy_event_dispatcher.cc
@@ -118,8 +118,8 @@ bool LazyEventDispatcher::QueueEventDispatch(
     dispatched_event->will_dispatch_callback.Reset();
   }
 
-  queue->AddPendingTaskToDispatchEvent(
-      dispatch_context, base::BindOnce(dispatch_function_, dispatched_event));
+  queue->AddPendingTask(dispatch_context,
+                        base::BindOnce(dispatch_function_, dispatched_event));
 
   return true;
 }

diff --git a/extensions/browser/extension_registrar.cc b/extensions/browser/extension_registrar.cc
@@ -17,6 +17,7 @@
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/lazy_background_task_queue.h"
+#include "extensions/browser/lazy_context_id.h"
 #include "extensions/browser/notification_types.h"
 #include "extensions/browser/process_manager.h"
 #include "extensions/browser/renderer_startup_helper.h"
@@ -522,7 +523,8 @@ void ExtensionRegistrar::MaybeSpinUpLazyBackgroundPage(
   // Wake up the event page by posting a dummy task.
   LazyBackgroundTaskQueue* queue =
       LazyBackgroundTaskQueue::Get(browser_context_);
-  queue->AddPendingTask(browser_context_, extension->id(), base::DoNothing());
+  queue->AddPendingTask(LazyContextId(browser_context_, extension->id()),
+                        base::DoNothing());
 }
 
 }  // namespace extensions
diff --git a/extensions/browser/guest_view/app_view/app_view_guest.cc b/extensions/browser/guest_view/app_view/app_view_guest.cc
@@ -20,6 +20,7 @@
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/guest_view/app_view/app_view_constants.h"
 #include "extensions/browser/lazy_background_task_queue.h"
+#include "extensions/browser/lazy_context_id.h"
 #include "extensions/browser/process_manager.h"
 #include "extensions/browser/view_type_utils.h"
 #include "extensions/common/api/app_runtime.h"
@@ -197,7 +198,7 @@ void AppViewGuest::CreateWebContents(const base::DictionaryValue& create_params,
       LazyBackgroundTaskQueue::Get(browser_context());
   if (queue->ShouldEnqueueTask(browser_context(), guest_extension)) {
     queue->AddPendingTask(
-        browser_context(), guest_extension->id(),
+        LazyContextId(browser_context(), guest_extension->id()),
         base::BindOnce(&AppViewGuest::LaunchAppAndFireEvent,
                        weak_ptr_factory_.GetWeakPtr(), data->CreateDeepCopy(),
                        std::move(callback)));

diff --git a/extensions/browser/lazy_background_task_queue.cc b/extensions/browser/lazy_background_task_queue.cc
@@ -82,21 +82,14 @@ bool LazyBackgroundTaskQueue::ShouldEnqueueTask(
   return false;
 }
 
-void LazyBackgroundTaskQueue::AddPendingTaskToDispatchEvent(
-    const LazyContextId& context_id,
-    LazyContextTaskQueue::PendingTask task) {
-  AddPendingTask(context_id.browser_context(), context_id.extension_id(),
-                 std::move(task));
-}
-
-void LazyBackgroundTaskQueue::AddPendingTask(
-    content::BrowserContext* browser_context,
-    const std::string& extension_id,
-    PendingTask task) {
+void LazyBackgroundTaskQueue::AddPendingTask(const LazyContextId& context_id,
+                                             PendingTask task) {
   if (ExtensionsBrowserClient::Get()->IsShuttingDown()) {
     std::move(task).Run(nullptr);
     return;
   }
+  const ExtensionId& extension_id = context_id.extension_id();
+  content::BrowserContext* const browser_context = context_id.browser_context();
   PendingTasksList* tasks_list = nullptr;
   PendingTasksKey key(browser_context, extension_id);
   auto it = pending_tasks_.find(key);

diff --git a/extensions/browser/lazy_background_task_queue.h b/extensions/browser/lazy_background_task_queue.h
@@ -56,19 +56,14 @@ class LazyBackgroundTaskQueue : public KeyedService,
   bool ShouldEnqueueTask(content::BrowserContext* context,
                          const Extension* extension) override;
 
-  // TODO(lazyboy): Find a better way to use AddPendingTask instead of this.
-  void AddPendingTaskToDispatchEvent(const LazyContextId& context_id,
-                                     PendingTask task) override;
-
   // Adds a task to the queue for a given extension. If this is the first
   // task added for the extension, its lazy background page will be loaded.
   // The task will be called either when the page is loaded, or when the
   // page fails to load for some reason (e.g. a crash or browser
   // shutdown). In the latter case, |task| will be called with an empty
   // std::unique_ptr<ContextItem> parameter.
-  void AddPendingTask(content::BrowserContext* context,
-                      const std::string& extension_id,
-                      PendingTask task);
+  void AddPendingTask(const LazyContextId& context_id,
+                      PendingTask task) override;
 
  private:
   FRIEND_TEST_ALL_PREFIXES(LazyBackgroundTaskQueueTest, AddPendingTask);

diff --git a/extensions/browser/lazy_background_task_queue_unittest.cc b/extensions/browser/lazy_background_task_queue_unittest.cc
@@ -16,6 +16,7 @@
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_registry_factory.h"
 #include "extensions/browser/extensions_test.h"
+#include "extensions/browser/lazy_context_id.h"
 #include "extensions/browser/process_manager.h"
 #include "extensions/browser/process_manager_factory.h"
 #include "extensions/browser/test_extensions_browser_client.h"
@@ -148,17 +149,17 @@ TEST_F(LazyBackgroundTaskQueueTest, AddPendingTask) {
 
   // Adding a pending task increases the number of extensions with tasks, but
   // doesn't run the task.
-  queue.AddPendingTask(browser_context(),
-                       no_background->id(),
+  const LazyContextId no_background_context_id(browser_context(),
+                                               no_background->id());
+  queue.AddPendingTask(no_background_context_id,
                        base::Bind(&LazyBackgroundTaskQueueTest::RunPendingTask,
                                   base::Unretained(this)));
   EXPECT_EQ(1u, queue.pending_tasks_.size());
   EXPECT_EQ(0, task_run_count());
 
   // Another task on the same extension doesn't increase the number of
   // extensions that have tasks and doesn't run any tasks.
-  queue.AddPendingTask(browser_context(),
-                       no_background->id(),
+  queue.AddPendingTask(no_background_context_id,
                        base::Bind(&LazyBackgroundTaskQueueTest::RunPendingTask,
                                   base::Unretained(this)));
   EXPECT_EQ(1u, queue.pending_tasks_.size());
@@ -168,8 +169,9 @@ TEST_F(LazyBackgroundTaskQueueTest, AddPendingTask) {
   // a background host, and if that fails, runs the task immediately.
   scoped_refptr<const Extension> lazy_background =
       CreateLazyBackgroundExtension();
-  queue.AddPendingTask(browser_context(),
-                       lazy_background->id(),
+  const LazyContextId lazy_background_context_id(browser_context(),
+                                                 lazy_background->id());
+  queue.AddPendingTask(lazy_background_context_id,
                        base::Bind(&LazyBackgroundTaskQueueTest::RunPendingTask,
                                   base::Unretained(this)));
   EXPECT_EQ(1u, queue.pending_tasks_.size());
@@ -189,8 +191,7 @@ TEST_F(LazyBackgroundTaskQueueTest, ProcessPendingTasks) {
   EXPECT_EQ(0, task_run_count());
 
   // Schedule a task to run.
-  queue.AddPendingTask(browser_context(),
-                       extension->id(),
+  queue.AddPendingTask(LazyContextId(browser_context(), extension->id()),
                        base::Bind(&LazyBackgroundTaskQueueTest::RunPendingTask,
                                   base::Unretained(this)));
   EXPECT_EQ(0, task_run_count());
@@ -225,7 +226,7 @@ TEST_F(LazyBackgroundTaskQueueTest, CreateLazyBackgroundPageOnExtensionLoaded) {
   // Did not try to create a background host because there are no queued tasks.
   EXPECT_EQ(0, process_manager()->create_count());
 
-  queue.AddPendingTask(browser_context(), lazy_background->id(),
+  queue.AddPendingTask(LazyContextId(browser_context(), lazy_background->id()),
                        base::Bind(&LazyBackgroundTaskQueueTest::RunPendingTask,
                                   base::Unretained(this)));
   EXPECT_EQ(1u, queue.pending_tasks_.size());

diff --git a/extensions/browser/lazy_context_task_queue.h b/extensions/browser/lazy_context_task_queue.h
@@ -65,15 +65,8 @@ class LazyContextTaskQueue {
   // be loaded. The task will be called either when the page is loaded,
   // or when the page fails to load for some reason (e.g. a crash or browser
   // shutdown). In the latter case, the ContextInfo will be nullptr.
-  //
-  // TODO(lazyboy): Remove "ToDispatchEvent" suffix and simply call this
-  // AddPendingTask. Issues:
-  // 1. We already have LazyBackgroundTaskQueue::AddPendingTask. Moreover, that
-  //    is heavily used thoughout the codebase.
-  // 2. LazyBackgroundTaskQueue::AddPendingTask is tied to ExtensionHost. This
-  //    class should be ExtensionHost agnostic.
-  virtual void AddPendingTaskToDispatchEvent(const LazyContextId& context_id,
-                                             PendingTask task) = 0;
+  virtual void AddPendingTask(const LazyContextId& context_id,
+                              PendingTask task) = 0;
 };
 
 }  // namespace extensions

diff --git a/extensions/browser/process_manager.cc b/extensions/browser/process_manager.cc
@@ -33,6 +33,7 @@
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/extensions_browser_client.h"
 #include "extensions/browser/lazy_background_task_queue.h"
+#include "extensions/browser/lazy_context_id.h"
 #include "extensions/browser/notification_types.h"
 #include "extensions/browser/process_manager_delegate.h"
 #include "extensions/browser/process_manager_factory.h"
@@ -434,7 +435,7 @@ bool ProcessManager::WakeEventPage(const std::string& extension_id,
   LazyBackgroundTaskQueue* queue =
       LazyBackgroundTaskQueue::Get(browser_context_);
   queue->AddPendingTask(
-      browser_context_, extension_id,
+      LazyContextId(browser_context_, extension_id),
       base::BindOnce(&PropagateExtensionWakeResult, std::move(callback)));
   return true;
 }

diff --git a/extensions/browser/service_worker_task_queue.cc b/extensions/browser/service_worker_task_queue.cc
@@ -200,9 +200,8 @@ bool ServiceWorkerTaskQueue::ShouldEnqueueTask(BrowserContext* context,
   return true;
 }
 
-void ServiceWorkerTaskQueue::AddPendingTaskToDispatchEvent(
-    const LazyContextId& context_id,
-    PendingTask task) {
+void ServiceWorkerTaskQueue::AddPendingTask(const LazyContextId& context_id,
+                                            PendingTask task) {
   DCHECK(context_id.is_for_service_worker());
 
   // TODO(lazyboy): Do we need to handle incognito context?

diff --git a/extensions/browser/service_worker_task_queue.h b/extensions/browser/service_worker_task_queue.h
@@ -37,8 +37,8 @@ class ServiceWorkerTaskQueue : public KeyedService,
 
   bool ShouldEnqueueTask(content::BrowserContext* context,
                          const Extension* extension) override;
-  void AddPendingTaskToDispatchEvent(const LazyContextId& context_id,
-                                     PendingTask task) override;
+  void AddPendingTask(const LazyContextId& context_id,
+                      PendingTask task) override;
 
   // Performs Service Worker related tasks upon |extension| activation,
   // e.g. registering |extension|'s worker, executing any pending tasks.