Title: [234691] trunk
Revision
234691
Author
[email protected]
Date
2018-08-08 08:06:38 -0700 (Wed, 08 Aug 2018)

Log Message

[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 Ryosuke Niwa.

Tools:

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/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).

LayoutTests:

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:
* platform/ios/TestExpectations:

Remove the failing test expecation.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (234690 => 234691)


--- trunk/LayoutTests/ChangeLog	2018-08-08 12:07:33 UTC (rev 234690)
+++ trunk/LayoutTests/ChangeLog	2018-08-08 15:06:38 UTC (rev 234691)
@@ -1,3 +1,20 @@
+2018-08-08  Wenson Hsieh  <[email protected]>
+
+        [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 Ryosuke Niwa.
+
+        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:
+        * platform/ios/TestExpectations:
+
+        Remove the failing test expecation.
+
 2018-08-08  Manuel Rego Casasnovas  <[email protected]>
 
         [css-grid] Update behavior of percentage row tracks and gutters

Modified: trunk/LayoutTests/fast/events/ios/contenteditable-autocapitalize.html (234690 => 234691)


--- trunk/LayoutTests/fast/events/ios/contenteditable-autocapitalize.html	2018-08-08 12:07:33 UTC (rev 234690)
+++ trunk/LayoutTests/fast/events/ios/contenteditable-autocapitalize.html	2018-08-08 15:06:38 UTC (rev 234691)
@@ -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>

Modified: trunk/LayoutTests/platform/ios/TestExpectations (234690 => 234691)


--- trunk/LayoutTests/platform/ios/TestExpectations	2018-08-08 12:07:33 UTC (rev 234690)
+++ trunk/LayoutTests/platform/ios/TestExpectations	2018-08-08 15:06:38 UTC (rev 234691)
@@ -3078,9 +3078,6 @@
 
 webkit.org/b/172052 [ Release ] imported/w3c/web-platform-tests/html/webappapis/timers/type-long-setinterval.html [ Pass Failure ]
 
-# <rdar://problem/32542300> REGRESSION (iOS 11): LayoutTest fast/events/ios/contenteditable-autocapitalize.html is failing
-fast/events/ios/contenteditable-autocapitalize.html [ Failure ]
-
 webkit.org/b/183441 mathml/presentation/multiscripts-equivalence.html [ ImageOnlyFailure ]
 
 # <rdar://problem/32632415> REGRESSION (ImageIO-1666): LayoutTests fast/images are failing together.

Modified: trunk/Tools/ChangeLog (234690 => 234691)


--- trunk/Tools/ChangeLog	2018-08-08 12:07:33 UTC (rev 234690)
+++ trunk/Tools/ChangeLog	2018-08-08 15:06:38 UTC (rev 234691)
@@ -1,3 +1,44 @@
+2018-08-08  Wenson Hsieh  <[email protected]>
+
+        [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 Ryosuke Niwa.
+
+        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/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).
+
 2018-08-05  Darin Adler  <[email protected]>
 
         [Cocoa] More tweaks and refactoring to prepare for ARC

Modified: trunk/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.h (234690 => 234691)


--- trunk/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.h	2018-08-08 12:07:33 UTC (rev 234690)
+++ trunk/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.h	2018-08-08 15:06:38 UTC (rev 234691)
@@ -50,6 +50,7 @@
 @property (nonatomic, assign) UIEdgeInsets overrideSafeAreaInsets;
 
 @property (nonatomic, assign) BOOL usesSafariLikeRotation;
+@property (nonatomic, readonly, getter=isInteractingWithFormControl) BOOL interactingWithFormControl;
 
 #endif
 

Modified: trunk/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm (234690 => 234691)


--- trunk/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm	2018-08-08 12:07:33 UTC (rev 234690)
+++ trunk/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm	2018-08-08 15:06:38 UTC (rev 234691)
@@ -49,6 +49,7 @@
 
 @interface TestRunnerWKWebView () <WKUIDelegatePrivate> {
     RetainPtr<NSNumber *> m_stableStateOverride;
+    BOOL m_isInteractingWithFormControl;
 }
 
 @property (nonatomic, copy) void (^zoomToScaleCompletionHandler)(void);
@@ -103,6 +104,8 @@
 
 - (void)didStartFormControlInteraction
 {
+    m_isInteractingWithFormControl = YES;
+
     if (self.didStartFormControlInteractionCallback)
         self.didStartFormControlInteractionCallback();
 }
@@ -109,10 +112,17 @@
 
 - (void)didEndFormControlInteraction
 {
+    m_isInteractingWithFormControl = NO;
+
     if (self.didEndFormControlInteractionCallback)
         self.didEndFormControlInteractionCallback();
 }
 
+- (BOOL)isInteractingWithFormControl
+{
+    return m_isInteractingWithFormControl;
+}
+
 - (void)_didShowForcePressPreview
 {
     if (self.didShowForcePressPreviewCallback)

Modified: trunk/Tools/WebKitTestRunner/ios/TestControllerIOS.mm (234690 => 234691)


--- trunk/Tools/WebKitTestRunner/ios/TestControllerIOS.mm	2018-08-08 12:07:33 UTC (rev 234690)
+++ trunk/Tools/WebKitTestRunner/ios/TestControllerIOS.mm	2018-08-08 15:06:38 UTC (rev 234691)
@@ -45,6 +45,18 @@
 
 namespace WTR {
 
+static bool isDoneWaitingForKeyboardToDismiss = true;
+
+static void handleKeyboardWillHideNotification(CFNotificationCenterRef, void*, CFStringRef, const void*, CFDictionaryRef)
+{
+    isDoneWaitingForKeyboardToDismiss = false;
+}
+
+static void handleKeyboardDidHideNotification(CFNotificationCenterRef, void*, CFStringRef, const void*, CFDictionaryRef)
+{
+    isDoneWaitingForKeyboardToDismiss = true;
+}
+
 void TestController::notifyDone()
 {
 }
@@ -56,11 +68,19 @@
 
     [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 +118,12 @@
         [scrollView _removeAllAnimations:YES];
         [scrollView setZoomScale:1 animated:NO];
         [scrollView setContentOffset:CGPointZero];
+
+        if (webView.interactingWithFormControl)
+            [webView resignFirstResponder];
     }
+
+    runUntil(isDoneWaitingForKeyboardToDismiss, m_currentInvocation->shortTimeout());
 }
 
 void TestController::platformConfigureViewForTest(const TestInvocation& test)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to