Check whether the shadow page has been loaded in WebEmbeddedWorkerImpl::terminateWorkerContext().

Source/web/WebEmbeddedWorkerImpl.cpp

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 125 matching lines @@
     DEFINE_STATIC_LOCAL(HashSet<WebEmbeddedWorkerImpl*>, set, ());
     return set;
 }
 
 WebEmbeddedWorkerImpl::WebEmbeddedWorkerImpl(PassOwnPtr<WebServiceWorkerContextClient> client, PassOwnPtr<WebWorkerContentSettingsClientProxy> ContentSettingsClient)
     : m_workerContextClient(client)
     , m_contentSettingsClient(ContentSettingsClient)
     , m_workerInspectorProxy(WorkerInspectorProxy::create())
     , m_webView(0)
     , m_mainFrame(0)
+    , m_loadingShadowPage(false)
     , m_askedToTerminate(false)
     , m_pauseAfterDownloadState(DontPauseAfterDownload)
     , m_waitingForDebuggerState(NotWaitingForDebugger)
 {
     runningWorkerInstances().add(this);
 }
 
 WebEmbeddedWorkerImpl::~WebEmbeddedWorkerImpl()
 {
     // Prevent onScriptLoaderFinished from deleting 'this'.
     m_askedToTerminate = true;

[falken, 2015/06/16 06:48:15]

Maybe we also need a workerContextFailedtoStart here if m_loadingShadowPage is
true?

[horo, 2015/06/16 07:21:52]

No.
This destructor is called from workerContextFailedToStart() when
m_loadingShadowPage is true.

ServiceWorkerContextClient::workerContextFailedToStart()
 -> EmbeddedWorkerDispatcher::WorkerContextDestroyed()
 -> EmbeddedWorkerDispatcher::WorkerWrapper::~WorkerWrapper()
 -> WebEmbeddedWorkerImpl::~WebEmbeddedWorkerImpl()

 
     if (m_workerThread)
         m_workerThread->terminateAndWait();
 
     ASSERT(runningWorkerInstances().contains(this));
     runningWorkerInstances().remove(this);
     ASSERT(m_webView);
 
     // Detach the client before closing the view to avoid getting called back.
     m_mainFrame->setClient(0);
@@ skipping 22 matching lines @@
     if (data.pauseAfterDownloadMode == WebEmbeddedWorkerStartData::PauseAfterDownload)
         m_pauseAfterDownloadState = DoPauseAfterDownload;
     prepareShadowPageForLoader();
 }
 
 void WebEmbeddedWorkerImpl::terminateWorkerContext()
 {
     if (m_askedToTerminate)
         return;
     m_askedToTerminate = true;
+    if (m_loadingShadowPage) {
+        // This may delete 'this'.
+        m_workerContextClient->workerContextFailedToStart();
+        return;
+    }
     if (m_mainScriptLoader) {
         m_mainScriptLoader->cancel();
         m_mainScriptLoader.clear();
         // This may delete 'this'.
         m_workerContextClient->workerContextFailedToStart();
         return;
     }
     if (m_pauseAfterDownloadState == IsPausedAfterDownload) {
         // This may delete 'this'.
         m_workerContextClient->workerContextFailedToStart();
@@ skipping 110 matching lines @@
     loadShadowPage();
 }
 
 void WebEmbeddedWorkerImpl::loadShadowPage()
 {
     // Construct substitute data source for the 'shadow page'. We only need it
     // to have same origin as the worker so the loading checks work correctly.
     CString content("");
     int length = static_cast<int>(content.length());
     RefPtr<SharedBuffer> buffer(SharedBuffer::create(content.data(), length));
+    m_loadingShadowPage = true;
     m_mainFrame->frame()->loader().load(FrameLoadRequest(0, ResourceRequest(m_workerStartData.scriptURL), SubstituteData(buffer, "text/html", "UTF-8", KURL())));
 }
 
 void WebEmbeddedWorkerImpl::willSendRequest(
     WebLocalFrame* frame, unsigned, WebURLRequest& request,
     const WebURLResponse& redirectResponse)
 {
     if (m_networkProvider)
         m_networkProvider->willSendRequest(frame->dataSource(), request);
 }
 
 void WebEmbeddedWorkerImpl::didFinishDocumentLoad(WebLocalFrame* frame)
 {
     ASSERT(!m_mainScriptLoader);
     ASSERT(!m_networkProvider);
     ASSERT(m_mainFrame);
     ASSERT(m_workerContextClient);

[falken, 2015/06/16 06:48:15]

Should we deal with m_askedToTerminate here? As I understand it, the bug is that
terminate() comes in between loadShadowPage() and didFinishDocumentLoad(). In
that case we'd continue on loading and then bail out in onScriptLoaderFinished.

[horo, 2015/06/16 07:21:52]

Added ASSERT().
This method will not be called after we call "m_mainFrame->setClient(0);" in the
destructor.

+    m_loadingShadowPage = false;
     m_networkProvider = adoptPtr(m_workerContextClient->createServiceWorkerNetworkProvider(frame->dataSource()));
     m_mainScriptLoader = Loader::create();
     m_mainScriptLoader->load(
         m_mainFrame->frame()->document(),
         m_workerStartData.scriptURL,
         bind(&WebEmbeddedWorkerImpl::onScriptLoaderFinished, this));
 }
 
 void WebEmbeddedWorkerImpl::sendProtocolMessage(int callId, const WebString& message, const WebString& state)
 {
@@ skipping 73 matching lines @@
     m_mainScriptLoader.clear();
 
     m_workerGlobalScopeProxy = ServiceWorkerGlobalScopeProxy::create(*this, *document, *m_workerContextClient);
     m_loaderProxy = WorkerLoaderProxy::create(this);
     m_workerThread = ServiceWorkerThread::create(m_loaderProxy, *m_workerGlobalScopeProxy);
     m_workerThread->start(startupData.release());
     m_workerInspectorProxy->workerThreadCreated(document, m_workerThread.get(), scriptURL);
 }
 
 } // namespace blink