Title: [282606] trunk
Revision
282606
Author
cdu...@apple.com
Date
2021-09-16 15:20:35 -0700 (Thu, 16 Sep 2021)

Log Message

[ MacOS & iOS ] imported/w3c/web-platform-tests/html/semantics/links/links-created-by-a-and-area-elements/htmlanchorelement_noopener.html is flaky failing
https://bugs.webkit.org/show_bug.cgi?id=228089
<rdar://problem/80801807>

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

Rebaseline test that is now consistently passing.

* web-platform-tests/html/semantics/links/links-created-by-a-and-area-elements/htmlanchorelement_noopener-expected.txt:

Source/WebCore:

As per the HTML specification, window.close() should schedule a task on the event loop to actually
close the window:
- https://html.spec.whatwg.org/multipage/window-object.html#dom-window-close

We were failing to do so and this was causing flakiness because event ordering was inconsistent.

This was discussed on upstream WPT here:
- https://github.com/web-platform-tests/wpt/pull/30001

No new tests, unskipped existing test.

* page/DOMWindow.cpp:
(WebCore::DOMWindow::close):

LayoutTests:

Unskip test that should no longer be flaky.

* platform/ios-wk2/TestExpectations:
* platform/mac-wk1/TestExpectations:
* platform/mac-wk2/TestExpectations:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (282605 => 282606)


--- trunk/LayoutTests/ChangeLog	2021-09-16 22:10:58 UTC (rev 282605)
+++ trunk/LayoutTests/ChangeLog	2021-09-16 22:20:35 UTC (rev 282606)
@@ -1,3 +1,17 @@
+2021-09-16  Chris Dumez  <cdu...@apple.com>
+
+        [ MacOS & iOS ] imported/w3c/web-platform-tests/html/semantics/links/links-created-by-a-and-area-elements/htmlanchorelement_noopener.html is flaky failing
+        https://bugs.webkit.org/show_bug.cgi?id=228089
+        <rdar://problem/80801807>
+
+        Reviewed by Darin Adler.
+
+        Unskip test that should no longer be flaky.
+
+        * platform/ios-wk2/TestExpectations:
+        * platform/mac-wk1/TestExpectations:
+        * platform/mac-wk2/TestExpectations:
+
 2021-09-16  Ayumi Kojima  <ayumi_koj...@apple.com>
 
         [ iOS ] http/tests/workers/service/client-removed-from-clients-while-in-page-cache.html is a flaky timeout.

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (282605 => 282606)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2021-09-16 22:10:58 UTC (rev 282605)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2021-09-16 22:20:35 UTC (rev 282606)
@@ -1,5 +1,17 @@
 2021-09-16  Chris Dumez  <cdu...@apple.com>
 
+        [ MacOS & iOS ] imported/w3c/web-platform-tests/html/semantics/links/links-created-by-a-and-area-elements/htmlanchorelement_noopener.html is flaky failing
+        https://bugs.webkit.org/show_bug.cgi?id=228089
+        <rdar://problem/80801807>
+
+        Reviewed by Darin Adler.
+
+        Rebaseline test that is now consistently passing.
+
+        * web-platform-tests/html/semantics/links/links-created-by-a-and-area-elements/htmlanchorelement_noopener-expected.txt:
+
+2021-09-16  Chris Dumez  <cdu...@apple.com>
+
         Add violations reporting support for Cross-Origin-Embedder-Policy
         https://bugs.webkit.org/show_bug.cgi?id=230269
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/links/links-created-by-a-and-area-elements/htmlanchorelement_noopener-expected.txt (282605 => 282606)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/links/links-created-by-a-and-area-elements/htmlanchorelement_noopener-expected.txt	2021-09-16 22:10:58 UTC (rev 282605)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/links/links-created-by-a-and-area-elements/htmlanchorelement_noopener-expected.txt	2021-09-16 22:20:35 UTC (rev 282606)
@@ -3,6 +3,6 @@
 PASS Check that rel=noopener with target=_self does a normal load
 PASS Check that rel=noopener with target=_parent does a normal load
 PASS Check that rel=noopener with target=_top does a normal load
