Skip to content
This repository was archived by the owner on Jun 24, 2022. It is now read-only.

Commit 81131ef

Browse files
author
[email protected]
committed
Source/WebKit:
REGRESSION(r266634): [macOS release] 4 layout tests became flaky failures https://bugs.webkit.org/show_bug.cgi?id=216275 <rdar://problem/68515242> Reviewed by Tim Horton. After r266634, activity state changes caused by adding/removing view will not be dispatched to web process until transaction is committed in UI process. To make sure web process picks up the changes, we need to invoke the callbacks after activity state changes are dispatched. * UIProcess/API/Cocoa/WKWebViewPrivateForTesting.h: * UIProcess/API/Cocoa/WKWebViewTesting.mm: (-[WKWebView _doAfterActivityStateUpdate:]): * UIProcess/Cocoa/WebPageProxyCocoa.mm: (WebKit::WebPageProxy::addActivityStateUpdateCompletionHandler): * UIProcess/WebPageProxy.cpp: (WebKit::WebPageProxy::dispatchActivityStateChange): * UIProcess/WebPageProxy.h: Tools: REGRESSION (r266634): [macOS release] 4 layout tests became flaky failures https://bugs.webkit.org/show_bug.cgi?id=216275 <rdar://problem/68515242> Reviewed by Tim Horton. * WebKitTestRunner/cocoa/UIScriptControllerCocoa.mm: (WTR::UIScriptControllerCocoa::removeViewFromWindow): (WTR::UIScriptControllerCocoa::addViewToWindow): LayoutTests: REGRESSION (r266634): [macOS release] 4 layout tests became flaky failures https://bugs.webkit.org/show_bug.cgi?id=216275 <rdar://problem/68515242> Reviewed by Tim Horton. Add more event handlers to make test more stable, and add extra logging for debugging. * fast/events/page-visibility-iframe-move-test-expected.txt: * fast/events/page-visibility-iframe-move-test.html: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@266847 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 8ede6b6 commit 81131ef

File tree

11 files changed

+116
-34
lines changed

11 files changed

+116
-34
lines changed

‎LayoutTests/ChangeLog

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,16 @@
1+
2020-09-10 Sihui Liu <[email protected]>
2+
3+
REGRESSION (r266634): [macOS release] 4 layout tests became flaky failures
4+
https://bugs.webkit.org/show_bug.cgi?id=216275
5+
<rdar://problem/68515242>
6+
7+
Reviewed by Tim Horton.
8+
9+
Add more event handlers to make test more stable, and add extra logging for debugging.
10+
11+
* fast/events/page-visibility-iframe-move-test-expected.txt:
12+
* fast/events/page-visibility-iframe-move-test.html:
13+
114
2020-09-10 Antti Koivisto <[email protected]>
215

316
Don't create event regions when the page has no subscrollers

‎LayoutTests/fast/events/page-visibility-iframe-move-test-expected.txt

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,16 @@ This test checks that an iframe that moves between pages with different visibili
33
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
44

55

6-
Window 1 Loaded
7-
Window 2 Loaded
6+
Window 1 is loaded
7+
Window 2 is loaded
88
PASS window.document.hidden is false
99
PASS window2.document.hidden is false
1010
PASS iframe.contentDocument.hidden is false
11+
VisibilityChange event is received
1112
PASS window.document.hidden is true
1213
PASS window2.document.hidden is false
1314
PASS iframe.contentDocument.hidden is false
14-
Adopted iframe to Window 1
15+
Frame is moved
1516
PASS window.document.hidden is true
1617
PASS window2.document.hidden is false
1718
PASS iframe.contentDocument.hidden is true

‎LayoutTests/fast/events/page-visibility-iframe-move-test.html

Lines changed: 39 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -8,66 +8,76 @@
88
description("This test checks that an iframe that moves between pages with different visibility will have the correct visibility value.");
99

