service worker: Prepare extensions code for UI thread core.

The thread ServiceWorkerContextCore lives on (the "core thread") will
move from the IO thread to the UI thread when the ServiceWorkerOnUI
feature is enabled.

This makes most tests involving extension service workers to pass.

Bug: 824858, 984522
Change-Id: Ide0d60cbe9be15ea6149d29761a50506ac4376fb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1763560
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695394}

chrome/browser/extensions/service_worker_apitest.cc

@@ -1203,11 +1203,17 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerTest, UpdatePackedExtension) {
   // a callback that forces itself to reload.
   constexpr char kScript1[] =
       R"(
+         addEventListener('activate', event => {
+           // TODO(crbug.com/999027, crbug.com/984522): Delaying sending
+           // 'ready1' until activation seems to reduce the flakiness described
+           // in the bugs. It'd be better to fix the underlying bugs.
+           chrome.test.sendMessage('ready1');
+         });
+
          chrome.runtime.onUpdateAvailable.addListener(function(details) {
            chrome.test.assertEq('%s', details.version);
            chrome.runtime.reload();
          });
-         chrome.test.sendMessage('ready1');
         )";
 
   std::string id;
@@ -1289,11 +1295,29 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerTest, UpdateUnpackedExtension) {
          })";
   constexpr char kScript[] =
       R"(
+         let activated;
+         let installed;
+
+         addEventListener('activate', event => {
+           activated = true;
+           NotifyIfDone();
+         });
+
          chrome.runtime.onInstalled.addListener(function(details) {
+           installed = true;
            chrome.test.assertEq('%s', details.reason);
-           chrome.test.sendMessage('onInstalled');
+           NotifyIfDone();
          });
-         chrome.test.sendMessage('%s');
+
+         // TODO(crbug.com/999027, crbug.com/984522): Delaying notification
+         // until activation seems to reduce the flakiness described in the
+         // bugs. It'd be better to fix the underlying bugs.
+         function NotifyIfDone() {
+           if (!activated || !installed)
+             return;
+           chrome.test.sendMessage('%s');
+           chrome.test.sendMessage('onInstalled');
+         }
         )";
 
   std::string id;

extensions/browser/events/event_ack_data.cc

@@ -26,32 +26,34 @@ using EventInfo = std::pair<std::string, int>;
 }  // namespace
 
 // Class that holds map of unacked event information keyed by event id, accessed
-// on IO thread.
+// on service worker core thread.
+// TODO(crbug.com/824858): This shouldn't be needed after service worker core
+// thread moves to the UI thread.
 // TODO(lazyboy): Could this potentially be owned exclusively (and deleted) on
