[iOS] fast/events/ios/contenteditable-autocapitalize.html is a flaky failure
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 8 Aug 2018 15:06:38 +0000 (15:06 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 8 Aug 2018 15:06:38 +0000 (15:06 +0000)
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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@234691 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/events/ios/contenteditable-autocapitalize.html
LayoutTests/platform/ios/TestExpectations
Tools/ChangeLog
Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.h
Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm
Tools/WebKitTestRunner/ios/TestControllerIOS.mm

index 0d32781..1103a97 100644 (file)
@@ -1,3 +1,20 @@
+2018-08-08  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 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  <rego@igalia.com>
 
         [css-grid] Update behavior of percentage row tracks and gutters
index 0f3362f..01af70a 100644 (file)
                 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();
             };
             });
         }
 
-        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>
index 36944fa..8c7ebed 100644 (file)
@@ -3078,9 +3078,6 @@ webkit.org/b/155092 js/arraybuffer-wrappers.html [ Pass Timeout ]
 
 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.
index 6d5d062..5beae2a 100644 (file)
@@ -1,3 +1,44 @@
+2018-08-08  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 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  <darin@apple.com>
 
         [Cocoa] More tweaks and refactoring to prepare for ARC
index 4ed0377..b9b8047 100644 (file)
@@ -50,6 +50,7 @@
 @property (nonatomic, assign) UIEdgeInsets overrideSafeAreaInsets;
 
 @property (nonatomic, assign) BOOL usesSafariLikeRotation;
+@property (nonatomic, readonly, getter=isInteractingWithFormControl) BOOL interactingWithFormControl;
 
 #endif
 
index 7ff7fd5..9bb53cd 100644 (file)
@@ -49,6 +49,7 @@
 
 @interface TestRunnerWKWebView () <WKUIDelegatePrivate> {
     RetainPtr<NSNumber *> m_stableStateOverride;
+    BOOL m_isInteractingWithFormControl;
 }
 
 @property (nonatomic, copy) void (^zoomToScaleCompletionHandler)(void);
 
 - (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)
index fb90090..f28a942 100644 (file)
 
 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 @@ 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 +118,12 @@ void TestController::platformResetStateToConsistentValues()
         [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)