Don't terminate SharedWorker while loading the script.

content/worker/websharedworker_stub.cc

 // Copyright (c) 2012 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 "content/worker/websharedworker_stub.h"
 
 #include "base/compiler_specific.h"
 #include "content/child/child_process.h"
 #include "content/child/child_thread.h"
 #include "content/child/fileapi/file_system_dispatcher.h"
 #include "content/child/shared_worker_devtools_agent.h"
 #include "content/child/webmessageportchannel_impl.h"
 #include "content/common/worker_messages.h"
 #include "content/worker/worker_thread.h"
 #include "third_party/WebKit/public/web/WebSharedWorker.h"
 #include "third_party/WebKit/public/platform/WebString.h"
 #include "third_party/WebKit/public/platform/WebURL.h"
 
 namespace content {
 
 WebSharedWorkerStub::WebSharedWorkerStub(
     const GURL& url,
     const base::string16& name,
     const base::string16& content_security_policy,
     blink::WebContentSecurityPolicyType security_policy_type,
     int route_id)
     : route_id_(route_id),
       client_(route_id, this),
       running_(false),
+      terminating_(false),
       url_(url) {
 
   WorkerThread* worker_thread = WorkerThread::current();
   DCHECK(worker_thread);
   worker_thread->AddWorkerStub(this);
   // Start processing incoming IPCs for this worker.
   worker_thread->AddRoute(route_id_, this);
 
   // TODO(atwilson): Add support for NaCl when they support MessagePorts.
   impl_ = blink::WebSharedWorker::create(client());
@@ skipping 55 matching lines @@
   } else {
     // If two documents try to load a SharedWorker at the same time, the
     // WorkerMsg_Connect for one of the documents can come in before the
     // worker is started. Just queue up the connect and deliver it once the
     // worker starts.
     pending_channels_.push_back(channel);
   }
 }
 
 void WebSharedWorkerStub::OnTerminateWorkerContext() {
-  impl_->terminateWorkerContext();
-
-  // Call the client to make sure context exits.
-  EnsureWorkerContextTerminates();
-  running_ = false;
+  if (running_) {
+    impl_->terminateWorkerContext();

[kinuko, 2014/02/19 04:24:08]

It feels like we should always nullify the impl_ after calling this?

Do we want to call this only when running_ is true?

If we call this before WorkerScriptLoaded is called WorkerScriptLoadFailed will
be called because m_askedToTerminate is set to true in WebSharedWorkerImpl...
was that not the case?

[horo, 2014/02/19 05:29:09]

The bug is caused by "delete this" in Shutdown() which may be called via
WorkerScriptLoaded() in the current stack.

I changed this cl to just move the position of
"impl_->terminateWorkerContext();".

+    // Call the client to make sure context exits.
+    EnsureWorkerContextTerminates();
+    running_ = false;
+  } else {
+    terminating_ = true;
+  }
 }
 
 void WebSharedWorkerStub::WorkerScriptLoaded() {
   running_ = true;
+  if (terminating_) {
+    OnTerminateWorkerContext();
+    return;
+  }
   // Process any pending connections.
   for (PendingChannelList::const_iterator iter = pending_channels_.begin();
        iter != pending_channels_.end();
        ++iter) {
     impl_->connect(*iter);
     WorkerThread::current()->Send(
         new WorkerHostMsg_WorkerConnected((*iter)->message_port_id(),
                                           route_id_));
   }
   pending_channels_.clear();
 }
 
 void WebSharedWorkerStub::WorkerScriptLoadFailed() {
   for (PendingChannelList::const_iterator iter = pending_channels_.begin();
        iter != pending_channels_.end();
        ++iter) {
     blink::WebMessagePortChannel* channel = *iter;
     channel->destroy();
   }
   pending_channels_.clear();
   Shutdown();
 }
 
 }  // namespace content