Changeset 259551 in webkit

Timestamp:

Apr 5, 2020 9:49:59 AM (4 years ago)

Author:

dbates@webkit.org

Message:

RequestTextInputContext.Simple iframe sub-tests may sometimes fail
​https://bugs.webkit.org/show_bug.cgi?id=210010
<rdar://problem/61303697>

Reviewed by Simon Fraser and Darin Adler.

Make the iframe sub tests wait until the DOM Load event has fired in the main frame.
When it fires this means that all immediate child frames (like the one added by these
sub tests) have fired their DOM Load event. This ensures deterministic results.

Also while I am here, extract out the iframe sub-tests into their own TEST() method
for testing and debugging convenience. I also fixed up -performAfterLoading so that
it wouldn't leak the user script string and made it work if called more than once in the
same TEST() by caching this script and only adding it to the user content controller
once. Moreover, removed duplication by writing it in terms of -performAfterReceivingMessage.

• TestWebKitAPI/Tests/WebKitCocoa/RequestTextInputContext.mm:

(TEST):
(webViewLoadHTMLStringAndWaitForDOMLoadEvent): Helper function to load an HTML string
into a web view and wait until a DOM Load event is fired at the main frame.

• TestWebKitAPI/cocoa/TestWKWebView.h:
• TestWebKitAPI/cocoa/TestWKWebView.mm:

(-[TestWKWebView performAfterLoading:]): Modernized and simplified. Modified user script
to use addEventListener() instead of onload as the latter overrides any existing load handler.
This user script is also added only to the main frame since all existing callers just
want to know that all main frame and immediate child frames loaded. Also listen for the load
event during the capture phase, which is a tiny bit faster to receive, instead of the bubbling
phase. See second paragraph above for more details on the changes I made to this function.

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• TestWebKitAPI/Tests/WebKitCocoa/RequestTextInputContext.mm (modified) (2 diffs)
• TestWebKitAPI/cocoa/TestWKWebView.h (modified) (1 diff)
• TestWebKitAPI/cocoa/TestWKWebView.mm (modified) (2 diffs)

trunk/Tools/ChangeLog

@@ +1 @@
+2020-04-05  Daniel Bates  <dabates@apple.com>
+
+        RequestTextInputContext.Simple iframe sub-tests may sometimes fail
+        https://bugs.webkit.org/show_bug.cgi?id=210010
+        <rdar://problem/61303697>
+
+        Reviewed by Simon Fraser and Darin Adler.
+
+        Make the iframe sub tests wait until the DOM Load event has fired in the main frame.
+        When it fires this means that all immediate child frames (like the one added by these
+        sub tests) have fired their DOM Load event. This ensures deterministic results.
+
+        Also while I am here, extract out the iframe sub-tests into their own TEST() method
+        for testing and debugging convenience. I also fixed up -performAfterLoading so that
+        it wouldn't leak the user script string and made it work if called more than once in the
+        same TEST() by caching this script and only adding it to the user content controller
+        once. Moreover, removed duplication by writing it in terms of -performAfterReceivingMessage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/RequestTextInputContext.mm:
+        (TEST):
+        (webViewLoadHTMLStringAndWaitForDOMLoadEvent): Helper function to load an HTML string
+        into a web view and wait until a DOM Load event is fired at the main frame.
+        * TestWebKitAPI/cocoa/TestWKWebView.h:
+        * TestWebKitAPI/cocoa/TestWKWebView.mm:
+        (-[TestWKWebView performAfterLoading:]): Modernized and simplified. Modified user script
+        to use addEventListener() instead of onload as the latter overrides any existing load handler.
+        This user script is also added only to the main frame since all existing callers just
+        want to know that all main frame and immediate child frames loaded. Also listen for the load
+        event during the capture phase, which is a tiny bit faster to receive, instead of the bubbling
+        phase. See second paragraph above for more details on the changes I made to this function.
+
+
 2020-04-04  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/RequestTextInputContext.mm

@@ -98 +98 @@
     EXPECT_EQ(CGRectMake(0, 0, 100, 100), contexts[0].boundingRect);
 
-    // Basic inputs inside subframe.
-
-    [webView synchronouslyLoadHTMLString:applyIframe(@"<input type='text' style='width: 50px; height: 50px;'>")];
-    contexts = [webView synchronouslyRequestTextInputContextsInRect:[webView bounds]];
-    EXPECT_EQ(1UL, contexts.count);
-    EXPECT_EQ(CGRectMake(0, 200, 50, 50), contexts[0].boundingRect);
-
-    [webView synchronouslyLoadHTMLString:applyIframe(@"<textarea style='width: 100px; height: 100px;'></textarea>")];
-    contexts = [webView synchronouslyRequestTextInputContextsInRect:[webView bounds]];
-    EXPECT_EQ(1UL, contexts.count);
-    EXPECT_EQ(CGRectMake(0, 200, 100, 100), contexts[0].boundingRect);
-
-    [webView synchronouslyLoadHTMLString:applyIframe(@"<div contenteditable style='width: 100px; height: 100px;'></div>")];
-    contexts = [webView synchronouslyRequestTextInputContextsInRect:[webView bounds]];
-    EXPECT_EQ(1UL, contexts.count);
-    EXPECT_EQ(CGRectMake(0, 200, 100, 100), contexts[0].boundingRect);
-
     // Read only inputs; should not be included.
 
