Fix nullptr deref in maybeRenderFallbackContent()

third_party/WebKit/Source/web/WebLocalFrameImpl.cpp

Index: third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
diff --git a/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp b/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
index 1943a85561a0a1532dd6762f7aa613ffa32afffd..b3f3867b8d56f7ff9897508ec262cfd1885ebef4 100644
--- a/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
+++ b/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
@@ -2078,8 +2078,8 @@ bool WebLocalFrameImpl::maybeRenderFallbackContent(
     return false;
 
   FrameLoader& frameloader = frame()->loader();
-  DCHECK(frameloader.provisionalDocumentLoader());
-  frameloader.loadFailed(frameloader.provisionalDocumentLoader(), error);
+  if (frameloader.provisionalDocumentLoader())

[arthursonzogni, 2017/03/31 09:16:33]

Can you add a comment that explain why the provisionalDocumentLoader have been
detached?

+    frameloader.loadFailed(frameloader.provisionalDocumentLoader(), error);
   return true;

[arthursonzogni, 2017/03/31 09:16:33]

Returning true here will not work. You can try to open the fallback.html page
you have made. The browser is still waiting the page to load the error page. You
can see the animated loading icon in the tab that is still running.

The FrameHostMsg_DidStopLoading should be sent to inform it that the load
stopped.

FYI: When a fallback content is displayed, this message will be sent because of
this list of call:
1) RenderFrameImpl::OnFailedNavigation()
2) WebLocalFrameImpl::maybeRenderFallbackContent()
3) Frameloader::loadFailed()
4) FrameLoader::checkCompleted()
5) ProgressTracker::progressCompleted()
6) LocalFrameClientImpl::didStopLoading()
7) RenderFrameImpl::didStopLoading()

If you don't call anything, the message will not be sent and the browser will
wait forever.

[Nate Chapin, 2017/03/31 23:59:59]

Is it safe to unconditionally call DidStopLoading here? It's hypothetically
possible (though surely very unlikely in practice) to end up in a situation
where the committed DocumentLoader is loading, but the provisional
DocumentLoader has been clobbered. In that case, we don't want to replace the
content with an error page, but at the same time the DCHECK is wrong. It's not
clear to me whether that's possible to do in a way that leaves the navigation
running in the browser process.

Assuming that path is possible: I've updated RenderFrameImpl to check
WebLocalFrame::isLoading(), and early-exiting with a FrameHostMsg_DidStopLoading
in that case, but no FrameHostMsg_DidStopLoading if maybeRenderFallbackContent()
is called and returns true. WDYT?

 }