[ESW] Ignore FinishExternalRequest result for stopped worker's event ack.

If a worker stops right after it sends an extension event ack,
decrementing ref count (FinishExternalRequest) can fail in the
browser process. This happens if the browser process has already dropped
knowledge about the worker.

Ignore FinishExternalRequest's result in this case, after examining
the worker's status in ProcessManager. This reduces flaky failures
of ServiceWorkerTest.Update(Un)PackedExtension tests.

Bug: 999027
Change-Id: Iad0287067eff0feab7cf64ff21768c204009969c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1832491
Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: David Bertoni <dbertoni@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706082}

extensions/browser/events/event_ack_data.cc

@@ -78,6 +78,7 @@ void EventAckData::FinishExternalRequestOnCoreThread(
     int render_process_id,
     int64_t version_id,
     int event_id,
+    bool worker_stopped,
     scoped_refptr<CoreThreadEventInfo> unacked_events,
     base::OnceClosure failure_callback) {
   DCHECK_CURRENTLY_ON(content::ServiceWorkerContext::GetCoreThreadId());
@@ -95,6 +96,11 @@ void EventAckData::FinishExternalRequestOnCoreThread(
 
   content::ServiceWorkerExternalRequestResult result =
       context->FinishedExternalRequest(version_id, request_uuid);
+  // If the worker was already stopped, the FinishedExternalRequest will
+  // legitimately fail.
+  if (worker_stopped)
+    return;
+
   if (result != content::ServiceWorkerExternalRequestResult::kOk) {
     LOG(ERROR) << "FinishExternalRequest failed: " << static_cast<int>(result);
     std::move(failure_callback).Run();
@@ -130,12 +136,13 @@ void EventAckData::DecrementInflightEvent(
     int render_process_id,
     int64_t version_id,
     int event_id,
+    bool worker_stopped,
     base::OnceClosure failure_callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   if (content::ServiceWorkerContext::IsServiceWorkerOnUIEnabled()) {
     FinishExternalRequestOnCoreThread(context, render_process_id, version_id,
-                                      event_id, unacked_events_,
+                                      event_id, worker_stopped, unacked_events_,
                                       std::move(failure_callback));
   } else {
     content::ServiceWorkerContext::RunTask(
@@ -143,7 +150,8 @@ void EventAckData::DecrementInflightEvent(
         FROM_HERE, context,
         base::BindOnce(&EventAckData::FinishExternalRequestOnCoreThread,
                        context, render_process_id, version_id, event_id,
-                       unacked_events_, std::move(failure_callback)));
+                       worker_stopped, unacked_events_,
+                       std::move(failure_callback)));
   }
 }
 

extensions/browser/events/event_ack_data.h

@@ -35,6 +35,7 @@ class EventAckData {
                               int render_process_id,
                               int64_t version_id,
                               int event_id,
+                              bool worker_stopped,
                               base::OnceClosure failure_callback);
 
  private:
@@ -52,6 +53,7 @@ class EventAckData {
       int render_process_id,
       int64_t version_id,
       int event_id,
+      bool worker_stopped,
       scoped_refptr<CoreThreadEventInfo> unacked_events,
       base::OnceClosure failure_callback);
 

extensions/browser/extension_service_worker_message_filter.cc

@@ -11,6 +11,7 @@
 #include "extensions/browser/event_router.h"
 #include "extensions/browser/events/event_ack_data.h"
 #include "extensions/browser/extension_function_dispatcher.h"
+#include "extensions/browser/process_manager.h"
 #include "extensions/browser/service_worker_task_queue.h"
 #include "extensions/common/extension_messages.h"
 
@@ -110,14 +111,20 @@ void ExtensionServiceWorkerMessageFilter::OnDecrementServiceWorkerActivity(
 }
 
 void ExtensionServiceWorkerMessageFilter::OnEventAckWorker(
+    const ExtensionId& extension_id,
     int64_t service_worker_version_id,
+    int worker_thread_id,
     int event_id) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  const bool worker_stopped =
+      !ProcessManager::Get(browser_context_)
+           ->HasServiceWorker({extension_id, render_process_id_,
+                               service_worker_version_id, worker_thread_id});
   EventRouter::Get(browser_context_)
       ->event_ack_data()
       ->DecrementInflightEvent(
           service_worker_context_, render_process_id_,
-          service_worker_version_id, event_id,
+          service_worker_version_id, event_id, worker_stopped,
           base::BindOnce(&ExtensionServiceWorkerMessageFilter::
                              DidFailDecrementInflightEvent,
                          this));

extensions/browser/extension_service_worker_message_filter.h

@@ -48,7 +48,10 @@ class ExtensionServiceWorkerMessageFilter
                                         const std::string& request_uuid);
   void OnDecrementServiceWorkerActivity(int64_t service_worker_version_id,
                                         const std::string& request_uuid);
-  void OnEventAckWorker(int64_t service_worker_version_id, int event_id);
+  void OnEventAckWorker(const ExtensionId& extension_id,
+                        int64_t service_worker_version_id,
+                        int thread_id,
+                        int event_id);
   void OnDidInitializeServiceWorkerContext(const ExtensionId& extension_id,
                                            int64_t service_worker_version_id,
                                            int thread_id);

extensions/common/extension_messages.h

@@ -1026,8 +1026,10 @@ IPC_MESSAGE_CONTROL2(ExtensionHostMsg_DecrementServiceWorkerActivity,
 
 // Tells the browser that an event with |event_id| was successfully dispatched
 // to the worker with version |service_worker_version_id|.
-IPC_MESSAGE_CONTROL2(ExtensionHostMsg_EventAckWorker,
+IPC_MESSAGE_CONTROL4(ExtensionHostMsg_EventAckWorker,
+                     std::string /* extension_id */,
                      int64_t /* service_worker_version_id */,
+                     int /* worker_thread_id */,
                      int /* event_id */)
 
 // Tells the browser that an extension service worker context was initialized,

extensions/renderer/worker_thread_dispatcher.cc

@@ -208,8 +208,10 @@ void WorkerThreadDispatcher::OnDispatchEvent(
   }
   data->bindings_system()->DispatchEventInContext(
       params.event_name, &event_args, &params.filtering_info, data->context());
-  Send(new ExtensionHostMsg_EventAckWorker(data->service_worker_version_id(),
-                                           params.event_id));
+  const int worker_thread_id = content::WorkerThread::GetCurrentId();
+  Send(new ExtensionHostMsg_EventAckWorker(data->context()->GetExtensionID(),
+                                           data->service_worker_version_id(),
+                                           worker_thread_id, params.event_id));
 }
 
 void WorkerThreadDispatcher::OnDispatchOnConnect(