Refine error code returned by ServiceWorkerVersion::StartWorker (e.g. PROCESS_NOT_FOUND etc)

BUG=313530

Review URL: https://codereview.chromium.org/147593006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@247845 0039d316-1c4b-4281-b951-d872f2087c98

content/browser/service_worker/embedded_worker_instance.cc

@@ -15,34 +15,36 @@ EmbeddedWorkerInstance::~EmbeddedWorkerInstance() {
   registry_->RemoveWorker(process_id_, embedded_worker_id_);
 }
 
-bool EmbeddedWorkerInstance::Start(
+ServiceWorkerStatusCode EmbeddedWorkerInstance::Start(
     int64 service_worker_version_id,
     const GURL& script_url) {
   DCHECK(status_ == STOPPED);
   if (!ChooseProcess())
-    return false;
+    return SERVICE_WORKER_ERROR_PROCESS_NOT_FOUND;
   status_ = STARTING;
-  bool success = registry_->StartWorker(
+  ServiceWorkerStatusCode status = registry_->StartWorker(
       process_id_,
       embedded_worker_id_,
       service_worker_version_id,
       script_url);
-  if (!success) {
+  if (status != SERVICE_WORKER_OK) {
     status_ = STOPPED;
     process_id_ = -1;
   }
-  return success;
+  return status;
 }
 
-bool EmbeddedWorkerInstance::Stop() {
+ServiceWorkerStatusCode EmbeddedWorkerInstance::Stop() {
   DCHECK(status_ == STARTING || status_ == RUNNING);
-  const bool success = registry_->StopWorker(process_id_, embedded_worker_id_);
-  if (success)
+  ServiceWorkerStatusCode status =
+      registry_->StopWorker(process_id_, embedded_worker_id_);
+  if (status == SERVICE_WORKER_OK)
     status_ = STOPPING;
-  return success;
+  return status;
 }
 
-bool EmbeddedWorkerInstance::SendMessage(const IPC::Message& message) {
+ServiceWorkerStatusCode EmbeddedWorkerInstance::SendMessage(
+    const IPC::Message& message) {
   DCHECK(status_ == RUNNING);
   return registry_->Send(process_id_,
                          new EmbeddedWorkerContextMsg_SendMessageToWorker(

content/browser/service_worker/embedded_worker_instance.h

@@ -14,6 +14,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/observer_list.h"
 #include "content/common/content_export.h"
+#include "content/common/service_worker/service_worker_status_code.h"
 
 class GURL;
 
@@ -50,21 +51,18 @@ class CONTENT_EXPORT EmbeddedWorkerInstance {
 
   // Starts the worker. It is invalid to call this when the worker is
   // not in STOPPED status.
-  // This returns false if starting a worker fails immediately, e.g. when
-  // IPC couldn't be sent to the worker or no process was available.
-  bool Start(int64 service_worker_version_id,
-             const GURL& script_url);
+  ServiceWorkerStatusCode Start(int64 service_worker_version_id,
+                                const GURL& script_url);
 
   // Stops the worker. It is invalid to call this when the worker is
   // not in STARTING or RUNNING status.
   // This returns false if stopping a worker fails immediately, e.g. when
   // IPC couldn't be sent to the worker.
-  bool Stop();
+  ServiceWorkerStatusCode Stop();
 
   // Sends |message| to the embedded worker running in the child process.
-  // This returns false if sending IPC fails.
   // It is invalid to call this while the worker is not in RUNNING status.
-  bool SendMessage(const IPC::Message& message);
+  ServiceWorkerStatusCode SendMessage(const IPC::Message& message);
 
   // Add or remove |process_id| to the internal process set where this
   // worker can be started.

content/browser/service_worker/embedded_worker_instance_unittest.cc

@@ -55,14 +55,16 @@ TEST_F(EmbeddedWorkerInstanceTest, StartAndStop) {
   const GURL url("http://example.com/worker.js");
 
   // This fails as we have no available process yet.
-  EXPECT_FALSE(worker->Start(service_worker_version_id, url));
+  EXPECT_EQ(SERVICE_WORKER_ERROR_PROCESS_NOT_FOUND,
+            worker->Start(service_worker_version_id, url));
   EXPECT_EQ(EmbeddedWorkerInstance::STOPPED, worker->status());
 
   // Simulate adding one process to the worker.
   helper_->SimulateAddProcess(embedded_worker_id, process_id);
 
   // Start should succeed.
-  EXPECT_TRUE(worker->Start(service_worker_version_id, url));
+  EXPECT_EQ(SERVICE_WORKER_OK,
+            worker->Start(service_worker_version_id, url));
   EXPECT_EQ(EmbeddedWorkerInstance::STARTING, worker->status());
   base::RunLoop().RunUntilIdle();
 
@@ -71,7 +73,7 @@ TEST_F(EmbeddedWorkerInstanceTest, StartAndStop) {
   EXPECT_EQ(process_id, worker->process_id());
 
   // Stop the worker.
-  EXPECT_TRUE(worker->Stop());
+  EXPECT_EQ(SERVICE_WORKER_OK, worker->Stop());
   EXPECT_EQ(EmbeddedWorkerInstance::STOPPING, worker->status());
   base::RunLoop().RunUntilIdle();
 
@@ -101,7 +103,8 @@ TEST_F(EmbeddedWorkerInstanceTest, ChooseProcess) {
   helper_->SimulateAddProcess(embedded_worker_id, 3);
 
   // Process 3 has the biggest # of references and it should be chosen.
-  EXPECT_TRUE(worker->Start(1L, GURL("http://example.com/worker.js")));
+  EXPECT_EQ(SERVICE_WORKER_OK,
+            worker->Start(1L, GURL("http://example.com/worker.js")));
   EXPECT_EQ(EmbeddedWorkerInstance::STARTING, worker->status());
   EXPECT_EQ(3, worker->process_id());
 

content/browser/service_worker/embedded_worker_registry.cc

@@ -25,7 +25,7 @@ scoped_ptr<EmbeddedWorkerInstance> EmbeddedWorkerRegistry::CreateWorker() {
   return worker.Pass();
 }
 
-bool EmbeddedWorkerRegistry::StartWorker(
+ServiceWorkerStatusCode EmbeddedWorkerRegistry::StartWorker(
     int process_id,
     int embedded_worker_id,
     int64 service_worker_version_id,
@@ -36,8 +36,8 @@ bool EmbeddedWorkerRegistry::StartWorker(
                                                script_url));
 }
 
-bool EmbeddedWorkerRegistry::StopWorker(int process_id,
-                                        int embedded_worker_id) {
+ServiceWorkerStatusCode EmbeddedWorkerRegistry::StopWorker(
+    int process_id, int embedded_worker_id) {
   return Send(process_id,
               new EmbeddedWorkerMsg_StopWorker(embedded_worker_id));
 }
@@ -107,13 +107,16 @@ EmbeddedWorkerInstance* EmbeddedWorkerRegistry::GetWorker(
 
 EmbeddedWorkerRegistry::~EmbeddedWorkerRegistry() {}
 
-bool EmbeddedWorkerRegistry::Send(int process_id, IPC::Message* message) {
+ServiceWorkerStatusCode EmbeddedWorkerRegistry::Send(
+    int process_id, IPC::Message* message) {
   if (!context_)
-    return false;
+    return SERVICE_WORKER_ERROR_ABORT;
   ProcessToSenderMap::iterator found = process_sender_map_.find(process_id);
   if (found == process_sender_map_.end())
-    return false;
-  return found->second->Send(message);
+    return SERVICE_WORKER_ERROR_PROCESS_NOT_FOUND;
+  if (!found->second->Send(message))
+    return SERVICE_WORKER_ERROR_IPC_FAILED;
+  return SERVICE_WORKER_OK;
 }
 
 void EmbeddedWorkerRegistry::RemoveWorker(int process_id,

content/browser/service_worker/embedded_worker_registry.h

@@ -12,6 +12,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "content/common/content_export.h"
+#include "content/common/service_worker/service_worker_status_code.h"
 
 class GURL;
 
@@ -41,12 +42,12 @@ class CONTENT_EXPORT EmbeddedWorkerRegistry
   scoped_ptr<EmbeddedWorkerInstance> CreateWorker();
 
   // Called from EmbeddedWorkerInstance, relayed to the child process.
-  bool StartWorker(int process_id,
-                   int embedded_worker_id,
-                   int64 service_worker_version_id,
-                   const GURL& script_url);
-  bool StopWorker(int process_id,
-                  int embedded_worker_id);
+  ServiceWorkerStatusCode StartWorker(int process_id,
+                                      int embedded_worker_id,
+                                      int64 service_worker_version_id,
+                                      const GURL& script_url);
+  ServiceWorkerStatusCode StopWorker(int process_id,
+                                     int embedded_worker_id);
 
   // Called back from EmbeddedWorker in the child process, relayed via
   // ServiceWorkerDispatcherHost.
@@ -70,7 +71,7 @@ class CONTENT_EXPORT EmbeddedWorkerRegistry
   typedef std::map<int, IPC::Sender*> ProcessToSenderMap;
 
   ~EmbeddedWorkerRegistry();
-  bool Send(int process_id, IPC::Message* message);
+  ServiceWorkerStatusCode Send(int process_id, IPC::Message* message);
 
   // RemoveWorker is called when EmbeddedWorkerInstance is destructed.
   // |process_id| could be invalid (i.e. -1) if it's not running.

content/browser/service_worker/service_worker_browsertest.cc

@@ -131,14 +131,14 @@ class EmbeddedWorkerBrowserTest : public ServiceWorkerBrowserTest,
     const int64 service_worker_version_id = 33L;
     const GURL script_url = embedded_test_server()->GetURL(
         "/service_worker/worker.js");
-    const bool started = worker_->Start(
+    ServiceWorkerStatusCode status = worker_->Start(
         service_worker_version_id, script_url);
 
     last_worker_status_ = worker_->status();
-    EXPECT_TRUE(started);
+    EXPECT_EQ(SERVICE_WORKER_OK, status);
     EXPECT_EQ(EmbeddedWorkerInstance::STARTING, last_worker_status_);
 
-    if (!started && !done_closure_.is_null())
+    if (status != SERVICE_WORKER_OK && !done_closure_.is_null())
       done_closure_.Run();
   }
 
@@ -146,13 +146,13 @@ class EmbeddedWorkerBrowserTest : public ServiceWorkerBrowserTest,
     ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::IO));
     EXPECT_EQ(EmbeddedWorkerInstance::RUNNING, worker_->status());
 
-    const bool stopped = worker_->Stop();
+    ServiceWorkerStatusCode status = worker_->Stop();
 
     last_worker_status_ = worker_->status();
-    EXPECT_TRUE(stopped);
+    EXPECT_EQ(SERVICE_WORKER_OK, status);
     EXPECT_EQ(EmbeddedWorkerInstance::STOPPING, last_worker_status_);
 
-    if (!stopped && !done_closure_.is_null())
+    if (status != SERVICE_WORKER_OK && !done_closure_.is_null())
       done_closure_.Run();
   }
 

content/browser/service_worker/service_worker_registration_status.cc

@@ -32,6 +32,7 @@ void GetServiceWorkerRegistrationStatusResponse(
       return;
 
     case SERVICE_WORKER_ERROR_ABORT:
+    case SERVICE_WORKER_ERROR_IPC_FAILED:
     case SERVICE_WORKER_ERROR_FAILED:
     case SERVICE_WORKER_ERROR_PROCESS_NOT_FOUND:
       // Unexpected, or should bail out before calling this, or we don't

content/browser/service_worker/service_worker_version.cc

@@ -130,12 +130,12 @@ void ServiceWorkerVersion::StartWorker(const StatusCallback& callback) {
     return;
   }
   observer_.reset(new StartObserver(this, callback));
-  const bool started = embedded_worker_->Start(
+  ServiceWorkerStatusCode status = embedded_worker_->Start(
       version_id_,
       registration_->script_url());
-  if (!started) {
+  if (status != SERVICE_WORKER_OK) {
     observer_.reset();
-    RunSoon(base::Bind(callback, SERVICE_WORKER_ERROR_START_WORKER_FAILED));
+    RunSoon(base::Bind(callback, status));
   }
 }
 
@@ -147,10 +147,10 @@ void ServiceWorkerVersion::StopWorker(const StatusCallback& callback) {
     return;
   }
   observer_.reset(new StopObserver(this, callback));
-  const bool stopped = embedded_worker_->Stop();
-  if (!stopped) {
+  ServiceWorkerStatusCode status = embedded_worker_->Stop();
+  if (status != SERVICE_WORKER_OK) {
     observer_.reset();
-    RunSoon(base::Bind(callback, SERVICE_WORKER_ERROR_FAILED));
+    RunSoon(base::Bind(callback, status));
   }
 }
 
@@ -159,7 +159,7 @@ bool ServiceWorkerVersion::DispatchFetchEvent(
   if (status() != RUNNING)
     return false;
   return embedded_worker_->SendMessage(
-      ServiceWorkerMsg_FetchEvent(request));
+      ServiceWorkerMsg_FetchEvent(request)) == SERVICE_WORKER_OK;
 }
 
 void ServiceWorkerVersion::AddProcessToWorker(int process_id) {

content/common/service_worker/service_worker_status_code.cc

@@ -24,6 +24,8 @@ const char* ServiceWorkerStatusToString(ServiceWorkerStatusCode status) {
       return "ServiceWorker failed to install";
     case SERVICE_WORKER_ERROR_ACTIVATE_WORKER_FAILED:
       return "ServiceWorker failed to activate";
+    case SERVICE_WORKER_ERROR_IPC_FAILED:
+      return "IPC connection was closed or IPC error has occured";
   }
   NOTREACHED();
   return "";

content/common/service_worker/service_worker_status_code.h

@@ -32,6 +32,10 @@ enum ServiceWorkerStatusCode {
 
   // Activate event handling failed.
   SERVICE_WORKER_ERROR_ACTIVATE_WORKER_FAILED,
+
+  // Sending an IPC to the worker failed (often due to child process is
+  // terminated).
+  SERVICE_WORKER_ERROR_IPC_FAILED,
 };
 
 CONTENT_EXPORT const char* ServiceWorkerStatusToString(