-FAIL Check that targeting of rel=noopener with a given name reuses an existing window with that name assert_equals: expected object "[object Window]" but got null
+PASS Check that targeting of rel=noopener with a given name reuses an existing window with that name
 PASS Check that targeting of rel=noopener with a given name reuses an existing subframe with that name
 

Modified: trunk/LayoutTests/platform/gtk/TestExpectations (282605 => 282606)


--- trunk/LayoutTests/platform/gtk/TestExpectations	2021-09-16 22:10:58 UTC (rev 282605)
+++ trunk/LayoutTests/platform/gtk/TestExpectations	2021-09-16 22:20:35 UTC (rev 282606)
@@ -1232,8 +1232,6 @@
 
 webkit.org/b/230017 http/wpt/cross-origin-opener-policy/non-secure-to-secure-context-navigation.https.html [ Failure Pass ]
 
-webkit.org/b/228089 imported/w3c/web-platform-tests/html/semantics/links/links-created-by-a-and-area-elements/htmlanchorelement_noopener.html  [ Pass Failure ]
-
 #////////////////////////////////////////////////////////////////////////////////////////
 # End of Flaky tests
 #////////////////////////////////////////////////////////////////////////////////////////

Modified: trunk/LayoutTests/platform/ios-wk2/TestExpectations (282605 => 282606)


--- trunk/LayoutTests/platform/ios-wk2/TestExpectations	2021-09-16 22:10:58 UTC (rev 282605)
+++ trunk/LayoutTests/platform/ios-wk2/TestExpectations	2021-09-16 22:20:35 UTC (rev 282606)
@@ -1449,8 +1449,6 @@
 # Newly imported WPT test that is flaky on iOS only.
 imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-compositing-change.html [ ImageOnlyFailure Pass ]
 
-webkit.org/b/228089 imported/w3c/web-platform-tests/html/semantics/links/links-created-by-a-and-area-elements/htmlanchorelement_noopener.html [ Pass Failure ]
-
 webkit.org/b/209205 fast/events/autoscroll-in-iframe.html [ Pass Failure ]
 
 webkit.org/b/209080 imported/w3c/web-platform-tests/css/css-writing-modes/wm-propagation-body-scroll-offset-vertical-lr.html [ Failure ]

Modified: trunk/LayoutTests/platform/mac-wk1/TestExpectations (282605 => 282606)


--- trunk/LayoutTests/platform/mac-wk1/TestExpectations	2021-09-16 22:10:58 UTC (rev 282605)
+++ trunk/LayoutTests/platform/mac-wk1/TestExpectations	2021-09-16 22:20:35 UTC (rev 282606)
@@ -1333,9 +1333,6 @@
 [ BigSur ] imported/w3c/web-platform-tests/css/css-will-change/will-change-stacking-context-filter-1.html [ Pass ImageOnlyFailure ]
 [ BigSur ] imported/w3c/web-platform-tests/css/css-will-change/will-change-stacking-context-opacity-1.html [ Pass ImageOnlyFailure ]
 
-# This test relies on BroadcastChannel and has been flaky on WK1 since we added support for BroadcastChannel.
-webkit.org/b/228089 imported/w3c/web-platform-tests/html/semantics/links/links-created-by-a-and-area-elements/htmlanchorelement_noopener.html  [ Pass Failure ]
-
 # rdar://80331434 ([ Mac WK1 Release ] accessibility/loading-iframe-sends-notification.html is a flaky timeout)
 [ Release ] accessibility/loading-iframe-sends-notification.html [ Pass Timeout ]
 

Modified: trunk/LayoutTests/platform/mac-wk2/TestExpectations (282605 => 282606)


--- trunk/LayoutTests/platform/mac-wk2/TestExpectations	2021-09-16 22:10:58 UTC (rev 282605)
+++ trunk/LayoutTests/platform/mac-wk2/TestExpectations	2021-09-16 22:20:35 UTC (rev 282606)
@@ -1063,8 +1063,6 @@
 
 webkit.org/b/209006 imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/execution-timing/085.html [ Pass Failure ]
 
