Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(167)

Unified Diff: content/browser/service_worker/embedded_worker_instance_unittest.cc

Issue 1327723005: Fix crash during EmbeddedWorkerInstance startup sequence failures (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: added unittests Created 5 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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
+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

Powered by Google App Engine
This is Rietveld 408576698