Bug 88079 – ASSERT(m_loadState == LoadStateCommitted) in WebFrameProxy::didFinishLoad(), when navigating back from a previously canceled page.

• Status: NEW
• Component: Page Loading

[alan]

1. load google.com (or any page that ends up in page cache) (Page A)
2. load relatively big page like nytimes.com to be able to cancel while loading. (Page B)
3. hit cancel while loading Page B.
4. navigate back to Page A.
5. ASSERT(m_loadState == LoadStateCommitted) hits in WebFrameProxy::didFinishLoad() as m_loadState == LoadStateProvisional

here is how:
1. when Page B gets canceled, loading stops and load failed status is dispatched to WebFrameProxy.
2. when navigating back to Page A, FrameLoader::loadProvisionalItemFromCachedPage() gets called as we are about to restore Page A from page cache.
3. loadProvisionalItemFromCachedPage() -> commitProvisionalLoad() -> CachedPage::restore() -> CachedFrame::open()->FrameLoader::open(CachedFrameBase& cachedFrame) -> FrameLoader::clear()
4. In clear(), some cleanup is done on the current Page A's document. It calls m_frame->document()->cancelParsing() (still Page A), which usually early returns as m_parser is NULL due to a prior call to Document::detachCall() in normal, non-canceling condition.
5. however, in this case Document::cancelParsing() gets through as m_parser != NULL, detaches the parser and calls explicitClose(), which in turn calls FrameLoader::checkCompleted() -> FrameLoader::checkLoadCompleteForThisFrame().
6. FrameLoader at this point (in checkLoadCompleteForThisFrame()) is in FrameStateCommittedPage state (though not yet dispatched to the client) and it gets moved to FrameStateComplete and the state change is dispatched to the client.
7. From the loader client pow, state jumps from state provisional to state complete. -> ASSERT.

in a non-cancel case, Document::implicitClose() detaches the parser and at #4, the call to cancelParsing() early returns, no explicitClose() call or any dispatching occurs.

Obviously an unconditional Document::cancelParsing() call at FrameLoader::stopLoading() would solve this problem and put the Document in the same state as if was fully loaded as far as this context is concerned, but I am not sure if that's the intended behavior (as this far the canceled document is neither explicitly nor implicitly closed). Especially after looking at how parser canceling is handled at FrameLoader::stopLoading(). It would look odd to call cancelParsing after it was stopped a few times already.

http://trac.webkit.org/browser/trunk/Source/WebCore/loader/FrameLoader.cpp#L351 and
http://trac.webkit.org/browser/trunk/Source/WebCore/loader/FrameLoader.cpp#L403

if (m_frame->document() && m_frame->document()->parser())
    m_frame->document()->parser()->stopParsing();
.....
.....
if (m_frame->document() && m_frame->document()->parsing()) {
    finishedParsing();
    m_frame->document()->setParsing(false);
}

[alan]

wondering why we don't want to implicit close the document at FrameLoader::stopLoading() 

m_didCallImplicitClose = true; // don't want that one either