-// the IO thread, thus not requiring RefCounted?
-class EventAckData::IOEventInfo
-    : public base::RefCountedThreadSafe<IOEventInfo> {
+// the core thread, thus not requiring RefCounted?
+class EventAckData::CoreThreadEventInfo
+    : public base::RefCountedThreadSafe<CoreThreadEventInfo> {
  public:
-  IOEventInfo() = default;
+  CoreThreadEventInfo() = default;
 
   // Map of event information keyed by event_id.
   std::map<int, EventInfo> event_map;
 
  private:
-  friend class base::RefCountedThreadSafe<IOEventInfo>;
-  ~IOEventInfo() = default;
+  friend class base::RefCountedThreadSafe<CoreThreadEventInfo>;
+  ~CoreThreadEventInfo() = default;
 
-  DISALLOW_COPY_AND_ASSIGN(IOEventInfo);
+  DISALLOW_COPY_AND_ASSIGN(CoreThreadEventInfo);
 };
 
 // static
-void EventAckData::StartExternalRequestOnIO(
+void EventAckData::StartExternalRequestOnCoreThread(
     content::ServiceWorkerContext* context,
     int render_process_id,
     int64_t version_id,
     int event_id,
-    scoped_refptr<IOEventInfo> unacked_events) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+    scoped_refptr<CoreThreadEventInfo> unacked_events) {
+  DCHECK_CURRENTLY_ON(content::ServiceWorkerContext::GetCoreThreadId());
 
   std::string request_uuid = base::GenerateGUID();
 
@@ -71,14 +73,14 @@ void EventAckData::StartExternalRequestOnIO(
 }
 
 // static
-void EventAckData::FinishExternalRequestOnIO(
+void EventAckData::FinishExternalRequestOnCoreThread(
     content::ServiceWorkerContext* context,
     int render_process_id,
     int64_t version_id,
     int event_id,
-    scoped_refptr<IOEventInfo> unacked_events,
+    scoped_refptr<CoreThreadEventInfo> unacked_events,
     base::OnceClosure failure_callback) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(content::ServiceWorkerContext::GetCoreThreadId());
 
   std::map<int, EventInfo>& unacked_events_map = unacked_events->event_map;
   auto request_info_iter = unacked_events_map.find(event_id);
@@ -100,7 +102,7 @@ void EventAckData::FinishExternalRequestOnIO(
 }
 
 EventAckData::EventAckData()
-    : unacked_events_(base::MakeRefCounted<IOEventInfo>()) {}
+    : unacked_events_(base::MakeRefCounted<CoreThreadEventInfo>()) {}
 EventAckData::~EventAckData() = default;
 
 void EventAckData::IncrementInflightEvent(
@@ -110,11 +112,17 @@ void EventAckData::IncrementInflightEvent(
     int event_id) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  content::ServiceWorkerContext::RunTask(
-      base::CreateSingleThreadTaskRunner({content::BrowserThread::IO}),
-      FROM_HERE, context,
-      base::BindOnce(&EventAckData::StartExternalRequestOnIO, context,
-                     render_process_id, version_id, event_id, unacked_events_));
+  if (content::ServiceWorkerContext::IsServiceWorkerOnUIEnabled()) {
+    StartExternalRequestOnCoreThread(context, render_process_id, version_id,
+                                     event_id, unacked_events_);
+  } else {
+    content::ServiceWorkerContext::RunTask(
+        base::CreateSingleThreadTaskRunner({content::BrowserThread::IO}),
+        FROM_HERE, context,
+        base::BindOnce(&EventAckData::StartExternalRequestOnCoreThread, context,
+                       render_process_id, version_id, event_id,
+                       unacked_events_));
+  }
 }
 
 void EventAckData::DecrementInflightEvent(
@@ -125,12 +133,18 @@ void EventAckData::DecrementInflightEvent(
     base::OnceClosure failure_callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  content::ServiceWorkerContext::RunTask(
-      base::CreateSingleThreadTaskRunner({content::BrowserThread::IO}),
-      FROM_HERE, context,
-      base::BindOnce(&EventAckData::FinishExternalRequestOnIO, context,
-                     render_process_id, version_id, event_id, unacked_events_,
-                     std::move(failure_callback)));
+  if (content::ServiceWorkerContext::IsServiceWorkerOnUIEnabled()) {
+    FinishExternalRequestOnCoreThread(context, render_process_id, version_id,
+                                      event_id, unacked_events_,
+                                      std::move(failure_callback));
+  } else {
+    content::ServiceWorkerContext::RunTask(
+        base::CreateSingleThreadTaskRunner({content::BrowserThread::IO}),
+        FROM_HERE, context,
+        base::BindOnce(&EventAckData::FinishExternalRequestOnCoreThread,
+                       context, render_process_id, version_id, event_id,
+                       unacked_events_, std::move(failure_callback)));
+  }
 }
 
 }  // namespace extensions

extensions/browser/events/event_ack_data.h

@@ -38,26 +38,26 @@ class EventAckData {
                               base::OnceClosure failure_callback);
 
  private:
-  class IOEventInfo;
+  class CoreThreadEventInfo;
 
-  static void StartExternalRequestOnIO(
+  static void StartExternalRequestOnCoreThread(
       content::ServiceWorkerContext* context,
       int render_process_id,
       int64_t version_id,
       int event_id,
-      scoped_refptr<EventAckData::IOEventInfo> unacked_events);
+      scoped_refptr<EventAckData::CoreThreadEventInfo> unacked_events);
 
-  static void FinishExternalRequestOnIO(
+  static void FinishExternalRequestOnCoreThread(
       content::ServiceWorkerContext* context,
       int render_process_id,
       int64_t version_id,
       int event_id,
-      scoped_refptr<IOEventInfo> unacked_events,
+      scoped_refptr<CoreThreadEventInfo> unacked_events,
       base::OnceClosure failure_callback);
 
   // Contains map of unacked event information keyed by event id.
-  // Created on UI thread, but accessed only on IO thread.
-  scoped_refptr<IOEventInfo> unacked_events_;
+  // Created on UI thread, but accessed only on the core thread.
+  scoped_refptr<CoreThreadEventInfo> unacked_events_;
 
   base::WeakPtrFactory<EventAckData> weak_factory_{this};
 

extensions/browser/extension_service_worker_message_filter.cc

@@ -41,6 +41,12 @@ void ExtensionServiceWorkerMessageFilter::OverrideThreadForMessage(
       message.type() == ExtensionHostMsg_DidStopServiceWorkerContext::ID) {
     *thread = content::BrowserThread::UI;
   }
+
+  if (content::ServiceWorkerContext::IsServiceWorkerOnUIEnabled() &&
+      (message.type() == ExtensionHostMsg_IncrementServiceWorkerActivity::ID ||
+       message.type() == ExtensionHostMsg_DecrementServiceWorkerActivity::ID)) {
+    *thread = content::BrowserThread::UI;
+  }
 }
 
 bool ExtensionServiceWorkerMessageFilter::OnMessageReceived(
@@ -73,7 +79,7 @@ void ExtensionServiceWorkerMessageFilter::OnRequestWorker(
 void ExtensionServiceWorkerMessageFilter::OnIncrementServiceWorkerActivity(
     int64_t service_worker_version_id,
     const std::string& request_uuid) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(content::ServiceWorkerContext::GetCoreThreadId());
   active_request_uuids_.insert(request_uuid);
   // The worker might have already stopped before we got here, so the increment
   // below might fail legitimately. Therefore, we do not send bad_message to the
@@ -85,7 +91,7 @@ void ExtensionServiceWorkerMessageFilter::OnIncrementServiceWorkerActivity(
 void ExtensionServiceWorkerMessageFilter::OnDecrementServiceWorkerActivity(
     int64_t service_worker_version_id,
     const std::string& request_uuid) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(content::ServiceWorkerContext::GetCoreThreadId());
   content::ServiceWorkerExternalRequestResult result =
       service_worker_context_->FinishedExternalRequest(
           service_worker_version_id, request_uuid);
@@ -93,6 +99,7 @@ void ExtensionServiceWorkerMessageFilter::OnDecrementServiceWorkerActivity(
     LOG(ERROR) << "ServiceWorkerContext::FinishedExternalRequest failed: "
                << static_cast<int>(result);
   }
+
   bool erased = active_request_uuids_.erase(request_uuid) == 1;
   // The worker may have already stopped before we got here, so only report
   // a bad message if we didn't have an increment for the UUID.

extensions/browser/process_manager.cc

@@ -129,14 +129,14 @@ void PropagateExtensionWakeResult(
 void StartServiceWorkerExternalRequest(content::ServiceWorkerContext* context,
                                        int64_t service_worker_version_id,
                                        const std::string& request_uuid) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(content::ServiceWorkerContext::GetCoreThreadId());
   context->StartingExternalRequest(service_worker_version_id, request_uuid);
 }
 
 void FinishServiceWorkerExternalRequest(content::ServiceWorkerContext* context,
                                         int64_t service_worker_version_id,
                                         const std::string& request_uuid) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(content::ServiceWorkerContext::GetCoreThreadId());
   content::ServiceWorkerExternalRequestResult result =
       context->FinishedExternalRequest(service_worker_version_id, request_uuid);
   DCHECK_EQ(result, content::ServiceWorkerExternalRequestResult::kOk);
@@ -787,10 +787,16 @@ std::string ProcessManager::IncrementServiceWorkerKeepaliveCount(
                                                           extension->url())
           ->GetServiceWorkerContext();
 
-  content::ServiceWorkerContext::RunTask(
-      worker_task_runner_, FROM_HERE, service_worker_context,
-      base::BindOnce(&StartServiceWorkerExternalRequest, service_worker_context,
-                     service_worker_version_id, request_uuid));
+  if (content::ServiceWorkerContext::IsServiceWorkerOnUIEnabled()) {
+    StartServiceWorkerExternalRequest(service_worker_context,
+                                      service_worker_version_id, request_uuid);
+  } else {
+    content::ServiceWorkerContext::RunTask(
+        worker_task_runner_, FROM_HERE, service_worker_context,
+        base::BindOnce(&StartServiceWorkerExternalRequest,
+                       service_worker_context, service_worker_version_id,
+                       request_uuid));
+  }
   return request_uuid;
 }
 
@@ -847,11 +853,16 @@ void ProcessManager::DecrementServiceWorkerKeepaliveCount(
                                                           extension->url())
           ->GetServiceWorkerContext();
 
-  content::ServiceWorkerContext::RunTask(
-      worker_task_runner_, FROM_HERE, service_worker_context,
-      base::BindOnce(&FinishServiceWorkerExternalRequest,
-                     service_worker_context, service_worker_version_id,
-                     request_uuid));
+  if (content::ServiceWorkerContext::IsServiceWorkerOnUIEnabled()) {
+    FinishServiceWorkerExternalRequest(service_worker_context,
+                                       service_worker_version_id, request_uuid);
+  } else {
+    content::ServiceWorkerContext::RunTask(
+        worker_task_runner_, FROM_HERE, service_worker_context,
+        base::BindOnce(&FinishServiceWorkerExternalRequest,
+                       service_worker_context, service_worker_version_id,
+                       request_uuid));
+  }
 }
 
 void ProcessManager::OnLazyBackgroundPageIdle(const std::string& extension_id,

extensions/browser/process_manager.h

@@ -351,6 +351,7 @@ class ProcessManager : public KeyedService,
   ExtensionRenderFrames all_extension_frames_;
 
   // TaskRunner for interacting with ServiceWorkerContexts.
+  // TODO(crbug.com/824858): This is unused when ServiceWorkerOnUI is enabled.
   scoped_refptr<base::SequencedTaskRunner> worker_task_runner_;
 
   // Contains all active extension Service Worker information for all

extensions/browser/service_worker_task_queue.cc

@@ -42,8 +42,10 @@ const char kServiceWorkerVersion[] = "version";
 ServiceWorkerTaskQueue::TestObserver* g_test_observer = nullptr;
 
 void DidStartWorkerFail() {
-  DCHECK(false) << "DidStartWorkerFail";
-  // TODO(lazyboy): Handle failure case.
+  // TODO(lazyboy): Handle failure case. One case this can happen is when the
+  // registration got unregistered right before we tried to start it, see
+  // crbug.com/999027.
+  LOG(ERROR) << "DidStartWorkerFail";
 }
 
 }  // namespace
@@ -63,29 +65,38 @@ ServiceWorkerTaskQueue* ServiceWorkerTaskQueue::Get(BrowserContext* context) {
 }
 
 // static
-void ServiceWorkerTaskQueue::DidStartWorkerForScopeOnIO(
+void ServiceWorkerTaskQueue::DidStartWorkerForScopeOnCoreThread(
     const LazyContextId& context_id,
     base::WeakPtr<ServiceWorkerTaskQueue> task_queue,
     int64_t version_id,
     int process_id,
     int thread_id) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
-  base::PostTask(FROM_HERE, {content::BrowserThread::UI},
-                 base::BindOnce(&ServiceWorkerTaskQueue::DidStartWorkerForScope,
-                                task_queue, context_id, version_id, process_id,
-                                thread_id));
+  DCHECK_CURRENTLY_ON(content::ServiceWorkerContext::GetCoreThreadId());
+  if (content::ServiceWorkerContext::IsServiceWorkerOnUIEnabled()) {
+    if (task_queue) {
+      task_queue->DidStartWorkerForScope(context_id, version_id, process_id,
+                                         thread_id);
+    }
+  } else {
+    base::PostTask(
+        FROM_HERE, {content::BrowserThread::UI},
+        base::BindOnce(&ServiceWorkerTaskQueue::DidStartWorkerForScope,
+                       task_queue, context_id, version_id, process_id,
+                       thread_id));
+  }
 }
 
 // static
-void ServiceWorkerTaskQueue::StartServiceWorkerOnIOToRunTasks(
+void ServiceWorkerTaskQueue::StartServiceWorkerOnCoreThreadToRunTasks(
     base::WeakPtr<ServiceWorkerTaskQueue> task_queue_weak,
     const LazyContextId& context_id,
     content::ServiceWorkerContext* service_worker_context) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(content::ServiceWorkerContext::GetCoreThreadId());
   service_worker_context->StartWorkerForScope(
       context_id.service_worker_scope(),
-      base::BindOnce(&ServiceWorkerTaskQueue::DidStartWorkerForScopeOnIO,
-                     context_id, std::move(task_queue_weak)),
+      base::BindOnce(
+          &ServiceWorkerTaskQueue::DidStartWorkerForScopeOnCoreThread,
+          context_id, std::move(task_queue_weak)),
       base::BindOnce(&DidStartWorkerFail));
 }
 
@@ -239,12 +250,17 @@ void ServiceWorkerTaskQueue::RunTasksAfterStartWorker(
   content::ServiceWorkerContext* service_worker_context =
       partition->GetServiceWorkerContext();
 
-  content::ServiceWorkerContext::RunTask(
-      base::CreateSingleThreadTaskRunner({content::BrowserThread::IO}),
-      FROM_HERE, service_worker_context,
-      base::BindOnce(&ServiceWorkerTaskQueue::StartServiceWorkerOnIOToRunTasks,
-                     weak_factory_.GetWeakPtr(), context_id,
-                     service_worker_context));
+  if (content::ServiceWorkerContext::IsServiceWorkerOnUIEnabled()) {
+    StartServiceWorkerOnCoreThreadToRunTasks(
+        weak_factory_.GetWeakPtr(), context_id, service_worker_context);
+  } else {
+    content::ServiceWorkerContext::RunTask(
+        base::CreateSingleThreadTaskRunner({content::BrowserThread::IO}),
+        FROM_HERE, service_worker_context,
+        base::BindOnce(
+            &ServiceWorkerTaskQueue::StartServiceWorkerOnCoreThreadToRunTasks,
+            weak_factory_.GetWeakPtr(), context_id, service_worker_context));
+  }
 }
 
 void ServiceWorkerTaskQueue::DidRegisterServiceWorker(
@@ -273,7 +289,7 @@ void ServiceWorkerTaskQueue::DidRegisterServiceWorker(
   if (!pending_tasks_[context_id].empty()) {
     // TODO(lazyboy): If worker for |context_id| is already running, consider
     // not calling StartWorker. This isn't straightforward as service worker's
-    // internal state is mostly on IO thread.
+    // internal state is mostly on the core thread.
     RunTasksAfterStartWorker(context_id);
   }
 }

extensions/browser/service_worker_task_queue.h

@@ -92,13 +92,13 @@ class ServiceWorkerTaskQueue : public KeyedService,
   static void SetObserverForTest(TestObserver* observer);
 
  private:
-  static void DidStartWorkerForScopeOnIO(
+  static void DidStartWorkerForScopeOnCoreThread(
       const LazyContextId& context_id,
       base::WeakPtr<ServiceWorkerTaskQueue> task_queue,
       int64_t version_id,
       int process_id,
       int thread_id);
-  static void StartServiceWorkerOnIOToRunTasks(
+  static void StartServiceWorkerOnCoreThreadToRunTasks(
       base::WeakPtr<ServiceWorkerTaskQueue> task_queue_weak,
       const LazyContextId& context_id,
       content::ServiceWorkerContext* service_worker_context);