1010
var jsTestIsAsync = true;
11-
1211
var window2, iframe;
13-
var numVisibilityChanges = 0;
12+
13+
function startTest() {
14+
if (!window.testRunner)
15+
return;
16+
testRunner.waitUntilDone();
17+
testRunner.setCanOpenWindows();
18+
19+
debug("Window 1 is loaded");
20+
21+
if (window.document.hidden) {
22+
document.addEventListener("visibilitychange", openWindow, false);
23+
return;
24+
}
25+
26+
openWindow();
27+
}
28+
29+
function openWindow() {
30+
window2 = window.open("resources/page-visibility-iframe-move-new-page.html");
31+
window2.addEventListener("load", window2Loaded, false);
32+
}
1433

1534
function window2Loaded() {
16-
debug("Window 2 Loaded");
35+
debug("Window 2 is loaded");
36+
37+
if (window2.document.hidden) {
38+
window2.document.addEventListener("visibilitychange", changePageVisibility, false);
39+
return;
40+
}
41+
42+
changePageVisibility();
43+
}
1744

45+
function changePageVisibility() {
1846
iframe = window2.document.getElementById("iframe");
1947

2048
shouldBeFalse("window.document.hidden");
2149
shouldBeFalse("window2.document.hidden");
2250
shouldBeFalse("iframe.contentDocument.hidden");
2351

24-
// Change the visibility of the current page to invisible.
25-
if (window.testRunner) {
26-
numVisibilityChanges++;
52+
document.addEventListener("visibilitychange", onVisibilityChange, false);
53+
if (window.testRunner)
2754
window.testRunner.setPageVisibility("hidden");
28-
}
2955
}
3056

3157
function onVisibilityChange() {
58+
debug("VisibilityChange event is received");
59+
3260
shouldBeTrue("window.document.hidden");
3361
shouldBeFalse("window2.document.hidden");
3462
shouldBeFalse("iframe.contentDocument.hidden");
3563

3664
window.document.adoptNode(iframe);
3765
window.document.body.appendChild(iframe);
38-
debug("Adopted iframe to Window 1");
3966

67+
debug("Frame is moved")
4068
shouldBeTrue("window.document.hidden");
4169
shouldBeFalse("window2.document.hidden");
4270
shouldBeTrue("iframe.contentDocument.hidden");
4371

44-
window2.close();
45-
4672
finishTest();
4773
}
4874

49-
function startTest() {
50-
if (window.testRunner) {
51-
testRunner.waitUntilDone();
52-
testRunner.setCanOpenWindows();
53-
}
54-
55-
debug("Window 1 Loaded");
56-
document.addEventListener("visibilitychange",
57-
onVisibilityChange, false);
58-
59-
window2 = window.open("resources/page-visibility-iframe-move-new-page.html");
60-
window2.addEventListener("load", window2Loaded, false);
61-
}
62-
6375
function finishTest() {
64-
document.removeEventListener("visibilitychange",
65-
onVisibilityChange, false);
76+
window2.close();
77+
document.removeEventListener("visibilitychange", onVisibilityChange, false);
6678
window2.removeEventListener("load", window2Loaded, false);
79+
window2.document.removeEventListener("visibilitychange", changePageVisibility, false);
6780

68-
if (window.testRunner) {
69-
testRunner.resetPageVisibility();
70-
}
7181
finishJSTest();
7282
}
7383

‎Source/WebKit/ChangeLog

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,24 @@
1+
2020-09-10 Sihui Liu <[email protected]>
2+
3+
REGRESSION(r266634): [macOS release] 4 layout tests became flaky failures
4+
https://bugs.webkit.org/show_bug.cgi?id=216275
5+
<rdar://problem/68515242>
6+
7+
Reviewed by Tim Horton.
8+
9+
After r266634, activity state changes caused by adding/removing view will not be dispatched to web process until
10+
transaction is committed in UI process. To make sure web process picks up the changes, we need to invoke the
11+
callbacks after activity state changes are dispatched.
12+
13+
* UIProcess/API/Cocoa/WKWebViewPrivateForTesting.h:
14+
* UIProcess/API/Cocoa/WKWebViewTesting.mm:
15+
(-[WKWebView _doAfterActivityStateUpdate:]):
16+
* UIProcess/Cocoa/WebPageProxyCocoa.mm:
17+
(WebKit::WebPageProxy::addActivityStateUpdateCompletionHandler):
18+
* UIProcess/WebPageProxy.cpp:
19+
(WebKit::WebPageProxy::dispatchActivityStateChange):
20+
* UIProcess/WebPageProxy.h:
21+
122
2020-09-10 Chris Dumez <[email protected]>
223

