Summary: | Regression(r233865): Causes synchronous IPC in the middle of layout | ||
---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> |
Component: | WebKit2 | Assignee: | Jer Noble <jer.noble> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | achristensen, commit-queue, darin, ddkilzer, eric.carlson, ews-watchlist, ggaren, jeremyj-wk, jer.noble, rniwa, ryanhaddad, tsavell, webkit-bug-importer |
Priority: | P2 | Keywords: | InRadar |
Version: | WebKit Nightly Build | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=188310 | ||
Bug Depends on: | |||
Bug Blocks: | 186226 | ||
Attachments: |
Description
Chris Dumez
2018-08-03 08:42:59 PDT
Created attachment 346489 [details]
Patch
@Jer / Jeremy: Note that there is likely another bug here. Seems very odd that destroying a Media element's renderer would cause it to start autoplaying in the first place. (In reply to Chris Dumez from comment #3) > @Jer / Jeremy: Note that there is likely another bug here. Seems very odd > that destroying a Media element's renderer would cause it to start > autoplaying in the first place. Filed https://bugs.webkit.org/show_bug.cgi?id=188310 for this. Comment on attachment 346489 [details]
Patch
iOS simulator failure looks like a real failure?
media/no-fullscreen-when-hidden.html [ Failure ]
(In reply to Geoffrey Garen from comment #5) > Comment on attachment 346489 [details] > Patch > > iOS simulator failure looks like a real failure? > > media/no-fullscreen-when-hidden.html [ Failure ] Yes, look related indeed. I verified it was passing on Mac. Comment on attachment 346489 [details] Patch Attachment 346489 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/8752011 New failing tests: media/no-fullscreen-when-hidden.html Created attachment 346503 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.4
To Jer as there is apparently a security aspect to this and will requires a little bit more work. (In reply to Build Bot from comment #7) > New failing tests: > media/no-fullscreen-when-hidden.html I’m guessing we have introduced a new race. Comment on attachment 346489 [details] Patch Attachment 346489 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8755092 New failing tests: http/tests/security/canvas-remote-read-remote-video-blocked-no-crossorigin.html Created attachment 346556 [details]
Archive of layout-test-results from ews206 for win-future
The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Created attachment 355082 [details]
Patch
Comment on attachment 355082 [details] Patch Attachment 355082 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/10020628 Number of test failures exceeded the failure limit. Created attachment 355093 [details]
Archive of layout-test-results from ews102 for mac-sierra
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 355082 [details] Patch Attachment 355082 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/10021010 Number of test failures exceeded the failure limit. Created attachment 355098 [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 on attachment 355082 [details] Patch Attachment 355082 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/10020766 Number of test failures exceeded the failure limit. Created attachment 355101 [details]
Archive of layout-test-results from ews117 for mac-sierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117 Port: mac-sierra Platform: Mac OS X 10.12.6
Created attachment 355123 [details]
Patch
Comment on attachment 355123 [details] Patch Clearing flags on attachment: 355123 Committed r238322: <https://trac.webkit.org/changeset/238322> All reviewed patches have been landed. Closing bug. I think this may have caused the following API test time outs: Timeout TestWebKitAPI.FullscreenZoomInitialFrame.WebKit TestWebKitAPI.ExitFullscreenOnEnterPiP.ElementFullscreen Looks like this changed the order of IPC messages slightly, and the WKFullscreenWindowController doesn't get a chance to register as a VideoFullscreenModelContext client before the context tells everyone we entered PiP mode, breaking the test. Both of these tests are incorrect in different ways: In TestWebKitAPI.ExitFullscreenOnEnterPiP.ElementFullscreen, the video element is not the main content; it's too small and it's not playing. We should only cancel fullscreen mode when PiP is entered if the video which was taken into PiP was main content. This worked before by accident; the video element entering PiP makes it the main content, but it wasn't when it was in fullscreen. The new behavior is actually correct; the test needs to be updated to make the video element explicitly the main content, in this case by playing the video during a user gesture. In TestWebKitAPI.FullscreenZoomInitialFrame.WebKit, the WK1 WebView isn't visible at all; it shouldn't be allowed to go into fullscreen. The reason this worked previously is because the "GetIsViewVisible" UIClient method was not implemented in WK1, and it's default return value is true. Reopening to attach new patch. Created attachment 355186 [details]
Follow-up test fix
Comment on attachment 355186 [details] Follow-up test fix Clearing flags on attachment: 355186 Committed r238348: <https://trac.webkit.org/changeset/238348> All reviewed patches have been landed. Closing bug. |