[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 <[email protected]>
Reviewed-by: Stuart Langley <[email protected]>
Reviewed-by: Ben Wells <[email protected]>
Reviewed-by: Istiaque Ahmed <[email protected]>
Cr-Commit-Position: refs/heads/master@{#617843}diff --git a/extensions/browser/api/messaging/message_service.cc b/extensions/browser/api/messaging/message_service.cc
index 95e8cc9..4cf086d 100644
--- 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 @@
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 @@
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 @@
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
index 7416632..50836799 100644
--- 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 @@
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 @@
->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
index dba9fc1..887d171 100644
--- a/extensions/browser/events/lazy_event_dispatcher.cc
+++ b/extensions/browser/events/lazy_event_dispatcher.cc
@@ -118,8 +118,8 @@
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
index 55d68f7..e54628e 100644
--- 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 @@
// 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
index 13f0910..f4596561 100644
--- 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 @@
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
index bfc6afc..69f0db3 100644
--- a/extensions/browser/lazy_background_task_queue.cc
+++ b/extensions/browser/lazy_background_task_queue.cc
@@ -82,21 +82,14 @@
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
index b9173320..6e0ddbf 100644
--- a/extensions/browser/lazy_background_task_queue.h
+++ b/extensions/browser/lazy_background_task_queue.h
@@ -56,19 +56,14 @@
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
index 740e115..926a3fc 100644
--- 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,8 +149,9 @@
// 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());
@@ -157,8 +159,7 @@
// 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 @@
// 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 @@
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 @@
// 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
index f6ef47f..1d3d642 100644
--- a/extensions/browser/lazy_context_task_queue.h
+++ b/extensions/browser/lazy_context_task_queue.h
@@ -65,15 +65,8 @@
// 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
index 882c7e7..67b3acf 100644
--- 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 @@
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
index 7f57446..6b1b9db9 100644
--- a/extensions/browser/service_worker_task_queue.cc
+++ b/extensions/browser/service_worker_task_queue.cc
@@ -200,9 +200,8 @@
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
index 36d27e8..8594889 100644
--- a/extensions/browser/service_worker_task_queue.h
+++ b/extensions/browser/service_worker_task_queue.h
@@ -37,8 +37,8 @@
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.
