- Revision
- 236631
- Author
- [email protected]
- Date
- 2018-09-28 18:38:26 -0700 (Fri, 28 Sep 2018)
Log Message
Regression(r236512): http/tests/navigation/keyboard-events-during-provisional-navigation.html is flaky
https://bugs.webkit.org/show_bug.cgi?id=190052
Reviewed by Ryosuke Niwa.
Source/WebKit:
Introduce a new IPC::SendOption indicating that the IPC should always be sent asynchronously,
even if the connection is in fully synchronous mode. This is used by WebKitTestRunner for
all text outputting (e.g. console.log) is asynchronous, and thus in order.
* Platform/IPC/Connection.cpp:
(IPC::Connection::sendMessage):
* Platform/IPC/Connection.h:
* WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:
(WKBundlePagePostMessageIgnoringFullySynchronousMode):
* WebProcess/InjectedBundle/API/c/WKBundlePage.h:
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::postMessageIgnoringFullySynchronousMode):
* WebProcess/WebPage/WebPage.h:
Tools:
The test relies on EventSender to send events to the page synchronously to the page and then uses console.log
to log those events. It also uses console.log() before sending those events to indicate what the test is about
to do. Note that console.log() normally causes the WebKitTestRunner to send an asynchronous IPC to the UIProcess
so that it can log the message.
The issue is that EventSender uses IPC::SendOption::UseFullySynchronousModeForTesting when sending the
sync IPC to the UIProcess. This option causes follow-up *asynchronous* IPC sent from the synchronous IPC reply
handler to be transformed into synchronous IPC.
As a result, some of the console.log IPC ended up being asynchronous and some other ended up being synchronous.
Because synchronous and asynchronous IPC is not necessarily processed in-order by the UIProcess, the logged
messages may end up being out of order, leading to flakiness.
To address the issue, we now make sure that InjectedBundle::outputText() uses a new IPC::SendOption indicating
that the IPC should always be sent asynchronously, even if the connection is in fully synchronous mode. As a
result, all text outputing IPC to the UIProcess will be asynchronous, and thus in order.
* WebKitTestRunner/InjectedBundle/InjectedBundle.cpp:
(WTR::InjectedBundle::outputText):
Modified Paths
Diff
Modified: trunk/LayoutTests/fast/forms/file/input-file-write-files-using-open-panel.html (236630 => 236631)
--- trunk/LayoutTests/fast/forms/file/input-file-write-files-using-open-panel.html 2018-09-29 01:36:27 UTC (rev 236630)
+++ trunk/LayoutTests/fast/forms/file/input-file-write-files-using-open-panel.html 2018-09-29 01:38:26 UTC (rev 236631)
@@ -18,8 +18,9 @@
file1.addEventListener("change", function() {
file2.addEventListener("change", function() {
- runTest(file1, file2);
- finishJSTest();
+ setTimeout(() => {
+ runTest(file1, file2);
+ }, 0);
});
window.setTimeout(function() {
dragFilesOntoInput(file2, ["bar.txt"]);
@@ -43,6 +44,8 @@
file1.files = file2.files;
shouldBe("file1.files.length", "1");
shouldBeEqualToString("file1.files.item(0).name", "bar.txt");
+
+ finishJSTest();
}
function dragFilesOntoInput(input, files) {
Modified: trunk/Source/WebKit/ChangeLog (236630 => 236631)
--- trunk/Source/WebKit/ChangeLog 2018-09-29 01:36:27 UTC (rev 236630)
+++ trunk/Source/WebKit/ChangeLog 2018-09-29 01:38:26 UTC (rev 236631)
@@ -1,3 +1,24 @@
+2018-09-28 Chris Dumez <[email protected]>
+
+ Regression(r236512): http/tests/navigation/keyboard-events-during-provisional-navigation.html is flaky
+ https://bugs.webkit.org/show_bug.cgi?id=190052
+
+ Reviewed by Ryosuke Niwa.
+
+ Introduce a new IPC::SendOption indicating that the IPC should always be sent asynchronously,
+ even if the connection is in fully synchronous mode. This is used by WebKitTestRunner for
+ all text outputting (e.g. console.log) is asynchronous, and thus in order.
+
+ * Platform/IPC/Connection.cpp:
+ (IPC::Connection::sendMessage):
+ * Platform/IPC/Connection.h:
+ * WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:
+ (WKBundlePagePostMessageIgnoringFullySynchronousMode):
+ * WebProcess/InjectedBundle/API/c/WKBundlePage.h:
+ * WebProcess/WebPage/WebPage.cpp:
+ (WebKit::WebPage::postMessageIgnoringFullySynchronousMode):
+ * WebProcess/WebPage/WebPage.h:
+
2018-09-28 John Wilander <[email protected]>
Skip debug assertion in ResourceLoadStatisticsMemoryStore::recursivelyGetAllDomainsThatHaveRedirectedToThisDomain()
Modified: trunk/Source/WebKit/Platform/IPC/Connection.cpp (236630 => 236631)
--- trunk/Source/WebKit/Platform/IPC/Connection.cpp 2018-09-29 01:36:27 UTC (rev 236630)
+++ trunk/Source/WebKit/Platform/IPC/Connection.cpp 2018-09-29 01:38:26 UTC (rev 236631)
@@ -400,7 +400,7 @@
if (!isValid())
return false;
- if (isMainThread() && m_inDispatchMessageMarkedToUseFullySynchronousModeForTesting && !encoder->isSyncMessage() && !(encoder->messageReceiverName() == "IPC")) {
+ if (isMainThread() && m_inDispatchMessageMarkedToUseFullySynchronousModeForTesting && !encoder->isSyncMessage() && !(encoder->messageReceiverName() == "IPC") && !sendOptions.contains(SendOption::IgnoreFullySynchronousMode)) {
uint64_t syncRequestID;
auto wrappedMessage = createSyncMessageEncoder("IPC", "WrappedAsyncMessageForTesting", encoder->destinationID(), syncRequestID);
wrappedMessage->setFullySynchronousModeForTesting();
Modified: trunk/Source/WebKit/Platform/IPC/Connection.h (236630 => 236631)
--- trunk/Source/WebKit/Platform/IPC/Connection.h 2018-09-29 01:36:27 UTC (rev 236630)
+++ trunk/Source/WebKit/Platform/IPC/Connection.h 2018-09-29 01:38:26 UTC (rev 236631)
@@ -61,6 +61,7 @@
// Whether this message should be dispatched when waiting for a sync reply.
// This is the default for synchronous messages.
DispatchMessageEvenWhenWaitingForSyncReply = 1 << 0,
+ IgnoreFullySynchronousMode = 1 << 1,
};
enum class SendSyncOption {
Modified: trunk/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp (236630 => 236631)
--- trunk/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp 2018-09-29 01:36:27 UTC (rev 236630)
+++ trunk/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp 2018-09-29 01:38:26 UTC (rev 236631)
@@ -665,6 +665,11 @@
toImpl(pageRef)->postMessage(toWTFString(messageNameRef), toImpl(messageBodyRef));
}
+void WKBundlePagePostMessageIgnoringFullySynchronousMode(WKBundlePageRef pageRef, WKStringRef messageNameRef, WKTypeRef messageBodyRef)
+{
+ toImpl(pageRef)->postMessageIgnoringFullySynchronousMode(toWTFString(messageNameRef), toImpl(messageBodyRef));
+}
+
void WKBundlePagePostSynchronousMessageForTesting(WKBundlePageRef pageRef, WKStringRef messageNameRef, WKTypeRef messageBodyRef, WKTypeRef* returnDataRef)
{
WebPage* page = toImpl(pageRef);
Modified: trunk/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.h (236630 => 236631)
--- trunk/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.h 2018-09-29 01:36:27 UTC (rev 236630)
+++ trunk/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.h 2018-09-29 01:38:26 UTC (rev 236631)
@@ -124,6 +124,8 @@
// Switches a connection into a fully synchronous mode, so all messages become synchronous until we get a response.
WK_EXPORT void WKBundlePagePostSynchronousMessageForTesting(WKBundlePageRef page, WKStringRef messageName, WKTypeRef messageBody, WKTypeRef* returnData);
+// Same as WKBundlePagePostMessage() but the message cannot become synchronous, even if the connection is in fully synchronous mode.
+WK_EXPORT void WKBundlePagePostMessageIgnoringFullySynchronousMode(WKBundlePageRef page, WKStringRef messageName, WKTypeRef messageBody);
#ifdef __cplusplus
}
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (236630 => 236631)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2018-09-29 01:36:27 UTC (rev 236630)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2018-09-29 01:38:26 UTC (rev 236631)
@@ -5828,6 +5828,11 @@
send(Messages::WebPageProxy::HandleMessage(messageName, UserData(WebProcess::singleton().transformObjectsToHandles(messageBody))));
}
+void WebPage::postMessageIgnoringFullySynchronousMode(const String& messageName, API::Object* messageBody)
+{
+ send(Messages::WebPageProxy::HandleMessage(messageName, UserData(WebProcess::singleton().transformObjectsToHandles(messageBody))), pageID(), IPC::SendOption::IgnoreFullySynchronousMode);
+}
+
void WebPage::postSynchronousMessageForTesting(const String& messageName, API::Object* messageBody, RefPtr<API::Object>& returnData)
{
UserData returnUserData;
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.h (236630 => 236631)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.h 2018-09-29 01:36:27 UTC (rev 236630)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.h 2018-09-29 01:38:26 UTC (rev 236631)
@@ -1014,6 +1014,7 @@
void postMessage(const String& messageName, API::Object* messageBody);
void postSynchronousMessageForTesting(const String& messageName, API::Object* messageBody, RefPtr<API::Object>& returnData);
+ void postMessageIgnoringFullySynchronousMode(const String& messageName, API::Object* messageBody);
#if PLATFORM(GTK)
void setInputMethodState(bool);
Modified: trunk/Tools/ChangeLog (236630 => 236631)
--- trunk/Tools/ChangeLog 2018-09-29 01:36:27 UTC (rev 236630)
+++ trunk/Tools/ChangeLog 2018-09-29 01:38:26 UTC (rev 236631)
@@ -1,3 +1,28 @@
+2018-09-28 Chris Dumez <[email protected]>
+
+ Regression(r236512): http/tests/navigation/keyboard-events-during-provisional-navigation.html is flaky
+ https://bugs.webkit.org/show_bug.cgi?id=190052
+
+ Reviewed by Ryosuke Niwa.
+
+ The test relies on EventSender to send events to the page synchronously to the page and then uses console.log
+ to log those events. It also uses console.log() before sending those events to indicate what the test is about
+ to do. Note that console.log() normally causes the WebKitTestRunner to send an asynchronous IPC to the UIProcess
+ so that it can log the message.
+ The issue is that EventSender uses IPC::SendOption::UseFullySynchronousModeForTesting when sending the
+ sync IPC to the UIProcess. This option causes follow-up *asynchronous* IPC sent from the synchronous IPC reply
+ handler to be transformed into synchronous IPC.
+ As a result, some of the console.log IPC ended up being asynchronous and some other ended up being synchronous.
+ Because synchronous and asynchronous IPC is not necessarily processed in-order by the UIProcess, the logged
+ messages may end up being out of order, leading to flakiness.
+
+ To address the issue, we now make sure that InjectedBundle::outputText() uses a new IPC::SendOption indicating
+ that the IPC should always be sent asynchronously, even if the connection is in fully synchronous mode. As a
+ result, all text outputing IPC to the UIProcess will be asynchronous, and thus in order.
+
+ * WebKitTestRunner/InjectedBundle/InjectedBundle.cpp:
+ (WTR::InjectedBundle::outputText):
+
2018-09-28 Myles C. Maxfield <[email protected]>
[WHLSL] Pointers should have automatically-generated equality checks
Modified: trunk/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp (236630 => 236631)
--- trunk/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp 2018-09-29 01:36:27 UTC (rev 236630)
+++ trunk/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp 2018-09-29 01:38:26 UTC (rev 236631)
@@ -603,7 +603,10 @@
return;
WKRetainPtr<WKStringRef> messageName(AdoptWK, WKStringCreateWithUTF8CString("TextOutput"));
WKRetainPtr<WKStringRef> messageBody(AdoptWK, WKStringCreateWithUTF8CString(output.utf8().data()));
- WKBundlePagePostMessage(page()->page(), messageName.get(), messageBody.get());
+ // We use WKBundlePagePostMessageIgnoringFullySynchronousMode() instead of WKBundlePagePostMessage() to make sure that all text output
+ // is done via asynchronous IPC, even if the connection is in fully synchronous mode due to a WKBundlePagePostSynchronousMessageForTesting()
+ // call. Otherwise, messages logged via sync and async IPC may end up out of order and cause flakiness.
+ WKBundlePagePostMessageIgnoringFullySynchronousMode(page()->page(), messageName.get(), messageBody.get());
}
void InjectedBundle::postNewBeforeUnloadReturnValue(bool value)
