Extension SW: Run all PendingTasks once a lazy context's worker is ready.

For running PendingTasks for a particular ContextId through
SWTQ::AddPendingTask(), call StartWorker* exactly once instead of calling it
each time for each AddPendingTask call. This has two advantages:
  - We don't call StartWorker* more than once when we don't need to.
  - Extension messaging subtly depends on running these tasks once. This is
    because how LazyBackgroundTaskQueue behaves in terms of multiple pending
    tasks and we need some sort of parity in SWTQ for future worker messaging
    implementation.

Detailed changes:
- ServiceWorkerTaskQueue: Remove the queue for registration
  (old |pending_tasks_|) and worker start (|waiting_did_start_worker_tasks_|)
  and merge them to a single queue (|pending_tasks_). Run these tasks
  once registration, service worker start (DidStartWorkerForScope) and
  service worker loadstop (DidStartServiceWorkerContext) completes.
- IPC: Add thread_id and SW scope to DidStartServiceWorkerContext IPC
  (loadstop), as PendingTasks that were pending after DidStartWorkerForScope
  but before the SW context has seen loadstop can use these thread_id and
  scope to build LazyContextTaskQueue::ContextInfo to run those tasks.
  Previously we used to remember these two pieces of information in
  WaitingDidStartWorkerTask. This also simplifies the queue in SWTQ.
- IPC: Add thread_id and SW scope to DidStopServiceWorkerContext IPC
  to clean up the task containers on worker shutdown. This will also
  be necessary for multiple worker support within extension in future.

Bug: 925927
Test: Service worker within extension internal change, no visible effect.
Change-Id: I06581f821550154ebca3ee6d9aa803953abf7d86
Reviewed-on: https://chromium-review.googlesource.com/c/1481580Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636239}

extensions/browser/extension_service_worker_message_filter.cc

@@ -104,28 +104,44 @@ void ExtensionServiceWorkerMessageFilter::OnEventAckWorker(
 
 void ExtensionServiceWorkerMessageFilter::OnDidStartServiceWorkerContext(
     const std::string& extension_id,
-    int64_t service_worker_version_id) {
+    const GURL& service_worker_scope,
+    int64_t service_worker_version_id,
+    int thread_id) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK_NE(kMainThreadId, thread_id);
   if (!ProcessMap::Get(browser_context_)
            ->Contains(extension_id, render_process_id_)) {
     // We can legitimately get here if the extension was already unloaded.
     return;
   }
+  CHECK(service_worker_scope.SchemeIs(kExtensionScheme) &&
+        extension_id == service_worker_scope.host_piece());
+
   ServiceWorkerTaskQueue::Get(browser_context_)
-      ->DidStartServiceWorkerContext(extension_id, service_worker_version_id);
+      ->DidStartServiceWorkerContext(render_process_id_, extension_id,
+                                     service_worker_scope,
+                                     service_worker_version_id, thread_id);
 }
 
 void ExtensionServiceWorkerMessageFilter::OnDidStopServiceWorkerContext(
     const std::string& extension_id,
-    int64_t service_worker_version_id) {
+    const GURL& service_worker_scope,
+    int64_t service_worker_version_id,
+    int thread_id) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK_NE(kMainThreadId, thread_id);
   if (!ProcessMap::Get(browser_context_)
            ->Contains(extension_id, render_process_id_)) {
     // We can legitimately get here if the extension was already unloaded.
     return;
   }
+  CHECK(service_worker_scope.SchemeIs(kExtensionScheme) &&
+        extension_id == service_worker_scope.host_piece());
+
   ServiceWorkerTaskQueue::Get(browser_context_)
