Bug 119416 - REGRESSION (r130783): Scrolling is broken going back to a cached page from a page that still has outstanding subresources
Summary: REGRESSION (r130783): Scrolling is broken going back to a cached page from a ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-08-01 17:21 PDT by Brady Eidson
Modified: 2013-08-02 08:17 PDT (History)
3 users (show)

See Also:

Attachments
Patch v1 (11.20 KB, patch)
2013-08-01 18:04 PDT, Brady Eidson 		darin: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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