Bug 149317 – REGRESSION (r182449, Mavericks ONLY): Pages re-open empty after swiping back and scrolling on them

• Component: Page Loading
• Status: RESOLVED FIXED

[Chris Dumez]

Pages re-open empty after swiping back and scrolling on them sometimes on Mavericks, after <http://trac.webkit.org/changeset/182449>.

[Chris Dumez]

rdar://problem/22521514

[Chris Dumez]

Created attachment 261459 [details]
Patch

[Chris Dumez]

Created attachment 261460 [details]
Patch

[Build Bot]

Comment on attachment 261460 [details]
Patch

Attachment 261460 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/181263

New failing tests:
http/tests/navigation/page-cache-pending-image-load.html

[Build Bot]

Created attachment 261462 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Build Bot]

Comment on attachment 261460 [details]
Patch

Attachment 261460 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/181273

New failing tests:
inspector/codemirror/prettyprinting-css-rules.html
http/tests/navigation/page-cache-pending-image-load.html

[Build Bot]

Created attachment 261464 [details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Alexey Proskuryakov]

Comment on attachment 261460 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +        (images and XHR). This is because it has been determined via bisection
> +        that this change is the one that introduced the bug on Mavericks.

Is there any theory at all as to how this would be possible?

[Chris Dumez]

(In reply to comment #8)
> Comment on attachment 261460 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=261460&action=review
> 
> > Source/WebCore/ChangeLog:12
> > +        (images and XHR). This is because it has been determined via bisection
> > +        that this change is the one that introduced the bug on Mavericks.
> 
> Is there any theory at all as to how this would be possible?

No, I have no idea why at the moment. Since this is Mavericks only for now, I don't know if it is worth spending too much time figuring it out either.

[Alexey Proskuryakov]

It's just not easy to imagine how the root cause can be Mavericks only.

[Chris Dumez]

(In reply to comment #10)
> It's just not easy to imagine how the root cause can be Mavericks only.

I tend to agree. However, we have been able to reproduce this one fairly easily on Mavericks but never on more recent OSes.

[Chris Dumez]

(In reply to comment #8)
> Comment on attachment 261460 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=261460&action=review
> 
> > Source/WebCore/ChangeLog:12
> > +        (images and XHR). This is because it has been determined via bisection
> > +        that this change is the one that introduced the bug on Mavericks.
> 
> Is there any theory at all as to how this would be possible?

This particular bug only happens when swiping back, not when clicking the back button. Therefore, chances are this is the gesture snapshot being blank and not going away for some reason. I remember that when I introduced the optimization in r182449, I had to make a follow-up fix in <http://trac.webkit.org/changeset/184399> because the gesture snapshot would stay up (until the 3 seconds timeout) due to the page being marked as having an error (due to the cancelled image loads). It looks like this fix may have been insufficient on Mavericks.

I looked at the code to try and figure out what could be different on Mavericks and I would the following:
#if __MAC_OS_X_VERSION_MIN_REQUIRED < 101000
#define ENABLE_LEGACY_SWIPE_SHADOW_STYLE 1
#endif

It looks like we use a slightly different code path for gesture snapshots on Mavericks so this could be related. Note however that this code has changed a lot since branching because Tim refactored the code so we share more between different platforms.

Now we could spend time trying to debug this Mavericks-only issue or we can simply disable the PageCache optimization that lets pages with cancelled loads (and thus in error state) into the PageCache. I would still vote for the latter.

[Tim Horton]

(In reply to comment #12)
> (In reply to comment #8)
> > Comment on attachment 261460 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=261460&action=review
> > 
> > > Source/WebCore/ChangeLog:12
> > > +        (images and XHR). This is because it has been determined via bisection
> > > +        that this change is the one that introduced the bug on Mavericks.
> > 
> > Is there any theory at all as to how this would be possible?
> 
> This particular bug only happens when swiping back, not when clicking the
> back button. Therefore, chances are this is the gesture snapshot being blank
> and not going away for some reason. I remember that when I introduced the
> optimization in r182449, I had to make a follow-up fix in
> <http://trac.webkit.org/changeset/184399> because the gesture snapshot would
> stay up (until the 3 seconds timeout) due to the page being marked as having
> an error (due to the cancelled image loads). It looks like this fix may have
> been insufficient on Mavericks.
> 
> I looked at the code to try and figure out what could be different on
> Mavericks and I would the following:
> #if __MAC_OS_X_VERSION_MIN_REQUIRED < 101000
> #define ENABLE_LEGACY_SWIPE_SHADOW_STYLE 1
> #endif
> 
> It looks like we use a slightly different code path for gesture snapshots on
> Mavericks so this could be related. Note however that this code has changed
> a lot since branching because Tim refactored the code so we share more
> between different platforms.
> 
> Now we could spend time trying to debug this Mavericks-only issue or we can
> simply disable the PageCache optimization that lets pages with cancelled
> loads (and thus in error state) into the PageCache. I would still vote for
> the latter.

I would love a CALayer tree dump when we're in this state...

[Chris Dumez]

Created attachment 261506 [details]
Patch

[Chris Dumez]

Comment on attachment 261506 [details]
Patch

Clearing flags on attachment: 261506

Committed r189976: <http://trac.webkit.org/changeset/189976>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.