-webkit.org/b/228089 imported/w3c/web-platform-tests/html/semantics/links/links-created-by-a-and-area-elements/htmlanchorelement_noopener.html [ Pass Failure ]
-
 webkit.org/b/209077 svg/custom/object-sizing-explicit-width.xhtml [ Pass Failure ]
 
 webkit.org/b/209052 fast/scrolling/mac/absolute-in-overflow-scroll-dynamic.html [ Pass ImageOnlyFailure ]

Modified: trunk/Source/WebCore/ChangeLog (282605 => 282606)


--- trunk/Source/WebCore/ChangeLog	2021-09-16 22:10:58 UTC (rev 282605)
+++ trunk/Source/WebCore/ChangeLog	2021-09-16 22:20:35 UTC (rev 282606)
@@ -1,5 +1,27 @@
 2021-09-16  Chris Dumez  <cdu...@apple.com>
 
+        [ MacOS & iOS ] imported/w3c/web-platform-tests/html/semantics/links/links-created-by-a-and-area-elements/htmlanchorelement_noopener.html is flaky failing
+        https://bugs.webkit.org/show_bug.cgi?id=228089
+        <rdar://problem/80801807>
+
+        Reviewed by Darin Adler.
+
+        As per the HTML specification, window.close() should schedule a task on the event loop to actually
+        close the window:
+        - https://html.spec.whatwg.org/multipage/window-object.html#dom-window-close
+
+        We were failing to do so and this was causing flakiness because event ordering was inconsistent.
+
+        This was discussed on upstream WPT here:
+        - https://github.com/web-platform-tests/wpt/pull/30001
+
+        No new tests, unskipped existing test.
+
+        * page/DOMWindow.cpp:
+        (WebCore::DOMWindow::close):
+
+2021-09-16  Chris Dumez  <cdu...@apple.com>
+
         Add violations reporting support for Cross-Origin-Embedder-Policy
         https://bugs.webkit.org/show_bug.cgi?id=230269
 

Modified: trunk/Source/WebCore/loader/EmptyClients.h (282605 => 282606)


--- trunk/Source/WebCore/loader/EmptyClients.h	2021-09-16 22:10:58 UTC (rev 282605)
+++ trunk/Source/WebCore/loader/EmptyClients.h	2021-09-16 22:20:35 UTC (rev 282606)
@@ -87,7 +87,7 @@
     bool canRunBeforeUnloadConfirmPanel() final { return false; }
     bool runBeforeUnloadConfirmPanel(const String&, Frame&) final { return true; }
 
-    void closeWindowSoon() final { }
+    void closeWindow() final { }
 
     void runJavaScriptAlert(Frame&, const String&) final { }
     bool runJavaScriptConfirm(Frame&, const String&) final { return false; }

Modified: trunk/Source/WebCore/page/Chrome.cpp (282605 => 282606)


--- trunk/Source/WebCore/page/Chrome.cpp	2021-09-16 22:10:58 UTC (rev 282605)
+++ trunk/Source/WebCore/page/Chrome.cpp	2021-09-16 22:20:35 UTC (rev 282606)
@@ -286,9 +286,9 @@
     return m_client.runBeforeUnloadConfirmPanel(message, frame);
 }
 
-void Chrome::closeWindowSoon()
+void Chrome::closeWindow()
 {
-    m_client.closeWindowSoon();
+    m_client.closeWindow();
 }
 
 void Chrome::runJavaScriptAlert(Frame& frame, const String& message)

Modified: trunk/Source/WebCore/page/Chrome.h (282605 => 282606)


--- trunk/Source/WebCore/page/Chrome.h	2021-09-16 22:10:58 UTC (rev 282605)
+++ trunk/Source/WebCore/page/Chrome.h	2021-09-16 22:20:35 UTC (rev 282606)
@@ -140,7 +140,7 @@
     bool canRunBeforeUnloadConfirmPanel();
     bool runBeforeUnloadConfirmPanel(const String& message, Frame&);
 
