Attachment 346753 Details for Bug 188401 – Patch

[patch] Patch

bug-188401-20180807201706.patch (text/plain), 11.19 KB, created by Wenson Hsieh on 2018-08-07 20:17:06 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2018-08-07 20:17:06 PDT

Size: 11.19 KB

patch

obsolete

>Subversion Revision: 234685
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 6d5d0624ccb43776c30b598021f2924e30391f1c..a7d5c059329b035b74d03ec81ea536916a3330d4 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,48 @@
>+2018-08-07  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] fast/events/ios/contenteditable-autocapitalize.html is a flaky failure
>+        https://bugs.webkit.org/show_bug.cgi?id=188401
>+        <rdar://problem/32542300>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When run individually, fast/events/ios/contenteditable-autocapitalize.html passes consistently; however, when
>+        run right after another layout test that finishes while the keyboard is shown, this test sometimes fails. This
>+        is because each of the three steps of this test ends when UIScriptController's `didHideKeyboardCallback` is
>+        invoked, and if the keyboard only begins to dismiss after the previous test completes, we have a race. When the
>+        keyboard finishes dismissing after the UI script is evaluated, it will trigger UI script completion early and
>+        skip over one of the steps in the layout test, resulting in a text diff failure.
>+
>+        To fix this, add a mechanism in WebKitTestRunner to wait until the keyboard is dismissed (with a short timeout)
>+        as a part of resetting test controller state, before moving on to the next layout test.
>+
>+        * WebKitTestRunner/TestController.h:
>+        * WebKitTestRunner/cocoa/TestRunnerWKWebView.h:
>+        * WebKitTestRunner/cocoa/TestRunnerWKWebView.mm:
>+        (-[TestRunnerWKWebView didStartFormControlInteraction]):
>+        (-[TestRunnerWKWebView didEndFormControlInteraction]):
>+
>+        Use these hooks to keep track of whether the previous test is presenting any form input UI.
>+
>+        (-[TestRunnerWKWebView isInteractingWithFormControl]):
>+        * WebKitTestRunner/ios/TestControllerIOS.mm:
>+        (WTR::handleKeyboardWillHideNotification):
>+        (WTR::handleKeyboardDidHideNotification):
>+        (WTR::TestController::platformInitialize):
>+        (WTR::TestController::platformDestroy):
>+
>+        Register during initialization (and unregister during teardown) for keyboard hiding notifications, to keep track
>+        of when the keyboard dismissal animation ends.
>+
>+        (WTR::TestController::platformResetStateToConsistentValues):
>+
>+        Make a couple of tweaks here: (1) if form input UI is being shown, tell the web view to resign first responder,
>+        which causes the field to lose focus. (2) If necessary, wait for the current keyboard dismissal animation to
>+        finish. This includes any keyboard dismissal animations triggered as a result of step (1).
>+
>+        (WTR::TestController::willHideKeyboard):
>+        (WTR::TestController::didHideKeyboard):
>+
> 2018-08-05  Darin Adler  <darin@apple.com>
> 
>         [Cocoa] More tweaks and refactoring to prepare for ARC
>diff --git a/Tools/WebKitTestRunner/TestController.h b/Tools/WebKitTestRunner/TestController.h
>index 42a2b6f74227b0dec77743c72c94ba34ce25f76b..b963a2856b08e843cbd454c45d18a919e50009a0 100644
>--- a/Tools/WebKitTestRunner/TestController.h
>+++ b/Tools/WebKitTestRunner/TestController.h
>@@ -219,6 +219,11 @@ public:
> 
>     void injectUserScript(WKStringRef);
> 
>+#if PLATFORM(IOS)
>+    void willHideKeyboard();
>+    void didHideKeyboard();
>+#endif
>+
> private:
>     WKRetainPtr<WKPageConfigurationRef> generatePageConfiguration(WKContextConfigurationRef);
>     WKRetainPtr<WKContextConfigurationRef> generateContextConfiguration() const;
>@@ -437,6 +442,10 @@ private:
> 
>     bool m_didReceiveServerRedirectForProvisionalNavigation { false };
> 
>+#if PLATFORM(IOS)
>+    bool m_isDoneWaitingForKeyboardToDismiss { true };
>+#endif
>+
>     WKRetainPtr<WKArrayRef> m_openPanelFileURLs;
> 
>     std::unique_ptr<EventSenderProxy> m_eventSenderProxy;
>diff --git a/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.h b/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.h
>index 4ed03771d8d22d55409a3c8667319025a9a612aa..b9b80470e144a6eb2b72d833246b2445e6250c4a 100644
>--- a/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.h
>+++ b/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.h
>@@ -50,6 +50,7 @@
> @property (nonatomic, assign) UIEdgeInsets overrideSafeAreaInsets;
> 
> @property (nonatomic, assign) BOOL usesSafariLikeRotation;
>+@property (nonatomic, readonly, getter=isInteractingWithFormControl) BOOL interactingWithFormControl;
> 
> #endif
> 
>diff --git a/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm b/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm
>index 7ff7fd5cf1260eb901363c8c4ef9a9403b7c350f..9bb53cd9cf853fcef570a17f9a2b8aaaa6ac4595 100644
>--- a/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm
>+++ b/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm
>@@ -49,6 +49,7 @@
> 
> @interface TestRunnerWKWebView () <WKUIDelegatePrivate> {
>     RetainPtr<NSNumber *> m_stableStateOverride;
>+    BOOL m_isInteractingWithFormControl;
> }
> 
> @property (nonatomic, copy) void (^zoomToScaleCompletionHandler)(void);
>@@ -103,16 +104,25 @@
> 
> - (void)didStartFormControlInteraction
> {
>+    m_isInteractingWithFormControl = YES;
>+
>     if (self.didStartFormControlInteractionCallback)
>         self.didStartFormControlInteractionCallback();
> }
> 
> - (void)didEndFormControlInteraction
> {
>+    m_isInteractingWithFormControl = NO;
>+
>     if (self.didEndFormControlInteractionCallback)
>         self.didEndFormControlInteractionCallback();
> }
> 
>+- (BOOL)isInteractingWithFormControl
>+{
>+    return m_isInteractingWithFormControl;
>+}
>+
> - (void)_didShowForcePressPreview
> {
>     if (self.didShowForcePressPreviewCallback)
>diff --git a/Tools/WebKitTestRunner/ios/TestControllerIOS.mm b/Tools/WebKitTestRunner/ios/TestControllerIOS.mm
>index fb9009016362ad73e07c613b7a9ddf826aee4b61..e4d87531a14d6a4723300ffb67688b45cfd2fea9 100644
>--- a/Tools/WebKitTestRunner/ios/TestControllerIOS.mm
>+++ b/Tools/WebKitTestRunner/ios/TestControllerIOS.mm
>@@ -45,6 +45,16 @@
> 
> namespace WTR {
> 
>+static void handleKeyboardWillHideNotification(CFNotificationCenterRef, void* observer, CFStringRef, const void*, CFDictionaryRef)
>+{
>+    static_cast<TestController*>(observer)->willHideKeyboard();
>+}
>+
>+static void handleKeyboardDidHideNotification(CFNotificationCenterRef, void* observer, CFStringRef, const void*, CFDictionaryRef)
>+{
>+    static_cast<TestController*>(observer)->didHideKeyboard();
>+}
>+
> void TestController::notifyDone()
> {
> }
>@@ -56,11 +66,19 @@ void TestController::platformInitialize()
> 
>     [UIApplication sharedApplication].idleTimerDisabled = YES;
>     [[UIScreen mainScreen] _setScale:2.0];
>+
>+    auto center = CFNotificationCenterGetLocalCenter();
>+    CFNotificationCenterAddObserver(center, this, handleKeyboardWillHideNotification, (CFStringRef)UIKeyboardWillHideNotification, nullptr, CFNotificationSuspensionBehaviorDeliverImmediately);
>+    CFNotificationCenterAddObserver(center, this, handleKeyboardDidHideNotification, (CFStringRef)UIKeyboardDidHideNotification, nullptr, CFNotificationSuspensionBehaviorDeliverImmediately);
> }
> 
> void TestController::platformDestroy()
> {
>     tearDownIOSLayoutTestCommunication();
>+
>+    auto center = CFNotificationCenterGetLocalCenter();
>+    CFNotificationCenterRemoveObserver(center, this, (CFStringRef)UIKeyboardWillHideNotification, nullptr);
>+    CFNotificationCenterRemoveObserver(center, this, (CFStringRef)UIKeyboardDidHideNotification, nullptr);
> }
> 
> void TestController::initializeInjectedBundlePath()
>@@ -98,7 +116,12 @@ void TestController::platformResetStateToConsistentValues()
>         [scrollView _removeAllAnimations:YES];
>         [scrollView setZoomScale:1 animated:NO];
>         [scrollView setContentOffset:CGPointZero];
>+
>+        if (webView.interactingWithFormControl)
>+            [webView resignFirstResponder];
>     }
>+
>+    runUntil(m_isDoneWaitingForKeyboardToDismiss, m_currentInvocation->shortTimeout());
> }
> 
> void TestController::platformConfigureViewForTest(const TestInvocation& test)
>@@ -155,4 +178,14 @@ void TestController::setHidden(bool)
>     // FIXME: implement for iOS
> }
> 
>+void TestController::willHideKeyboard()
>+{
>+    m_isDoneWaitingForKeyboardToDismiss = false;
>+}
>+
>+void TestController::didHideKeyboard()
>+{
>+    m_isDoneWaitingForKeyboardToDismiss = true;
>+}
>+
> } // namespace WTR
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index bf5e2df40039b78860231f1a7448ec919e381783..46efb82ef55ef91893e3fd66ef8d253376c03f24 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,17 @@
>+2018-08-07  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] fast/events/ios/contenteditable-autocapitalize.html is a flaky failure
>+        https://bugs.webkit.org/show_bug.cgi?id=188401
>+        <rdar://problem/32542300>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Minor tweaks to make this test a bit easier to follow. Use async-await for each step of the test, and pass in
>+        the current autocapitalization type to `runTestWithAutocapitalizeType` rather than the next type. See Tools
>+        ChangeLog for more details.
>+
>+        * fast/events/ios/contenteditable-autocapitalize.html:
>+
> 2018-08-07  Chris Dumez  <cdumez@apple.com>
> 
>         navigator.sendBeacon does not work in pagehide callbacks
>diff --git a/LayoutTests/fast/events/ios/contenteditable-autocapitalize.html b/LayoutTests/fast/events/ios/contenteditable-autocapitalize.html
>index 0f3362ff67adb59ae3799899e14203a994914077..01af70a51eee2702304e8d22f5e36d22f2b38901 100644
>--- a/LayoutTests/fast/events/ios/contenteditable-autocapitalize.html
>+++ b/LayoutTests/fast/events/ios/contenteditable-autocapitalize.html
>@@ -19,12 +19,12 @@
>                 resolveExpectedInputEvents();
>         }
> 
>-        function runUIScriptAndExpectInputEvents(inputEventCount, nextAutocapitalizeType)
>+        function runTestWithAutocapitalizeType(autocapitalizeType, inputEventCount)
>         {
>+            editable.autocapitalize = autocapitalizeType;
>             remainingInputEventCount = inputEventCount;
>             resolveExpectedInputEvents = () => {
>                 write(`With autocapitalize: ${editable.autocapitalize}, the output is: "${editable.textContent}"`);
>-                editable.autocapitalize = nextAutocapitalizeType;
>                 editable.textContent = "";
>                 editable.blur();
>             };
>@@ -45,15 +45,15 @@
>             });
>         }
> 
>-        function runTest()
>+        async function runTest()
>         {
>             if (!window.testRunner || !testRunner.runUIScript)
>                 return;
> 
>-            runUIScriptAndExpectInputEvents(2, "sentences")
>-                .then(() => runUIScriptAndExpectInputEvents(2, "characters"))
>-                .then(() => runUIScriptAndExpectInputEvents(2, null))
>-                .then(() => testRunner.notifyDone());
>+            await runTestWithAutocapitalizeType("none", 2);
>+            await runTestWithAutocapitalizeType("sentences", 2);
>+            await runTestWithAutocapitalizeType("characters", 2);
>+            testRunner.notifyDone();
>         }
>     </script>
>     <style>
>@@ -68,7 +68,7 @@
> </head>
> 
> <body onload=runTest()>
>-    <div contenteditable autocapitalize="none" id="editable" oninput=handleInput()></div>
>+    <div contenteditable id="editable" oninput=handleInput()></div>
>     <p>To manually test, type 't' into the contenteditable. The 't' should not be autocapitalized.</p>
>     <div id="output"></div>
> </body>

Actions: View | Formatted Diff | Diff

Attachments on bug 188401: 346753 | 346755 | 346769