[Webkit-unassigned] [Bug 117112] New: REGRESSION (r130783): Going "back" to a cached page from a page with a main resource error breaks scrolling, amongst other issues

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=117112

           Summary: REGRESSION (r130783): Going "back" to a cached page
                    from a page with a main resource error breaks
                    scrolling, amongst other issues
           Product: WebKit
           Version: 528+ (Nightly build)
          Platform: All
        OS/Version: All
            Status: NEW
          Keywords: InRadar
          Severity: Normal
          Priority: P2
         Component: Page Loading
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: beidson at apple.com
                CC: ap at webkit.org, andersca at apple.com, sam at webkit.org


REGRESSION (r130783): Going "back" to a cached page from a page with a main resource error breaks scrolling 

This includes pages with a long running main resource load where the acting of clicking "back" cancels that main resource load.

In these situations where the current page's main resource ended in an error, the Document's parsing was not cancelled and cleared out.

Later, when restoring the cached page, FrameLoader::clear() calls Document::cancelParsing(), which ends up closing the Document, which ends up causing a whole bunch of side effects such as checking whether or not the frame load is complete and then dispatching didFinishLoad.

Reading the intent of cancelParsing() all the way down the rabbit hole, it's clear that it was meant act on the Document for the current load.  But when FrameLoader::clear() is called during a cached page load it's *after* the cached page load has already started, and cancelParsing() doesn't affect the previous load.

So, in this case, the callbacks for the cached page load come in the following order:
didStartProvisionalLoad
didFinishLoad    <-- erroneously emitted during Document::cancelParsing()
didCommitLoad

By design, cancelParsing() is meant to be called multiple times per load, but it only does its dirty work the first time its called.  Normally it is called after the main document finishes loading.  But it apparently is never called when the main document load ends in error.

This doesn't cause a problem for normal page loads, because that process calls clear() very early on before any other work happens.  But cached page loads - for better or worse - need to follow a different code path.

It makes sense to me to call cancelParsing() the moment a main resource load finishes - whether it was due to completing successfully *or* due to error.  This will at least immediately free up the resources associated with the parser instead of waiting for them to be freed on the next navigation.

In radar as <rdar://problem/13751844>

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.