-    void closeWindowSoon();
+    void closeWindow();
 
     void runJavaScriptAlert(Frame&, const String&);
     bool runJavaScriptConfirm(Frame&, const String&);

Modified: trunk/Source/WebCore/page/ChromeClient.h (282605 => 282606)


--- trunk/Source/WebCore/page/ChromeClient.h	2021-09-16 22:10:58 UTC (rev 282605)
+++ trunk/Source/WebCore/page/ChromeClient.h	2021-09-16 22:20:35 UTC (rev 282606)
@@ -185,7 +185,7 @@
     virtual bool canRunBeforeUnloadConfirmPanel() = 0;
     virtual bool runBeforeUnloadConfirmPanel(const String& message, Frame&) = 0;
 
-    virtual void closeWindowSoon() = 0;
+    virtual void closeWindow() = 0;
 
     virtual void runJavaScriptAlert(Frame&, const String&) = 0;
     virtual bool runJavaScriptConfirm(Frame&, const String&) = 0;

Modified: trunk/Source/WebCore/page/DOMWindow.cpp (282605 => 282606)


--- trunk/Source/WebCore/page/DOMWindow.cpp	2021-09-16 22:10:58 UTC (rev 282605)
+++ trunk/Source/WebCore/page/DOMWindow.cpp	2021-09-16 22:20:35 UTC (rev 282606)
@@ -1056,7 +1056,11 @@
     ResourceLoadObserver::shared().updateCentralStatisticsStore([] { });
 
     page->setIsClosing();
-    page->chrome().closeWindowSoon();
+
+    document()->eventLoop().queueTask(TaskSource::DOMManipulation, [this, protectedThis = makeRef(*this)] {
+        if (auto* page = this->page())
+            page->chrome().closeWindow();
+    });
 }
 
 void DOMWindow::print()

Modified: trunk/Source/WebCore/page/FrameTree.cpp (282605 => 282606)


