Bug 119416

Summary: REGRESSION (r130783): Scrolling is broken going back to a cached page from a page that still has outstanding subresources
Product: WebKit Reporter: Brady Eidson <beidson>
Component: Page LoadingAssignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, japhet, simon.fraser
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch v1 darin: review+

Description Brady Eidson 2013-08-01 17:21:18 PDT
REGRESSION (r130783): Scrolling is broken going back to a cached page from a page that still has outstanding sub resources

This has the exact same symptom - but subtly different cause - as https://bugs.webkit.org/show_bug.cgi?id=117112

In radar as <rdar://problem/14601124>
Comment 1 Brady Eidson 2013-08-01 18:04:07 PDT
Created attachment 207973 [details]
Patch v1 

Uploading for EWS and review while layout tests also run locally.
Comment 2 Simon Fraser (smfr) 2013-08-01 18:12:21 PDT
Can you please file 2 more bugs to:
1. make the web process assert before the UI process does when things go wrong
2. make the loading logging detailed enough to detect mistakes like this
Comment 3 Darin Adler 2013-08-01 20:27:49 PDT
Comment on attachment 207973 [details]
Patch v1 

View in context: https://bugs.webkit.org/attachment.cgi?id=207973&action=review

r=me; I know you want to wait to see what Mac and Win EWS says, and I’m sure you’ll get those results soon.

> Source/WebCore/loader/DocumentLoader.cpp:311
> +    if (Document* doc = document())
> +        doc->cancelParsing();

When moving this it might be nice to get rid of the abbreviation too:

    if (Document* document = this->document())
        document->cancelParsing();
Comment 4 Brady Eidson 2013-08-01 21:55:57 PDT
(In reply to comment #3)
> (From update of attachment 207973 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=207973&action=review
> 
> r=me; I know you want to wait to see what Mac and Win EWS says, and I’m sure you’ll get those results soon.
> 
> > Source/WebCore/loader/DocumentLoader.cpp:311
> > +    if (Document* doc = document())
> > +        doc->cancelParsing();
> 
> When moving this it might be nice to get rid of the abbreviation too:
> 
>     if (Document* document = this->document())
>         document->cancelParsing();

DANGIT - I know you made the same comment last time after the fact, and I even made a mental note to do it this time.

Will do!
Comment 5 Brady Eidson 2013-08-01 22:00:19 PDT
(In reply to comment #2)
> Can you please file 2 more bugs to:
> 1. make the web process assert before the UI process does when things go wrong
> 2. make the loading logging detailed enough to detect mistakes like this

Doesn't really seem necessary to file two bugs for these.

Filed https://bugs.webkit.org/show_bug.cgi?id=119430
Comment 6 Brady Eidson 2013-08-02 08:17:19 PDT
http://trac.webkit.org/changeset/153649