Bug 181269 - Swipe Back gesture to previous page causes lag in rendering
Summary: Swipe Back gesture to previous page causes lag in rendering
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: Safari 11
Hardware: iPhone / iPad iOS 11
: P2 Minor
Assignee: Tim Horton
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-01-03 18:00 PST by Cagdas Tulek
Modified: 2018-01-10 18:20 PST (History)
9 users (show)

See Also:


Attachments
iOS 10.3 video capture (5.03 MB, video/mp4)
2018-01-03 18:00 PST, Cagdas Tulek
no flags Details
iOS 11.1 video capture (3.90 MB, video/mp4)
2018-01-03 18:01 PST, Cagdas Tulek
no flags Details
Patch (19.86 KB, patch)
2018-01-09 20:30 PST, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (20.45 KB, patch)
2018-01-10 10:06 PST, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (22.43 KB, patch)
2018-01-10 10:27 PST, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (22.14 KB, patch)
2018-01-10 10:34 PST, Tim Horton
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-sierra-wk2 (2.69 MB, application/zip)
2018-01-10 12:09 PST, EWS Watchlist
no flags Details
Patch (22.79 KB, patch)
2018-01-10 15:34 PST, Tim Horton
thorton: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Cagdas Tulek 2018-01-03 18:00:31 PST
Created attachment 330426 [details]
iOS 10.3 video capture

This bug probably happens for single page web apps. I have attached 2 video shots for iOS 10.3 and iOS 11.1 to show the difference in behavior.

I can reproduce the issue at outschool.com:
- Go to https://outschool.com/online-classes
- Click one of the class images to see the class detail page
- Use swipe back gesture to go to previous page
- Previous page renders empty for a couple of seconds and then appears fully rendered

Other remarks:
- This issue does not happen on iOS 10.3. There is still a short freeze of full UI but more acceptable than the current situation on iOS 11.1
- This issue does not happen for websites which are not single page apps. Try news.ycombinator.com as an example.
- This issue does not happen when regular back button is used instead of swiping
Comment 1 Cagdas Tulek 2018-01-03 18:01:05 PST
Created attachment 330427 [details]
iOS 11.1 video capture
Comment 2 Tim Horton 2018-01-03 18:09:01 PST
<rdar://problem/35110344>
Comment 3 Tim Horton 2018-01-09 20:21:36 PST
My fix for twitter seems to fix this site too, so I’ll use this bug (I’ve also confirmed that this site uses scrollRestoration and pushState, so it makes sense).
Comment 4 Tim Horton 2018-01-09 20:30:34 PST
Created attachment 330877 [details]
Patch
Comment 5 Tim Horton 2018-01-10 10:06:16 PST
Created attachment 330924 [details]
Patch
Comment 6 Tim Horton 2018-01-10 10:27:22 PST
Created attachment 330929 [details]
Patch
Comment 7 Tim Horton 2018-01-10 10:34:06 PST
Created attachment 330930 [details]
Patch
Comment 8 Simon Fraser (smfr) 2018-01-10 11:09:53 PST
Comment on attachment 330930 [details]
Patch

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

> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:321
> +    std::optional<uint64_t> _firstTransactionIDAfterPageRestore;

We really need a typedef for transactionIDs, or maybe use Identified or something.

> LayoutTests/swipe/pushstate-with-manual-scrollrestoration.html:9
> +history.scrollRestoration = "manual";

Maybe you should have a basic swipe test without scrollRestoration as well.

> LayoutTests/swipe/pushstate-with-manual-scrollrestoration.html:29
> +    measuredDurationShouldBeLessThan("snapshotRemoval", 1000, "Because we're using the page cache, it shouldn't be long between the gesture completing and the snapshot being removed.")

I wonder how flakey this will be on bots.
Comment 9 Tim Horton 2018-01-10 11:26:35 PST
(In reply to Simon Fraser (smfr) from comment #8)
> Comment on attachment 330930 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=330930&action=review
> 
> > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:321
> > +    std::optional<uint64_t> _firstTransactionIDAfterPageRestore;
> 
> We really need a typedef for transactionIDs, or maybe use Identified or
> something.

Yes.

> > LayoutTests/swipe/pushstate-with-manual-scrollrestoration.html:9
> > +history.scrollRestoration = "manual";
> 
> Maybe you should have a basic swipe test without scrollRestoration as well.

Probably!

> > LayoutTests/swipe/pushstate-with-manual-scrollrestoration.html:29
> > +    measuredDurationShouldBeLessThan("snapshotRemoval", 1000, "Because we're using the page cache, it shouldn't be long between the gesture completing and the snapshot being removed.")
> 
> I wonder how flakey this will be on bots.

If it comes up, we can replace this with a bit that disables the watchdog, and just let the test time out altogether.
Comment 10 EWS Watchlist 2018-01-10 12:09:09 PST
Comment on attachment 330930 [details]
Patch

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

New failing tests:
swipe/pushstate-with-manual-scrollrestoration.html
Comment 11 EWS Watchlist 2018-01-10 12:09:10 PST
Created attachment 330946 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 12 Tim Horton 2018-01-10 15:26:28 PST
Bug tracking the test failing on Mac (it’s pretty specific to the test): https://bugs.webkit.org/show_bug.cgi?id=181502. Going to skip the test on Mac for now.
Comment 13 Tim Horton 2018-01-10 15:34:41 PST
Created attachment 330975 [details]
Patch
Comment 14 Tim Horton 2018-01-10 18:19:01 PST
https://trac.webkit.org/changeset/226750/webkit
Comment 15 Radar WebKit Bug Importer 2018-01-10 18:20:25 PST
<rdar://problem/36424175>