Attachment 346769 Details for Bug 188401 – Patch for landing

[patch] Patch for landing

bug-188401-20180808072721.patch (text/plain), 10.79 KB, created by Wenson Hsieh on 2018-08-08 07:27:21 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2018-08-08 07:27:21 PDT

Size: 10.79 KB

patch

obsolete

>Subversion Revision: 234685
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 6d5d0624ccb43776c30b598021f2924e30391f1c..5beae2a31bd37f2cfddf518eb8684bae1b306128 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -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
>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..f28a94290f3ab92d380fdfc2e405c6867f6b6704 100644
>--- a/Tools/WebKitTestRunner/ios/TestControllerIOS.mm
>+++ b/Tools/WebKitTestRunner/ios/TestControllerIOS.mm
>@@ -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 @@ 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)
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index bf5e2df40039b78860231f1a7448ec919e381783..3f6c06289584eaf5cbde7c5ef8cd1ebea6d871ad 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -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-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>
>diff --git a/LayoutTests/platform/ios/TestExpectations b/LayoutTests/platform/ios/TestExpectations
>index 36944fae98d49834b305454e8120d292029cd2d1..8c7ebedf2d606b4b4a70bb46b0e4f334fca5d14c 100644
>--- a/LayoutTests/platform/ios/TestExpectations
>+++ b/LayoutTests/platform/ios/TestExpectations
>@@ -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.

Actions: View | Formatted Diff | Diff

Attachments on bug 188401: 346753 | 346755 | 346769