Diff
Modified: branches/safari-611-branch/Source/WebKit/ChangeLog (276156 => 276157)
--- branches/safari-611-branch/Source/WebKit/ChangeLog 2021-04-16 19:26:13 UTC (rev 276156)
+++ branches/safari-611-branch/Source/WebKit/ChangeLog 2021-04-16 19:26:19 UTC (rev 276157)
@@ -1,3 +1,79 @@
+2021-04-16 Russell Epstein <[email protected]>
+
+ Cherry-pick r274565. rdar://problem/76412930
+
+ Maybe-regression(STP121): window.open flakily returning null
+ https://bugs.webkit.org/show_bug.cgi?id=222590
+ <rdar://problem/75211786>
+
+ Reviewed by Geoffrey Garen.
+
+ Source/WebKit:
+
+ This was an IPC ordering bug. WebPageProxy::DidCommitLoadForFrame is async and WebPageProxy::CreateNewPage is
+ sync. As a result, it was possible for the WebPageProxy::CreateNewPage to get processed *BEFORE* the
+ WebPageProxy::DidCommitLoadForFrame IPC. This was causing trouble because Safari rejects the popup opening if
+ the main frame is doing a provisional load.
+
+ To address the issue, introduce a new IPC::SendSyncOption::MaintainOrderingWithAsyncMessages flag and
+ use it on WebPageProxy::CreateNewPage sync IPC so that it gets processed in order with surrounding async
+ messages.
+
+ * Platform/IPC/Connection.cpp:
+ (IPC::Connection::SyncMessageState::processIncomingMessage):
+ (IPC::Connection::sendSyncMessage):
+ * Platform/IPC/Connection.h:
+ * Platform/IPC/Decoder.cpp:
+ (IPC::Decoder::shouldMaintainOrderingWithAsyncMessages const):
+ * Platform/IPC/Decoder.h:
+ * Platform/IPC/Encoder.cpp:
+ (IPC::Encoder::setShouldMaintainOrderingWithAsyncMessages):
+ * Platform/IPC/Encoder.h:
+ * Platform/IPC/MessageFlags.h:
+ * WebProcess/WebCoreSupport/WebChromeClient.cpp:
+ (WebKit::WebChromeClient::createWindow):
+
+ Tools:
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKit/ModalAlertsSPI.cpp:
+ (TestWebKitAPI::TEST):
+ (TestWebKitAPI::checkFrameLoadStateAndCreateNewPage):
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@274565 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2021-03-17 Chris Dumez <[email protected]>
+
+ Maybe-regression(STP121): window.open flakily returning null
+ https://bugs.webkit.org/show_bug.cgi?id=222590
+ <rdar://problem/75211786>
+
+ Reviewed by Geoffrey Garen.
+
+ This was an IPC ordering bug. WebPageProxy::DidCommitLoadForFrame is async and WebPageProxy::CreateNewPage is
+ sync. As a result, it was possible for the WebPageProxy::CreateNewPage to get processed *BEFORE* the
+ WebPageProxy::DidCommitLoadForFrame IPC. This was causing trouble because Safari rejects the popup opening if
+ the main frame is doing a provisional load.
+
+ To address the issue, introduce a new IPC::SendSyncOption::MaintainOrderingWithAsyncMessages flag and
+ use it on WebPageProxy::CreateNewPage sync IPC so that it gets processed in order with surrounding async
+ messages.
+
+ * Platform/IPC/Connection.cpp:
+ (IPC::Connection::SyncMessageState::processIncomingMessage):
+ (IPC::Connection::sendSyncMessage):
+ * Platform/IPC/Connection.h:
+ * Platform/IPC/Decoder.cpp:
+ (IPC::Decoder::shouldMaintainOrderingWithAsyncMessages const):
+ * Platform/IPC/Decoder.h:
+ * Platform/IPC/Encoder.cpp:
+ (IPC::Encoder::setShouldMaintainOrderingWithAsyncMessages):
+ * Platform/IPC/Encoder.h:
+ * Platform/IPC/MessageFlags.h:
+ * WebProcess/WebCoreSupport/WebChromeClient.cpp:
+ (WebKit::WebChromeClient::createWindow):
+
2021-04-15 Russell Epstein <[email protected]>
Cherry-pick r275846. rdar://problem/76727548
Modified: branches/safari-611-branch/Source/WebKit/Platform/IPC/Connection.cpp (276156 => 276157)
--- branches/safari-611-branch/Source/WebKit/Platform/IPC/Connection.cpp 2021-04-16 19:26:13 UTC (rev 276156)
+++ branches/safari-611-branch/Source/WebKit/Platform/IPC/Connection.cpp 2021-04-16 19:26:19 UTC (rev 276157)
@@ -147,6 +147,12 @@
{
auto locker = holdLock(m_mutex);
shouldDispatch = m_didScheduleDispatchMessagesWorkSet.add(&connection).isNewEntry;
+ ASSERT(connection.m_incomingMessagesMutex.isHeld());
+ if (message->shouldMaintainOrderingWithAsyncMessages()) {
+ // This sync message should maintain ordering with async messages so we need to process the pending async messages first.
+ while (!connection.m_incomingMessages.isEmpty())
+ m_messagesToDispatchWhileWaitingForSyncReply.append(ConnectionAndIncomingMessage { connection, connection.m_incomingMessages.takeFirst() });
+ }
m_messagesToDispatchWhileWaitingForSyncReply.append(ConnectionAndIncomingMessage { connection, WTFMove(message) });
}
@@ -614,6 +620,9 @@
if (sendSyncOptions.contains(SendSyncOption::ForceDispatchWhenDestinationIsWaitingForUnboundedSyncReply))
sendOptions = sendOptions | IPC::SendOption::DispatchMessageEvenWhenWaitingForUnboundedSyncReply;
+ if (sendSyncOptions.contains(IPC::SendSyncOption::MaintainOrderingWithAsyncMessages))
+ encoder->setShouldMaintainOrderingWithAsyncMessages();
+
auto messageName = encoder->messageName();
sendMessage(WTFMove(encoder), sendOptions);
Modified: branches/safari-611-branch/Source/WebKit/Platform/IPC/Connection.h (276156 => 276157)
--- branches/safari-611-branch/Source/WebKit/Platform/IPC/Connection.h 2021-04-16 19:26:13 UTC (rev 276156)
+++ branches/safari-611-branch/Source/WebKit/Platform/IPC/Connection.h 2021-04-16 19:26:19 UTC (rev 276157)
@@ -69,6 +69,7 @@
InformPlatformProcessWillSuspend = 1 << 0,
UseFullySynchronousModeForTesting = 1 << 1,
ForceDispatchWhenDestinationIsWaitingForUnboundedSyncReply = 1 << 2,
+ MaintainOrderingWithAsyncMessages = 1 << 3,
};
enum class WaitForOption {
Modified: branches/safari-611-branch/Source/WebKit/Platform/IPC/Decoder.cpp (276156 => 276157)
--- branches/safari-611-branch/Source/WebKit/Platform/IPC/Decoder.cpp 2021-04-16 19:26:13 UTC (rev 276156)
+++ branches/safari-611-branch/Source/WebKit/Platform/IPC/Decoder.cpp 2021-04-16 19:26:19 UTC (rev 276157)
@@ -136,6 +136,11 @@
return m_messageFlags.contains(MessageFlags::UseFullySynchronousModeForTesting);
}
+bool Decoder::shouldMaintainOrderingWithAsyncMessages() const
+{
+ return m_messageFlags.contains(MessageFlags::MaintainOrderingWithAsyncMessages);
+}
+
#if PLATFORM(MAC)
void Decoder::setImportanceAssertion(std::unique_ptr<ImportanceAssertion> assertion)
{
Modified: branches/safari-611-branch/Source/WebKit/Platform/IPC/Decoder.h (276156 => 276157)
--- branches/safari-611-branch/Source/WebKit/Platform/IPC/Decoder.h 2021-04-16 19:26:13 UTC (rev 276156)
+++ branches/safari-611-branch/Source/WebKit/Platform/IPC/Decoder.h 2021-04-16 19:26:19 UTC (rev 276157)
@@ -60,6 +60,7 @@
bool isSyncMessage() const { return messageIsSync(messageName()); }
ShouldDispatchWhenWaitingForSyncReply shouldDispatchMessageWhenWaitingForSyncReply() const;
bool shouldUseFullySynchronousModeForTesting() const;
+ bool shouldMaintainOrderingWithAsyncMessages() const;
#if PLATFORM(MAC)
void setImportanceAssertion(std::unique_ptr<ImportanceAssertion>);
Modified: branches/safari-611-branch/Source/WebKit/Platform/IPC/Encoder.cpp (276156 => 276157)
--- branches/safari-611-branch/Source/WebKit/Platform/IPC/Encoder.cpp 2021-04-16 19:26:13 UTC (rev 276156)
+++ branches/safari-611-branch/Source/WebKit/Platform/IPC/Encoder.cpp 2021-04-16 19:26:19 UTC (rev 276157)
@@ -122,6 +122,11 @@
messageFlags().add(MessageFlags::UseFullySynchronousModeForTesting);
}
+void Encoder::setShouldMaintainOrderingWithAsyncMessages()
+{
+ messageFlags().add(MessageFlags::MaintainOrderingWithAsyncMessages);
+}
+
void Encoder::wrapForTesting(std::unique_ptr<Encoder> original)
{
ASSERT(isSyncMessage());
Modified: branches/safari-611-branch/Source/WebKit/Platform/IPC/Encoder.h (276156 => 276157)
--- branches/safari-611-branch/Source/WebKit/Platform/IPC/Encoder.h 2021-04-16 19:26:13 UTC (rev 276156)
+++ branches/safari-611-branch/Source/WebKit/Platform/IPC/Encoder.h 2021-04-16 19:26:19 UTC (rev 276157)
@@ -56,6 +56,7 @@
ShouldDispatchWhenWaitingForSyncReply shouldDispatchMessageWhenWaitingForSyncReply() const;
void setFullySynchronousModeForTesting();
+ void setShouldMaintainOrderingWithAsyncMessages();
void wrapForTesting(std::unique_ptr<Encoder>);
Modified: branches/safari-611-branch/Source/WebKit/Platform/IPC/MessageFlags.h (276156 => 276157)
--- branches/safari-611-branch/Source/WebKit/Platform/IPC/MessageFlags.h 2021-04-16 19:26:13 UTC (rev 276156)
+++ branches/safari-611-branch/Source/WebKit/Platform/IPC/MessageFlags.h 2021-04-16 19:26:19 UTC (rev 276157)
@@ -31,6 +31,7 @@
DispatchMessageWhenWaitingForSyncReply = 1 << 0,
DispatchMessageWhenWaitingForUnboundedSyncReply = 1 << 1,
UseFullySynchronousModeForTesting = 1 << 2,
+ MaintainOrderingWithAsyncMessages = 1 << 3,
};
enum class ShouldDispatchWhenWaitingForSyncReply : uint8_t {
@@ -48,7 +49,8 @@
IPC::MessageFlags,
IPC::MessageFlags::DispatchMessageWhenWaitingForSyncReply,
IPC::MessageFlags::DispatchMessageWhenWaitingForUnboundedSyncReply,
- IPC::MessageFlags::UseFullySynchronousModeForTesting
+ IPC::MessageFlags::UseFullySynchronousModeForTesting,
+ IPC::MessageFlags::MaintainOrderingWithAsyncMessages
>;
};
Modified: branches/safari-611-branch/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp (276156 => 276157)
--- branches/safari-611-branch/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp 2021-04-16 19:26:13 UTC (rev 276156)
+++ branches/safari-611-branch/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp 2021-04-16 19:26:19 UTC (rev 276157)
@@ -280,7 +280,7 @@
Optional<PageIdentifier> newPageID;
Optional<WebPageCreationParameters> parameters;
- if (!webProcess.parentProcessConnection()->sendSync(Messages::WebPageProxy::CreateNewPage(webFrame->info(), webFrame->page()->webPageProxyIdentifier(), navigationAction.resourceRequest(), windowFeatures, navigationActionData), Messages::WebPageProxy::CreateNewPage::Reply(newPageID, parameters), m_page.identifier()))
+ if (!webProcess.parentProcessConnection()->sendSync(Messages::WebPageProxy::CreateNewPage(webFrame->info(), webFrame->page()->webPageProxyIdentifier(), navigationAction.resourceRequest(), windowFeatures, navigationActionData), Messages::WebPageProxy::CreateNewPage::Reply(newPageID, parameters), m_page.identifier(), IPC::Timeout::infinity(), IPC::SendSyncOption::MaintainOrderingWithAsyncMessages))
return nullptr;
if (!newPageID)
Modified: branches/safari-611-branch/Tools/ChangeLog (276156 => 276157)
--- branches/safari-611-branch/Tools/ChangeLog 2021-04-16 19:26:13 UTC (rev 276156)
+++ branches/safari-611-branch/Tools/ChangeLog 2021-04-16 19:26:19 UTC (rev 276157)
@@ -1,3 +1,62 @@
+2021-04-16 Russell Epstein <[email protected]>
+
+ Cherry-pick r274565. rdar://problem/76412930
+
+ Maybe-regression(STP121): window.open flakily returning null
+ https://bugs.webkit.org/show_bug.cgi?id=222590
+ <rdar://problem/75211786>
+
+ Reviewed by Geoffrey Garen.
+
+ Source/WebKit:
+
+ This was an IPC ordering bug. WebPageProxy::DidCommitLoadForFrame is async and WebPageProxy::CreateNewPage is
+ sync. As a result, it was possible for the WebPageProxy::CreateNewPage to get processed *BEFORE* the
+ WebPageProxy::DidCommitLoadForFrame IPC. This was causing trouble because Safari rejects the popup opening if
+ the main frame is doing a provisional load.
+
+ To address the issue, introduce a new IPC::SendSyncOption::MaintainOrderingWithAsyncMessages flag and
+ use it on WebPageProxy::CreateNewPage sync IPC so that it gets processed in order with surrounding async
+ messages.
+
+ * Platform/IPC/Connection.cpp:
+ (IPC::Connection::SyncMessageState::processIncomingMessage):
+ (IPC::Connection::sendSyncMessage):
+ * Platform/IPC/Connection.h:
+ * Platform/IPC/Decoder.cpp:
+ (IPC::Decoder::shouldMaintainOrderingWithAsyncMessages const):
+ * Platform/IPC/Decoder.h:
+ * Platform/IPC/Encoder.cpp:
+ (IPC::Encoder::setShouldMaintainOrderingWithAsyncMessages):
+ * Platform/IPC/Encoder.h:
+ * Platform/IPC/MessageFlags.h:
+ * WebProcess/WebCoreSupport/WebChromeClient.cpp:
+ (WebKit::WebChromeClient::createWindow):
+
+ Tools:
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKit/ModalAlertsSPI.cpp:
+ (TestWebKitAPI::TEST):
+ (TestWebKitAPI::checkFrameLoadStateAndCreateNewPage):
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@274565 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2021-03-17 Chris Dumez <[email protected]>
+
+ Maybe-regression(STP121): window.open flakily returning null
+ https://bugs.webkit.org/show_bug.cgi?id=222590
+ <rdar://problem/75211786>
+
+ Reviewed by Geoffrey Garen.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKit/ModalAlertsSPI.cpp:
+ (TestWebKitAPI::TEST):
+ (TestWebKitAPI::checkFrameLoadStateAndCreateNewPage):
+
2021-04-15 Russell Epstein <[email protected]>
Cherry-pick r275656. rdar://problem/76727574
Modified: branches/safari-611-branch/Tools/TestWebKitAPI/Tests/WebKit/ModalAlertsSPI.cpp (276156 => 276157)
--- branches/safari-611-branch/Tools/TestWebKitAPI/Tests/WebKit/ModalAlertsSPI.cpp 2021-04-16 19:26:13 UTC (rev 276156)
+++ branches/safari-611-branch/Tools/TestWebKitAPI/Tests/WebKit/ModalAlertsSPI.cpp 2021-04-16 19:26:19 UTC (rev 276157)
@@ -137,8 +137,38 @@
WKPageLoadURL(webView.page(), url.get());
Util::run(&done);
+ openedWebView = nil;
}
+static WKPageRef checkFrameLoadStateAndCreateNewPage(WKPageRef page, WKURLRequestRef urlRequest, WKDictionaryRef features, WKEventModifiers modifiers, WKEventMouseButton mouseButton, const void *clientInfo)
+{
+ auto mainFrame = WKPageGetMainFrame(page);
+ ASSERT(mainFrame);
+ EXPECT_EQ(kWKFrameLoadStateCommitted, WKFrameGetFrameLoadState(mainFrame));
+ done = true;
+ return createNewPage(page, urlRequest, features, modifiers, mouseButton, clientInfo);
+}
+
+TEST(WebKit, CreateNewPageDelegateFrameLoadState)
+{
+ for (unsigned i = 0; i < 25; ++i) {
+ done = false;
+ auto context = adoptWK(WKContextCreateWithConfiguration(nullptr));
+ PlatformWebView webView(context.get());
+
+ WKPageUIClientV5 uiClient;
+ memset(&uiClient, 0, sizeof(uiClient));
+ uiClient.base.version = 5;
+ uiClient.createNewPage = checkFrameLoadStateAndCreateNewPage;
+ WKPageSetPageUIClient(webView.page(), &uiClient.base);
+
+ auto htmlString = Util::toWK("<script>open('about:blank', '_blank')</script>");
+ WKPageLoadHTMLString(webView.page(), htmlString.get(), nullptr);
+ Util::run(&done);
+ openedWebView = nil;
+ }
+}
+
} // namespace TestWebKitAPI
#endif