--- trunk/Source/WebCore/page/FrameTree.cpp	2021-09-16 22:10:58 UTC (rev 282605)
+++ trunk/Source/WebCore/page/FrameTree.cpp	2021-09-16 22:20:35 UTC (rev 282606)
@@ -270,7 +270,7 @@
         return nullptr;
     
     for (auto& otherPage : page->group().pages()) {
-        if (&otherPage == page)
+        if (&otherPage == page || otherPage.isClosing())
             continue;
         for (auto* frame = &otherPage.mainFrame(); frame; frame = frame->tree().traverseNext()) {
             if (frame->tree().uniqueName() == name && isFrameFamiliarWith(activeFrame, *frame))

Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp (282605 => 282606)


--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp	2021-09-16 22:10:58 UTC (rev 282605)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp	2021-09-16 22:20:35 UTC (rev 282606)
@@ -421,7 +421,7 @@
     return shouldClose;
 }
 
-void WebChromeClient::closeWindowSoon()
+void WebChromeClient::closeWindow()
 {
     // FIXME: This code assumes that the client will respond to a close page
     // message by actually closing the page. Safari does this, but there is

Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h (282605 => 282606)


--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h	2021-09-16 22:10:58 UTC (rev 282605)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h	2021-09-16 22:20:35 UTC (rev 282606)
@@ -102,7 +102,7 @@
     bool canRunBeforeUnloadConfirmPanel() final;
     bool runBeforeUnloadConfirmPanel(const String& message, WebCore::Frame&) final;
     
-    void closeWindowSoon() final;
+    void closeWindow() final;
     
     void runJavaScriptAlert(WebCore::Frame&, const String&) final;
     bool runJavaScriptConfirm(WebCore::Frame&, const String&) final;

Modified: trunk/Source/WebKitLegacy/mac/WebCoreSupport/WebChromeClient.h (282605 => 282606)


--- trunk/Source/WebKitLegacy/mac/WebCoreSupport/WebChromeClient.h	2021-09-16 22:10:58 UTC (rev 282605)
+++ trunk/Source/WebKitLegacy/mac/WebCoreSupport/WebChromeClient.h	2021-09-16 22:20:35 UTC (rev 282606)
@@ -89,7 +89,7 @@
     bool canRunBeforeUnloadConfirmPanel() final;
     bool runBeforeUnloadConfirmPanel(const String& message, WebCore::Frame&) final;
 
-    void closeWindowSoon() final;
+    void closeWindow() final;
 
     void runJavaScriptAlert(WebCore::Frame&, const String&) override;
     bool runJavaScriptConfirm(WebCore::Frame&, const String&) override;

Modified: trunk/Source/WebKitLegacy/mac/WebCoreSupport/WebChromeClient.mm (282605 => 282606)


--- trunk/Source/WebKitLegacy/mac/WebCoreSupport/WebChromeClient.mm	2021-09-16 22:10:58 UTC (rev 282605)
+++ trunk/Source/WebKitLegacy/mac/WebCoreSupport/WebChromeClient.mm	2021-09-16 22:20:35 UTC (rev 282606)
@@ -467,7 +467,7 @@
     return CallUIDelegateReturningBoolean(true, m_webView, @selector(webView:runBeforeUnloadConfirmPanelWithMessage:initiatedByFrame:), message, kit(&frame));
 }
 
-void WebChromeClient::closeWindowSoon()
+void WebChromeClient::closeWindow()
 {
     // We need to remove the parent WebView from WebViewSets here, before it actually
     // closes, to make sure that _javascript_ code that executes before it closes
@@ -484,7 +484,7 @@
 
     [m_webView setGroupName:nil];
     [m_webView stopLoading:nil];
-    [m_webView performSelector:@selector(_closeWindow) withObject:nil afterDelay:0.0];
+    [m_webView _closeWindow];
 }
 
 void WebChromeClient::runJavaScriptAlert(Frame& frame, const String& message)

Modified: trunk/Source/WebKitLegacy/win/WebCoreSupport/WebChromeClient.cpp (282605 => 282606)


--- trunk/Source/WebKitLegacy/win/WebCoreSupport/WebChromeClient.cpp	2021-09-16 22:10:58 UTC (rev 282605)
+++ trunk/Source/WebKitLegacy/win/WebCoreSupport/WebChromeClient.cpp	2021-09-16 22:20:35 UTC (rev 282606)
@@ -373,7 +373,7 @@
     return !!result;
 }
 
-void WebChromeClient::closeWindowSoon()
+void WebChromeClient::closeWindow()
 {
     // We need to remove the parent WebView from WebViewSets here, before it actually
     // closes, to make sure that _javascript_ code that executes before it closes
@@ -390,7 +390,7 @@
 
     m_webView->setGroupName(0);
     m_webView->stopLoading(0);
-    m_webView->closeWindowSoon();
+    m_webView->closeWindow();
 }
 
 void WebChromeClient::runJavaScriptAlert(Frame&, const String& message)

Modified: trunk/Source/WebKitLegacy/win/WebCoreSupport/WebChromeClient.h (282605 => 282606)


--- trunk/Source/WebKitLegacy/win/WebCoreSupport/WebChromeClient.h	2021-09-16 22:10:58 UTC (rev 282605)
+++ trunk/Source/WebKitLegacy/win/WebCoreSupport/WebChromeClient.h	2021-09-16 22:20:35 UTC (rev 282606)
@@ -85,7 +85,7 @@
     bool canRunBeforeUnloadConfirmPanel() final;
     bool runBeforeUnloadConfirmPanel(const WTF::String& message, WebCore::Frame&) final;
 
-    void closeWindowSoon() final;
+    void closeWindow() final;
 
     void runJavaScriptAlert(WebCore::Frame&, const WTF::String&) final;
     bool runJavaScriptConfirm(WebCore::Frame&, const WTF::String&) final;

