Bug 192290 - REGRESSION(PSON): Process swapping code doesn't set DisplayID in WebContent process
Summary: REGRESSION(PSON): Process swapping code doesn't set DisplayID in WebContent p...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-12-02 11:12 PST by Simon Fraser (smfr)
Modified: 2018-12-05 22:12 PST (History)
8 users (show)

See Also:

Attachments
Fixes the bug (2.73 KB, patch)
2018-12-05 16:11 PST, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews114 for mac-sierra (1.41 MB, application/zip)
2018-12-05 16:59 PST, EWS Watchlist 		no flags Details
Archive of layout-test-results from ews103 for mac-sierra (2.57 MB, application/zip)
2018-12-05 17:02 PST, EWS Watchlist 		no flags Details
Patch for landing (1.45 KB, patch)
2018-12-05 19:32 PST, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) 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 Simon Fraser (smfr) 2018-12-02 11:12:13 PST 
If you add logging in DisplayRefreshMonitorManager::scheduleAnimation() in the if (!client.hasDisplayID()) early return, you'll find that we early return there in PSON-launched processes when the page calls requestAnimationFrame.

To repro: launch MiniBrowser, go to webkit.org, then navigate to browserbench.org (PSON navigation), and run MotionMark. You'll see lots of that logging.
Comment 1 Simon Fraser (smfr) 2018-12-02 11:42:59 PST 
Actually log in DocumentAnimationScheduler::createDisplayRefreshMonitor(), where we pas s in a displayID of 0. This zero displayID gets to the UI process, where we call CVDisplayLinkCreateWithCGDisplay(, 0), which seems to work, but I guess it defaults to the main display.

Per Arne, we need better protection against silent failures in this DisplayRefreshMonitor code.
Comment 2 Radar WebKit Bug Importer 2018-12-02 11:43:34 PST 
<rdar://problem/46404049>
Comment 3 Simon Fraser (smfr) 2018-12-02 11:49:17 PST 
I guess WebPageProxy::finishAttachingToWebProcess() should call windowScreenDidChange() after initializeWebPage().
Comment 4 Ryosuke Niwa 2018-12-05 16:11:09 PST 
Created attachment 356674 [details]
Fixes the bug
Comment 5 EWS Watchlist 2018-12-05 16:59:04 PST 
Comment on attachment 356674 [details]
Fixes the bug

Attachment 356674 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/10284957

Number of test failures exceeded the failure limit.
Comment 6 EWS Watchlist 2018-12-05 16:59:05 PST 
Created attachment 356681 [details]
Archive of layout-test-results from ews114 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 7 EWS Watchlist 2018-12-05 17:02:28 PST 
Comment on attachment 356674 [details]
Fixes the bug

Attachment 356674 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10285201

New failing tests:
http/tests/misc/resource-timing-resolution.html
workers/bomb.html
Comment 8 EWS Watchlist 2018-12-05 17:02:29 PST 
Created attachment 356682 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 9 Ryosuke Niwa 2018-12-05 19:32:32 PST 
Created attachment 356700 [details]
Patch for landing
Comment 10 Ryosuke Niwa 2018-12-05 19:32:47 PST 
Landing without the assertion since NSWindow in WTR/DRT is off screen.
Comment 11 Ryosuke Niwa 2018-12-05 22:12:30 PST 
Committed r238927: <https://trac.webkit.org/changeset/238927>