Fix crash during EmbeddedWorkerInstance startup sequence failures

content/browser/service_worker/embedded_worker_instance_unittest.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/basictypes.h"
 #include "base/run_loop.h"
 #include "base/stl_util.h"
 #include "content/browser/service_worker/embedded_worker_instance.h"
 #include "content/browser/service_worker/embedded_worker_registry.h"
 #include "content/browser/service_worker/embedded_worker_test_helper.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
 #include "content/browser/service_worker/service_worker_context_wrapper.h"
 #include "content/common/service_worker/embedded_worker_messages.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace content {
 
 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,
                        const base::Closure& callback,
                        ServiceWorkerStatusCode status) {
   *out = status;
   callback.Run();
 }
 
 }  // namespace
 
 class EmbeddedWorkerInstanceTest : public testing::Test,
                                    public EmbeddedWorkerInstance::Listener {
  protected:
   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;
   }
 
   bool OnMessageReceived(const IPC::Message& message) override { return false; }
 
   void SetUp() override {
     helper_.reset(
         new EmbeddedWorkerTestHelper(base::FilePath(), kRenderProcessId));
@@ skipping 19 matching lines @@
     return context()->embedded_worker_registry();
   }
 
   IPC::TestSink* ipc_sink() { return helper_->ipc_sink(); }
 
   TestBrowserThreadBundle thread_bundle_;
   scoped_ptr<EmbeddedWorkerTestHelper> helper_;
   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();
   EXPECT_EQ(EmbeddedWorkerInstance::STOPPED, worker->status());
 
   const int64 service_worker_version_id = 55L;
   const GURL pattern("http://example.com/");
   const GURL url("http://example.com/worker.js");
 
   // Simulate adding one process to the pattern.
@@ skipping 124 matching lines @@
   EmbeddedWorkerRegistry* registry =
       helper_->context()->embedded_worker_registry();
   EXPECT_EQ(0UL,
             registry->worker_process_map_[kRenderProcessId].count(worker1_id));
   EXPECT_EQ(1UL, registry->worker_process_map_[kRenderProcessId].count(
                      worker2->embedded_worker_id()));
 
   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);
 }
 
 // Test stopping in the middle of the start worker sequence, before
 // a process is allocated.
 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;
   worker->Stop();
   EXPECT_EQ(EmbeddedWorkerInstance::STOPPED, worker->status());
   EXPECT_TRUE(detached_);
   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