Remove a bunch of pointless null checks in FrameLoaderClientImpl.

third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp

Index: third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp
diff --git a/third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp b/third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp
index ea59922ffa6cb579772311f0a099b72daa59283f..406e24178b3a5362c4220074cb552eca249c4315 100644
--- a/third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp
+++ b/third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp
@@ -93,11 +93,9 @@ WebSharedWorkerImpl::WebSharedWorkerImpl(WebSharedWorkerClient* client)
 
 WebSharedWorkerImpl::~WebSharedWorkerImpl() {
   DCHECK(m_webView);
-  // Detach the client before closing the view to avoid getting called back.
-  m_mainFrame->setClient(0);
 
   m_webView->close();
-  m_mainFrame->close();
+
   if (m_loaderProxy)
     m_loaderProxy->detachProvider(this);
 }
@@ -147,6 +145,8 @@ void WebSharedWorkerImpl::initializeLoader() {
 
 WebApplicationCacheHost* WebSharedWorkerImpl::createApplicationCacheHost(
     WebApplicationCacheHostClient* appcacheHostClient) {
+  if (m_askedToTerminate)
+    return nullptr;
   return m_client->createApplicationCacheHost(appcacheHostClient);
 }
 
@@ -163,11 +163,17 @@ void WebSharedWorkerImpl::loadShadowPage() {
 
 void WebSharedWorkerImpl::willSendRequest(WebLocalFrame* frame,
                                           WebURLRequest& request) {
+  if (m_askedToTerminate)
+    return;
+
   if (m_networkProvider)
     m_networkProvider->willSendRequest(frame->dataSource(), request);
 }
 
 void WebSharedWorkerImpl::didFinishDocumentLoad(WebLocalFrame* frame) {
+  if (m_askedToTerminate)

[dcheng, 2016/10/17 22:33:29]

Similarly, I've added early returns. It feels a bit weird to need these: I'm
hoping that not invoking (most of) the successful loading callbacks if a
Document hasn't otherwise completed loading during Frame::detach might improve
this...

+    return;
+
   DCHECK(!m_loadingDocument);
   DCHECK(!m_mainScriptLoader);
   m_networkProvider = wrapUnique(
@@ -189,17 +195,22 @@ void WebSharedWorkerImpl::didFinishDocumentLoad(WebLocalFrame* frame) {
 
 bool WebSharedWorkerImpl::isControlledByServiceWorker(
     WebDataSource& dataSource) {
+  if (m_askedToTerminate)

[esprehn, 2016/10/24 21:27:07]

should m_networkProvider be nulled out once this is true?

+    return false;
+
   return m_networkProvider &&
          m_networkProvider->isControlledByServiceWorker(dataSource);
 }
 
 int64_t WebSharedWorkerImpl::serviceWorkerID(WebDataSource& dataSource) {
-  if (!m_networkProvider)
+  if (m_askedToTerminate || !m_networkProvider)
     return -1;
   return m_networkProvider->serviceWorkerID(dataSource);

[esprehn, 2016/10/24 21:27:07]

ditto?

 }
 
 InterfaceProvider* WebSharedWorkerImpl::interfaceProvider() {
+  if (m_askedToTerminate)
+    return nullptr;

[esprehn, 2016/10/24 21:27:07]

Can we return getEmptyInterfaceProvider() instead? That avoids null checks on
interfaceProvider() which should ideally never return null

   return Platform::current()->interfaceProvider();
 }