Fix crash during EmbeddedWorkerInstance startup sequence failures

content/browser/service_worker/embedded_worker_instance_unittest.cc

Index: content/browser/service_worker/embedded_worker_instance_unittest.cc
diff --git a/content/browser/service_worker/embedded_worker_instance_unittest.cc b/content/browser/service_worker/embedded_worker_instance_unittest.cc
index 727fcca1f8208fd03ce9bba3f5d5ba1833509d3f..e61e2b393ae117fa07b6a20e23b5ea1ca4eb986a 100644
--- a/content/browser/service_worker/embedded_worker_instance_unittest.cc
+++ b/content/browser/service_worker/embedded_worker_instance_unittest.cc
@@ -21,8 +21,13 @@ namespace {
 
 const int kRenderProcessId = 11;
 
-void SaveStatus(ServiceWorkerStatusCode* out, ServiceWorkerStatusCode status) {
-  *out = status;
+// Takes a scoped_ptr<>* because the object is passed in via Bind() where Pass()
+// cannot be used.

[kinuko (google), 2015/09/10 22:14:19]

base::Passed() or base::Owned() works?

[falken, 2015/09/11 09:48:16]

Huh yes... I must have made a mistake on my first attempt. Done.

+void DestroyWorker(scoped_ptr<EmbeddedWorkerInstance>* worker,
+                   ServiceWorkerStatusCode* out_status,
+                   ServiceWorkerStatusCode status) {
+  *out_status = status;
+  worker->reset();
 }
 
 void SaveStatusAndCall(ServiceWorkerStatusCode* out,
@@ -40,6 +45,11 @@ class EmbeddedWorkerInstanceTest : public testing::Test,
   EmbeddedWorkerInstanceTest()
       : thread_bundle_(TestBrowserThreadBundle::IO_MAINLOOP) {}
 
+  void OnStopped(EmbeddedWorkerInstance::Status old_status) override {
+    stopped_ = true;
+    stopped_old_status_ = old_status;
+  }
+
   void OnDetached(EmbeddedWorkerInstance::Status old_status) override {
     detached_ = true;
     detached_old_status_ = old_status;
@@ -79,11 +89,22 @@ class EmbeddedWorkerInstanceTest : public testing::Test,
   bool detached_ = false;
   EmbeddedWorkerInstance::Status detached_old_status_ =
       EmbeddedWorkerInstance::STOPPED;
+  bool stopped_ = false;
+  EmbeddedWorkerInstance::Status stopped_old_status_ =
+      EmbeddedWorkerInstance::STOPPED;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(EmbeddedWorkerInstanceTest);
 };
 
+class FailToSendIPCHelper : public EmbeddedWorkerTestHelper {
+ public:
+  FailToSendIPCHelper()
+      : EmbeddedWorkerTestHelper(base::FilePath(), kRenderProcessId) {}
+  ~FailToSendIPCHelper() override {}
+  bool Send(IPC::Message* message) override { return false; }
+};
+
 TEST_F(EmbeddedWorkerInstanceTest, StartAndStop) {
   scoped_ptr<EmbeddedWorkerInstance> worker =
       embedded_worker_registry()->CreateWorker();
@@ -228,18 +249,23 @@ TEST_F(EmbeddedWorkerInstanceTest, RemoveWorkerInSharedProcess) {
   worker2->Stop();
 }
 
-// Test detaching in the middle of the start worker sequence.
+// Test detaching in the middle of the start worker sequence. Additionally, have
+// the start callback destroy the worker, as is usual when starting a worker
+// fails, and ensure a crash doesn't occur.
 TEST_F(EmbeddedWorkerInstanceTest, DetachDuringStart) {
   scoped_ptr<EmbeddedWorkerInstance> worker =
       embedded_worker_registry()->CreateWorker();
   scoped_ptr<EmbeddedWorkerMsg_StartWorker_Params> params(
       new EmbeddedWorkerMsg_StartWorker_Params());
   ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_FAILED;
+
   // Pretend we had a process allocated but then got detached before
   // the start sequence reached SendStartWorker.
-  worker->process_id_ = -1;
-  worker->SendStartWorker(params.Pass(), base::Bind(&SaveStatus, &status), true,
+  worker->status_ = EmbeddedWorkerInstance::STOPPED;
+  worker->SendStartWorker(params.Pass(),
+                          base::Bind(&DestroyWorker, &worker, &status), true,
                           -1, false);
+  // The callback destroys the worker, but we shouldn't have crashed.
   EXPECT_EQ(SERVICE_WORKER_ERROR_ABORT, status);
 }
 
@@ -249,8 +275,6 @@ TEST_F(EmbeddedWorkerInstanceTest, StopDuringStart) {
   scoped_ptr<EmbeddedWorkerInstance> worker =
       embedded_worker_registry()->CreateWorker();
   worker->AddListener(this);
-  scoped_ptr<EmbeddedWorkerMsg_StartWorker_Params> params(
-      new EmbeddedWorkerMsg_StartWorker_Params());
   // Pretend we stop during starting before we got a process allocated.
   worker->status_ = EmbeddedWorkerInstance::STARTING;
   worker->process_id_ = -1;
@@ -260,4 +284,31 @@ TEST_F(EmbeddedWorkerInstanceTest, StopDuringStart) {
   EXPECT_EQ(EmbeddedWorkerInstance::STARTING, detached_old_status_);
 }
 
+// Test for when sending the start IPC failed.
+TEST_F(EmbeddedWorkerInstanceTest, FailToSendStartIPC) {
+  helper_.reset(new FailToSendIPCHelper());
+
+  const int64 version_id = 55L;
+  const GURL pattern("http://example.com/");
+  const GURL url("http://example.com/worker.js");
+
+  scoped_ptr<EmbeddedWorkerInstance> worker =
+      embedded_worker_registry()->CreateWorker();
+  helper_->SimulateAddProcessToPattern(pattern, kRenderProcessId);
+  ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_FAILED;
+  worker->AddListener(this);
+
+  // Attempt to start the worker.
+  base::RunLoop run_loop;
+  worker->Start(
+      version_id, pattern, url,
+      base::Bind(&SaveStatusAndCall, &status, run_loop.QuitClosure()));
+  run_loop.Run();
+
+  // The callback should have run, and we should have got an OnStopped message.
+  EXPECT_EQ(SERVICE_WORKER_ERROR_IPC_FAILED, status);
+  EXPECT_TRUE(stopped_);
+  EXPECT_EQ(EmbeddedWorkerInstance::STARTING, stopped_old_status_);
+}
+
 }  // namespace content