324
Some WebAudio tests give different output on different machines

‎Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivateForTesting.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,4 +75,6 @@ typedef enum {
7575
- (BOOL)_hasSleepDisabler;
7676
- (WKWebViewAudioRoutingArbitrationStatus)_audioRoutingArbitrationStatus;
7777
- (double)_audioRoutingArbitrationUpdateTime;
78+
79+
- (void)_doAfterActivityStateUpdate:(void (^)(void))completionHandler;
7880
@end

‎Source/WebKit/UIProcess/API/Cocoa/WKWebViewTesting.mm

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,4 +271,9 @@ - (double)_audioRoutingArbitrationUpdateTime
271271
#endif
272272
}
273273

274+
- (void)_doAfterActivityStateUpdate:(void (^)(void))completionHandler
275+
{
276+
_page->addActivityStateUpdateCompletionHandler(makeBlockPtr(completionHandler));
277+
}
278+
274279
@end

‎Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,16 @@
527527
m_activityStateChangeDispatcher->schedule();
528528
}
529529

530+
void WebPageProxy::addActivityStateUpdateCompletionHandler(CompletionHandler<void()>&& completionHandler)
531+
{
532+
if (!m_hasScheduledActivityStateUpdate) {
533+
completionHandler();
534+
return;
535+
}
536+
537+
m_activityStateUpdateCallbacks.append(WTFMove(completionHandler));
538+
}
539+
530540
} // namespace WebKit
531541

532542
#undef MESSAGE_CHECK_COMPLETION

‎Source/WebKit/UIProcess/WebPageProxy.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2056,6 +2056,12 @@ void WebPageProxy::dispatchActivityStateChange()
20562056
m_potentiallyChangedActivityStateFlags = { };
20572057
m_activityStateChangeWantsSynchronousReply = false;
20582058
m_viewWasEverInWindow |= isNowInWindow;
2059+
2060+
#if PLATFORM(COCOA)
2061+
for (auto& callback : m_activityStateUpdateCallbacks)
2062+
callback();
2063+
m_activityStateUpdateCallbacks.clear();
2064+
#endif
20592065
}
20602066

20612067
bool WebPageProxy::isAlwaysOnLoggingAllowed() const

‎Source/WebKit/UIProcess/WebPageProxy.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -901,6 +901,7 @@ class WebPageProxy : public API::ObjectImpl<API::Object::Type::Page>
901901
RemoteLayerTreeScrollingPerformanceData* scrollingPerformanceData() { return m_scrollingPerformanceData.get(); }
902902

903903
void scheduleActivityStateUpdate();
904+
void addActivityStateUpdateCompletionHandler(CompletionHandler<void()>&&);
904905
#endif // PLATFORM(COCOA)
905906

906907
void changeFontAttributes(WebCore::FontAttributeChanges&&);
@@ -2742,6 +2743,7 @@ class WebPageProxy : public API::ObjectImpl<API::Object::Type::Page>
27422743
bool m_scrollPerformanceDataCollectionEnabled { false };
27432744

27442745
bool m_hasScheduledActivityStateUpdate { false };
2746+
Vector<CompletionHandler<void()>> m_activityStateUpdateCallbacks;
27452747
#endif
27462748
UserObservablePageCounter::Token m_pageIsUserObservableCount;
27472749
ProcessSuppressionDisabledToken m_preventProcessSuppressionCount;

‎Tools/ChangeLog

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,15 @@
1+
2020-09-10 Sihui Liu <[email protected]>
2+
3+
REGRESSION (r266634): [macOS release] 4 layout tests became flaky failures
4+
https://bugs.webkit.org/show_bug.cgi?id=216275
5+
<rdar://problem/68515242>
6+
7+
Reviewed by Tim Horton.
8+
9+
* WebKitTestRunner/cocoa/UIScriptControllerCocoa.mm:
10+
(WTR::UIScriptControllerCocoa::removeViewFromWindow):
11+
(WTR::UIScriptControllerCocoa::addViewToWindow):
12+
113
2020-09-10 Chris Dumez <[email protected]>
214

315
Some WebAudio tests give different output on different machines

0 commit comments

Comments
 (0)