-      ->DidStopServiceWorkerContext(extension_id, service_worker_version_id);
+      ->DidStopServiceWorkerContext(render_process_id_, extension_id,
+                                    service_worker_scope,
+                                    service_worker_version_id, thread_id);
 }
 
 void ExtensionServiceWorkerMessageFilter::DidFailDecrementInflightEvent() {

extensions/browser/extension_service_worker_message_filter.h

@@ -10,6 +10,7 @@
 #include "content/public/browser/browser_thread.h"
 #include "extensions/common/extension_id.h"
 
+class GURL;
 struct ExtensionHostMsg_Request_Params;
 
 namespace content {
@@ -46,9 +47,13 @@ class ExtensionServiceWorkerMessageFilter
                                         const std::string& request_uuid);
   void OnEventAckWorker(int64_t service_worker_version_id, int event_id);
   void OnDidStartServiceWorkerContext(const ExtensionId& extension_id,
-                                      int64_t service_worker_version_id);
+                                      const GURL& service_worker_scope,
+                                      int64_t service_worker_version_id,
+                                      int thread_id);
   void OnDidStopServiceWorkerContext(const ExtensionId& extension_id,
-                                     int64_t service_worker_version_id);
+                                     const GURL& service_worker_scope,
+                                     int64_t service_worker_version_id,
+                                     int thread_id);
 
   void DidFailDecrementInflightEvent();
 

extensions/browser/service_worker_task_queue.h

@@ -7,6 +7,7 @@
 
 #include <map>
 #include <set>
+#include <vector>
 
 #include "base/memory/weak_ptr.h"
 #include "base/version.h"
@@ -25,6 +26,10 @@ class Extension;
 class LazyContextId;
 
 // TODO(lazyboy): Clean up queue when extension is unloaded/uninstalled.
