- Revision
- 236680
- Author
- [email protected]
- Date
- 2018-10-01 11:57:02 -0700 (Mon, 01 Oct 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:
* 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 indicated
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/Source/WebKit/ChangeLog (236679 => 236680)
--- trunk/Source/WebKit/ChangeLog 2018-10-01 18:38:49 UTC (rev 236679)
+++ trunk/Source/WebKit/ChangeLog 2018-10-01 18:57:02 UTC (rev 236680)
@@ -1,3 +1,20 @@
+2018-10-01 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.
+
+ * 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-10-01 Daniel Bates <[email protected]>
[iOS] Special keys are misidentified in DOM keyboard events
Modified: trunk/Source/WebKit/Platform/IPC/Connection.cpp (236679 => 236680)
--- trunk/Source/WebKit/Platform/IPC/Connection.cpp 2018-10-01 18:38:49 UTC (rev 236679)
+++ trunk/Source/WebKit/Platform/IPC/Connection.cpp 2018-10-01 18:57:02 UTC (rev 236680)
@@ -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 (236679 => 236680)
--- trunk/Source/WebKit/Platform/IPC/Connection.h 2018-10-01 18:38:49 UTC (rev 236679)
+++ trunk/Source/WebKit/Platform/IPC/Connection.h 2018-10-01 18:57:02 UTC (rev 236680)
@@ -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 (236679 => 236680)
--- trunk/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp 2018-10-01 18:38:49 UTC (rev 236679)
+++ trunk/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp 2018-10-01 18:57:02 UTC (rev 236680)
@@ -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 (236679 => 236680)
--- trunk/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.h 2018-10-01 18:38:49 UTC (rev 236679)
+++ trunk/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.h 2018-10-01 18:57:02 UTC (rev 236680)
@@ -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 (236679 => 236680)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2018-10-01 18:38:49 UTC (rev 236679)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2018-10-01 18:57:02 UTC (rev 236680)
@@ -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 (236679 => 236680)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.h 2018-10-01 18:38:49 UTC (rev 236679)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.h 2018-10-01 18:57:02 UTC (rev 236680)
@@ -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 (236679 => 236680)
--- trunk/Tools/ChangeLog 2018-10-01 18:38:49 UTC (rev 236679)
+++ trunk/Tools/ChangeLog 2018-10-01 18:57:02 UTC (rev 236680)
@@ -1,3 +1,28 @@
+2018-10-01 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 indicated
+ 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-10-01 Daniel Bates <[email protected]>
[iOS] Special keys are misidentified in DOM keyboard events
Modified: trunk/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp (236679 => 236680)
--- trunk/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp 2018-10-01 18:38:49 UTC (rev 236679)
+++ trunk/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp 2018-10-01 18:57:02 UTC (rev 236680)
@@ -559,7 +559,7 @@
WKRetainPtr<WKStringRef> audioResultKey = adoptWK(WKStringCreateWithUTF8CString("AudioResult"));
WKDictionarySetItem(doneMessageBody.get(), audioResultKey.get(), m_audioResult.get());
- WKBundlePagePostMessage(page()->page(), doneMessageName.get(), doneMessageBody.get());
+ WKBundlePagePostMessageIgnoringFullySynchronousMode(page()->page(), doneMessageName.get(), doneMessageBody.get());
closeOtherPages();
@@ -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)