@@ -166 +149 @@
     EXPECT_EQ(CGRectMake(0, 0, 50, 50), contexts[0].boundingRect);
     EXPECT_EQ(CGRectMake(0, 0, 100, 100), contexts[1].boundingRect);
+}
+
+// Consider moving this to TestWKWebView if it could be useful to other tests.
+static void webViewLoadHTMLStringAndWaitForDOMLoadEvent(TestWKWebView *webView, NSString *htmlString)
+{
+    ASSERT(webView); // Make passing a nil web view a more obvious failure than a hang.
+    bool didFireDOMLoadEvent = false;
+    [webView performAfterLoading:[&] { didFireDOMLoadEvent = true; }];
+    [webView loadHTMLString:htmlString baseURL:nil];
+    TestWebKitAPI::Util::run(&didFireDOMLoadEvent);
+}
+
+TEST(RequestTextInputContext, Iframe)
+{
+    auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
+
+    NSArray<_WKTextInputContext *> *contexts;
+
+    webViewLoadHTMLStringAndWaitForDOMLoadEvent(webView.get(), applyIframe(@"<input type='text' style='width: 50px; height: 50px;'>"));
+    contexts = [webView synchronouslyRequestTextInputContextsInRect:[webView bounds]];
+    EXPECT_EQ(1UL, contexts.count);
+    EXPECT_EQ(CGRectMake(0, 200, 50, 50), contexts[0].boundingRect);
+
+    webViewLoadHTMLStringAndWaitForDOMLoadEvent(webView.get(), applyIframe(@"<textarea style='width: 100px; height: 100px;'></textarea>"));
+    contexts = [webView synchronouslyRequestTextInputContextsInRect:[webView bounds]];
+    EXPECT_EQ(1UL, contexts.count);
+    EXPECT_EQ(CGRectMake(0, 200, 100, 100), contexts[0].boundingRect);
+
+    webViewLoadHTMLStringAndWaitForDOMLoadEvent(webView.get(), applyIframe(@"<div contenteditable style='width: 100px; height: 100px;'></div>"));
+    contexts = [webView synchronouslyRequestTextInputContextsInRect:[webView bounds]];
+    EXPECT_EQ(1UL, contexts.count);
+    EXPECT_EQ(CGRectMake(0, 200, 100, 100), contexts[0].boundingRect);
 }
 

trunk/Tools/TestWebKitAPI/cocoa/TestWKWebView.h

@@ -79 +79 @@
 - (void)performAfterReceivingMessage:(NSString *)message action:(dispatch_block_t)action;
 - (void)waitForMessage:(NSString *)message;
+
+// This function waits until a DOM load event is fired.
+// FIXME: Rename this function to better describe what "after loading" means.
 - (void)performAfterLoading:(dispatch_block_t)actions;
+
 - (void)waitForNextPresentationUpdate;
 - (void)forceDarkMode;

trunk/Tools/TestWebKitAPI/cocoa/TestWKWebView.mm

@@ -400 +400 @@
     RetainPtr<TestWKWebViewHostWindow> _hostWindow;
     RetainPtr<TestMessageHandler> _testHandler;
+    RetainPtr<WKUserScript> _onloadScript;
 #if PLATFORM(IOS_FAMILY)
     std::unique_ptr<ClassMethodSwizzler> _sharedCalloutBarSwizzler;
@@ -503 +504 @@
 - (void)performAfterLoading:(dispatch_block_t)actions
 {
-    TestMessageHandler *handler = [[TestMessageHandler alloc] init];
-    [handler addMessage:@"loaded" withHandler:actions];
-
-    NSString *onloadScript = @"window.onload = () => window.webkit.messageHandlers.onloadHandler.postMessage('loaded')";
-    WKUserScript *script = [[WKUserScript alloc] initWithSource:onloadScript injectionTime:WKUserScriptInjectionTimeAtDocumentStart forMainFrameOnly:NO];
-
-    WKUserContentController* contentController = [[self configuration] userContentController];
-    [contentController addUserScript:script];
-    [contentController addScriptMessageHandler:handler name:@"onloadHandler"];
+    NSString *const viewDidLoadMessage = @"TestWKWebViewDidLoad";
+    if (!_onloadScript) {
+        NSString *onloadScript = [NSString stringWithFormat:@"window.addEventListener('load', () => window.webkit.messageHandlers.testHandler.postMessage('%@'), true /* useCapture */)", viewDidLoadMessage];
+        _onloadScript = adoptNS([[WKUserScript alloc] initWithSource:onloadScript injectionTime:WKUserScriptInjectionTimeAtDocumentStart forMainFrameOnly:YES]);
+        [self.configuration.userContentController addUserScript:_onloadScript.get()];
+    }
+    [self performAfterReceivingMessage:viewDidLoadMessage action:actions];
 }