service worker: Don't BadMessageReceived when a response for a timed …

…out request arrives

If the worker is stopping, assume an unhandled message is due to the
request timing out which triggered the stopping.

I considered saving timed out request ids but it's a bit cumbersome: you
have to store the IPC message type along with the request id which involves
some changes to PendingRequest along with the data structure to hold
the timed out requests. Seems more trouble than it's worth to avoid this bug,
and we expect to migrate to Mojo soon anyway.

BUG=625040

Review-Url: https://codereview.chromium.org/2150863002
Cr-Commit-Position: refs/heads/master@{#405477}

content/browser/service_worker/embedded_worker_registry.cc

@@ -61,7 +61,14 @@ bool EmbeddedWorkerRegistry::OnMessageReceived(const IPC::Message& message,
     // purposely handling the message as no-op.
     return true;
   }
-  return worker->OnMessageReceived(message);
+  bool handled = worker->OnMessageReceived(message);
+
+  // Assume an unhandled message for a stopping worker is because the message
+  // was timed out and its handler removed prior to stopping.
+  // We might be more precise and record timed out request ids, but some
+  // cumbersome bookkeeping is needed and the IPC messaging will soon migrate
+  // to Mojo anyway.
+  return handled || worker->status() == EmbeddedWorkerStatus::STOPPING;
 }
 
 void EmbeddedWorkerRegistry::Shutdown() {

content/browser/service_worker/service_worker_dispatcher_host_unittest.cc

@@ -23,6 +23,7 @@
 #include "content/browser/service_worker/service_worker_handle.h"
 #include "content/common/service_worker/embedded_worker_messages.h"
 #include "content/common/service_worker/service_worker_messages.h"
+#include "content/common/service_worker/service_worker_utils.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/test/mock_resource_context.h"
 #include "content/public/test/test_browser_thread_bundle.h"
@@ -769,4 +770,30 @@ TEST_F(ServiceWorkerDispatcherHostTest, OnSetHostedVersionId) {
       ServiceWorkerMsg_AssociateRegistration::ID));
 }
 
+TEST_F(ServiceWorkerDispatcherHostTest, ReceivedTimedOutRequestResponse) {
+  GURL pattern = GURL("https://www.example.com/");
+  GURL script_url = GURL("https://www.example.com/service_worker.js");
+
+  SendProviderCreated(SERVICE_WORKER_PROVIDER_FOR_WINDOW, pattern);
+  SetUpRegistration(pattern, script_url);
+
+  version_->StartWorker(ServiceWorkerMetrics::EventType::UNKNOWN,
+                        base::Bind(&ServiceWorkerUtils::NoOpStatusCallback));
+  base::RunLoop().RunUntilIdle();
+
+  // Set the worker status to STOPPING.
+  version_->embedded_worker()->Stop();
+  EXPECT_EQ(EmbeddedWorkerStatus::STOPPING, version_->running_status());
+
+  // Receive a response for a timed out request. The bad message count should
+  // not increase.
+  const int kRequestId = 91;  // Dummy value
+  dispatcher_host_->OnMessageReceived(ServiceWorkerHostMsg_FetchEventResponse(
+      version_->embedded_worker()->embedded_worker_id(), kRequestId,
+      SERVICE_WORKER_FETCH_EVENT_RESULT_FALLBACK, ServiceWorkerResponse()));
+
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(0, dispatcher_host_->bad_messages_received_count_);
+}
+
 }  // namespace content