Changeset 249557 in webkit

Timestamp:

Sep 5, 2019 6:07:01 PM (5 years ago)

Author:

Chris Dumez

Message:

REGRESSION: http/tests/adClickAttribution/second-attribution-converted-with-higher-priority.html and http/tests/adClickAttribution/second-attribution-converted-with-lower-priority.html are flaky timeouts
​https://bugs.webkit.org/show_bug.cgi?id=201440
<rdar://problem/54998427>

Reviewed by Alex Christensen.

Source/WebKit:

Add private WKBundlePage API to query if the WebPage is currently suspended or not.

• WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:

(WKBundlePageIsSuspended):

• WebProcess/InjectedBundle/API/c/WKBundlePagePrivate.h:

Tools:

Since r249350, it is now possible for a WebContent process to have a suspended WebPage for the current web view.
This was confusing InjectedBundle::page() in WebKitTestRunner which was assuming that the WebPage that was first
created in the WebContent process is the one that is currently running the test and that we want to talk to.
Update InjectedBundle::page() to ignore suspended WebPages in order to address the issue.

• WebKitTestRunner/InjectedBundle/InjectedBundle.cpp:

(WTR::InjectedBundle::page const):

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp (modified) (1 diff)
• Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePagePrivate.h (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-09-05  Chris Dumez  <cdumez@apple.com>
+
+        REGRESSION: http/tests/adClickAttribution/second-attribution-converted-with-higher-priority.html and http/tests/adClickAttribution/second-attribution-converted-with-lower-priority.html are flaky timeouts
+        https://bugs.webkit.org/show_bug.cgi?id=201440
+        <rdar://problem/54998427>
+
+        Reviewed by Alex Christensen.
+
+        Add private WKBundlePage API to query if the WebPage is currently suspended or not.
+
+        * WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:
+        (WKBundlePageIsSuspended):
+        * WebProcess/InjectedBundle/API/c/WKBundlePagePrivate.h:
+
 2019-09-05  Sihui Liu  <sihui_liu@apple.com>
 

trunk/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp

@@ -725 +725 @@
 }
 
+bool WKBundlePageIsSuspended(WKBundlePageRef pageRef)
+{
+    return WebKit::toImpl(pageRef)->isSuspended();
+}
+
 void WKBundlePageAddUserScript(WKBundlePageRef pageRef, WKStringRef source, _WKUserScriptInjectionTime injectionTime, WKUserContentInjectedFrames injectedFrames)
 {

trunk/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePagePrivate.h

@@ -106 +106 @@
 WK_EXPORT void WKBundlePageInsertNewlineInQuotedContent(WKBundlePageRef page);
 
+WK_EXPORT bool WKBundlePageIsSuspended(WKBundlePageRef page);
+
 // This only works if the SuppressesIncrementalRendering preference is set as well.
 typedef unsigned WKRenderingSuppressionToken;

trunk/Tools/ChangeLog

@@ +1 @@
+2019-09-05  Chris Dumez  <cdumez@apple.com>
+
+        REGRESSION: http/tests/adClickAttribution/second-attribution-converted-with-higher-priority.html and http/tests/adClickAttribution/second-attribution-converted-with-lower-priority.html are flaky timeouts
+        https://bugs.webkit.org/show_bug.cgi?id=201440
+        <rdar://problem/54998427>
+
+        Reviewed by Alex Christensen.
+
+        Since r249350, it is now possible for a WebContent process to have a suspended WebPage for the current web view.
+        This was confusing InjectedBundle::page() in WebKitTestRunner which was assuming that the WebPage that was first
+        created in the WebContent process is the one that is currently running the test and that we want to talk to.
+        Update InjectedBundle::page() to ignore suspended WebPages in order to address the issue.
+
+        * WebKitTestRunner/InjectedBundle/InjectedBundle.cpp:
+        (WTR::InjectedBundle::page const):
+
 2019-09-05  Jonathan Bedard  <jbedard@apple.com>
 

trunk/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp

@@ -165 +165 @@
 {
     // It might be better to have the UI process send over a reference to the main
-    // page instead of just assuming it's the first one.
+    // page instead of just assuming it's the first non-suspended one.
+    for (auto& page : m_pages) {
+        if (!WKBundlePageIsSuspended(page->page()))
+            return page.get();
+    }
     return m_pages[0].get();
 }