Modified: trunk/Source/WebKitLegacy/win/WebView.cpp (282605 => 282606)


--- trunk/Source/WebKitLegacy/win/WebView.cpp	2021-09-16 22:10:58 UTC (rev 282605)
+++ trunk/Source/WebKitLegacy/win/WebView.cpp	2021-09-16 22:20:35 UTC (rev 282606)
@@ -1456,78 +1456,6 @@
     ::GetWindowRect(m_viewWindow, rect);
 }
 
-class WindowCloseTimer final : public WebCore::SuspendableTimerBase {
-public:
-    static WindowCloseTimer* create(WebView*);
-
-private:
-    WindowCloseTimer(ScriptExecutionContext&, WebView*);
-
-    // ActiveDOMObject API.
-    void contextDestroyed() override;
-    const char* activeDOMObjectName() const override { return "WindowCloseTimer"; }
-
-    // SuspendableTimerBase API.
-    void fired() override;
-
-    WebView* m_webView;
-};
-
-WindowCloseTimer* WindowCloseTimer::create(WebView* webView)
-{
-    ASSERT_ARG(webView, webView);
-    Frame* frame = core(webView->topLevelFrame());
-    ASSERT(frame);
-    if (!frame)
-        return nullptr;
-
-    Document* document = frame->document();
-    ASSERT(document);
-    if (!document)
-        return nullptr;
-
-    auto closeTimer = new WindowCloseTimer(*document, webView);
-    closeTimer->suspendIfNeeded();
-    return closeTimer;
-}
-
-WindowCloseTimer::WindowCloseTimer(ScriptExecutionContext& context, WebView* webView)
-    : SuspendableTimerBase(&context)
-    , m_webView(webView)
-{
-    ASSERT_ARG(webView, webView);
-}
-
-void WindowCloseTimer::contextDestroyed()
-{
-    SuspendableTimerBase::contextDestroyed();
-    delete this;
-}
-
-void WindowCloseTimer::fired()
-{
-    m_webView->closeWindowTimerFired();
-}
-
-void WebView::closeWindowSoon()
-{
-    if (m_closeWindowTimer)
-        return;
-
-    m_closeWindowTimer = WindowCloseTimer::create(this);
-    if (!m_closeWindowTimer)
-        return;
-    m_closeWindowTimer->startOneShot(0_s);
-
-    AddRef();
-}
-
-void WebView::closeWindowTimerFired()
-{
-    closeWindow();
-    Release();
-}
-
 void WebView::closeWindow()
 {
     if (m_hasSpellCheckerDocumentTag) {
@@ -5557,8 +5485,7 @@
 
     settings.setShowsToolTipOverTruncatedText(enabled);
 
-    if (!m_closeWindowTimer)
-        m_mainFrame->invalidate(); // FIXME
+    m_mainFrame->invalidate(); // FIXME
 
     hr = updateSharedSettingsFromPreferencesIfNeeded(preferences.get());
     if (FAILED(hr))

Modified: trunk/Source/WebKitLegacy/win/WebView.h (282605 => 282606)


--- trunk/Source/WebKitLegacy/win/WebView.h	2021-09-16 22:10:58 UTC (rev 282605)
+++ trunk/Source/WebKitLegacy/win/WebView.h	2021-09-16 22:20:35 UTC (rev 282606)
@@ -438,8 +438,6 @@
     void repaint(const WebCore::IntRect&, bool contentChanged, bool immediate = false, bool repaintContentOnly = false);
     void frameRect(RECT* rect);
     void closeWindow();
-    void closeWindowSoon();
-    void closeWindowTimerFired();
     bool didClose() const { return m_didClose; }
 
     bool transparent() const { return m_transparent; }
@@ -680,7 +678,6 @@
 
     static bool s_allowSiteSpecificHacks;
 
-    WebCore::SuspendableTimerBase* m_closeWindowTimer { nullptr };
     std::unique_ptr<TRACKMOUSEEVENT> m_mouseOutTracker;
 
     HWND m_topLevelParent { nullptr };
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to