+// TODO(lazyboy): Describe the flow of a task in this queue: i.e. when
+// a worker receives DidRegisterServiceWorker, DidStartWorkerForScope and
+// DidStartServiceWorkerContext events and how queued tasks react to these
+// events.
 class ServiceWorkerTaskQueue : public KeyedService,
                                public LazyContextTaskQueue {
  public:
@@ -48,11 +53,19 @@ class ServiceWorkerTaskQueue : public KeyedService,
   void DeactivateExtension(const Extension* extension);
 
   // Called once an extension Service Worker started running.
-  void DidStartServiceWorkerContext(const ExtensionId& extension_id,
-                                    int64_t service_worker_version_id);
+  // This can be thought as "loadstop", i.e. the global JS script of the worker
+  // has completed executing.
+  void DidStartServiceWorkerContext(int render_process_id,
+                                    const ExtensionId& extension_id,
+                                    const GURL& service_worker_scope,
+                                    int64_t service_worker_version_id,
+                                    int thread_id);
   // Called once an extension Service Worker was destroyed.
-  void DidStopServiceWorkerContext(const ExtensionId& extension_id,
-                                   int64_t service_worker_version_id);
+  void DidStopServiceWorkerContext(int render_process_id,
+                                   const ExtensionId& extension_id,
+                                   const GURL& service_worker_scope,
+                                   int64_t service_worker_version_id,
+                                   int thread_id);
 
   class TestObserver {
    public:
@@ -72,31 +85,24 @@ class ServiceWorkerTaskQueue : public KeyedService,
   static void SetObserverForTest(TestObserver* observer);
 
  private:
-  struct TaskInfo;
-
   static void DidStartWorkerForScopeOnIO(
-      PendingTask task,
-      const ExtensionId& extension_id,
+      const LazyContextId& context_id,
       base::WeakPtr<ServiceWorkerTaskQueue> task_queue,
       int64_t version_id,
       int process_id,
       int thread_id);
-  static void StartServiceWorkerOnIOToRunTask(
+  static void StartServiceWorkerOnIOToRunTasks(
       base::WeakPtr<ServiceWorkerTaskQueue> task_queue_weak,
-      const GURL& scope,
-      const ExtensionId& extension_id,
-      content::ServiceWorkerContext* service_worker_context,
-      PendingTask task);
+      const LazyContextId& context_id,
+      content::ServiceWorkerContext* service_worker_context);
 
-  void RunTaskAfterStartWorker(const LazyContextId& context_id,
-                               PendingTask task);
+  void RunTasksAfterStartWorker(const LazyContextId& context_id);
 
   void DidRegisterServiceWorker(const ExtensionId& extension_id, bool success);
   void DidUnregisterServiceWorker(const ExtensionId& extension_id,
                                   bool success);
 
-  void DidStartWorkerForScope(PendingTask task,
-                              const ExtensionId& extension_id,
+  void DidStartWorkerForScope(const LazyContextId& context_id,
                               int64_t version_id,
                               int process_id,
                               int thread_id);
@@ -116,22 +122,26 @@ class ServiceWorkerTaskQueue : public KeyedService,
   // |extension_id|.
   void RemoveRegisteredServiceWorkerInfo(const ExtensionId& extension_id);
 
+  // If the worker with |context_id| has seen worker start
+  // (DidStartWorkerForScope) and load (DidStartServiceWorkerContext) then runs
+  // all pending tasks for that worker.
+  void RunPendingTasksIfWorkerReady(const LazyContextId& context_id,
+                                    int64_t version_id,
+                                    int process_id,
+                                    int thread_id);
+
   // Set of extension ids that hasn't completed Service Worker registration.
   std::set<ExtensionId> pending_registrations_;
 
-  // Map of extension id -> pending tasks. These are run once the Service Worker
-  // registration of the extension completes.
-  std::map<ExtensionId, std::vector<TaskInfo>> pending_tasks_;
+  // Set of workers that has seen DidStartWorkerForScope.
+  std::set<LazyContextId> started_workers_;
 
-  struct WaitingDidStartWorkerTask;
+  // Set of workers that has seen DidStartServiceWorkerContext.
+  std::set<LazyContextId> loaded_workers_;
 
-  using WaitingDidStartWorkerTaskKey =
-      std::pair<ExtensionId, int64_t /* service_worker_version_id */>;
-  // All service workers that are currently loaded.
-  std::set<WaitingDidStartWorkerTaskKey> running_service_worker_contexts_;
-  // All service worker tasks that are waiting for the worker to start.
-  std::map<WaitingDidStartWorkerTaskKey, std::vector<WaitingDidStartWorkerTask>>
-      waiting_did_start_worker_tasks_;
+  // Pending tasks for a |LazyContextId|.
+  // These tasks will be run once the corresponding worker becomes ready.
+  std::map<LazyContextId, std::vector<PendingTask>> pending_tasks_;
 
   content::BrowserContext* const browser_context_ = nullptr;
 

extensions/common/extension_messages.h

@@ -1058,15 +1058,19 @@ IPC_MESSAGE_CONTROL2(ExtensionHostMsg_EventAckWorker,
 //     straightforward as it changes SW IPC ordering with respect of rest of
 //     Chrome.
 // See https://crbug.com/879015#c4 for details.
-IPC_MESSAGE_CONTROL2(ExtensionHostMsg_DidStartServiceWorkerContext,
+IPC_MESSAGE_CONTROL4(ExtensionHostMsg_DidStartServiceWorkerContext,
                      std::string /* extension_id */,
-                     int64_t /* service_worker_version_id */)
+                     GURL /* service_worker_scope */,
+                     int64_t /* service_worker_version_id */,
+                     int /* worker_thread_id */)
 
 // Tells the browser that an extension service worker context has been
 // destroyed.
-IPC_MESSAGE_CONTROL2(ExtensionHostMsg_DidStopServiceWorkerContext,
+IPC_MESSAGE_CONTROL4(ExtensionHostMsg_DidStopServiceWorkerContext,
                      std::string /* extension_id */,
-                     int64_t /* service_worker_version_id */)
+                     GURL /* service_worker_scope */,
+                     int64_t /* service_worker_version_id */,
+                     int /* worker_thread_id */)
 
 IPC_STRUCT_TRAITS_BEGIN(ui::AXNodeData)
   IPC_STRUCT_TRAITS_MEMBER(id)

extensions/renderer/dispatcher.cc

@@ -498,7 +498,8 @@ void Dispatcher::DidStartServiceWorkerContextOnWorkerThread(
     return;
 
   DCHECK_NE(content::WorkerThread::GetCurrentId(), kMainThreadId);
-  WorkerThreadDispatcher::Get()->DidStartContext(service_worker_version_id);
+  WorkerThreadDispatcher::Get()->DidStartContext(service_worker_scope,
+                                                 service_worker_version_id);
 }
 
 // static
@@ -519,7 +520,8 @@ void Dispatcher::WillDestroyServiceWorkerContextOnWorkerThread(
     ExtensionBindingsSystem* worker_bindings_system =
         WorkerThreadDispatcher::GetBindingsSystem();
     worker_bindings_system->WillReleaseScriptContext(script_context);
-    WorkerThreadDispatcher::Get()->DidStopContext(service_worker_version_id);
+    WorkerThreadDispatcher::Get()->DidStopContext(service_worker_scope,
+                                                  service_worker_version_id);
     // Note: we have to remove the context (and thus perform invalidation on
     // the native handlers) prior to removing the worker data, which destroys
     // the associated bindings system.

extensions/renderer/worker_thread_dispatcher.cc

@@ -169,18 +169,26 @@ void WorkerThreadDispatcher::AddWorkerData(
 }
 
 void WorkerThreadDispatcher::DidStartContext(
+    const GURL& service_worker_scope,
     int64_t service_worker_version_id) {
   ServiceWorkerData* data = g_data_tls.Pointer()->Get();
   DCHECK_EQ(service_worker_version_id, data->service_worker_version_id());
+  const int thread_id = content::WorkerThread::GetCurrentId();
+  DCHECK_NE(thread_id, kMainThreadId);
   Send(new ExtensionHostMsg_DidStartServiceWorkerContext(
-      data->context()->GetExtensionID(), service_worker_version_id));
+      data->context()->GetExtensionID(), service_worker_scope,
+      service_worker_version_id, thread_id));
 }
 
-void WorkerThreadDispatcher::DidStopContext(int64_t service_worker_version_id) {
+void WorkerThreadDispatcher::DidStopContext(const GURL& service_worker_scope,
+                                            int64_t service_worker_version_id) {
   ServiceWorkerData* data = g_data_tls.Pointer()->Get();
+  const int thread_id = content::WorkerThread::GetCurrentId();
+  DCHECK_NE(thread_id, kMainThreadId);
   DCHECK_EQ(service_worker_version_id, data->service_worker_version_id());
   Send(new ExtensionHostMsg_DidStopServiceWorkerContext(
-      data->context()->GetExtensionID(), service_worker_version_id));
+      data->context()->GetExtensionID(), service_worker_scope,
+      service_worker_version_id, thread_id));
 }
 
 void WorkerThreadDispatcher::RemoveWorkerData(

extensions/renderer/worker_thread_dispatcher.h

@@ -57,9 +57,11 @@ class WorkerThreadDispatcher : public content::RenderThreadObserver,
   EventBookkeeper* event_bookkeeper() { return &event_bookkeeper_; }
 
   // Called when a service worker context started running.
-  void DidStartContext(int64_t service_worker_version_id);
+  void DidStartContext(const GURL& service_worker_scope,
+                       int64_t service_worker_version_id);
   // Called when a service worker context was destroyed.
-  void DidStopContext(int64_t service_worker_version_id);
+  void DidStopContext(const GURL& service_worker_scope,
+                      int64_t service_worker_version_id);
 
   // content::RenderThreadObserver:
   bool OnControlMessageReceived(const IPC::Message& message) override;