Changeset 216145 in webkit

Timestamp:

May 3, 2017 3:04:18 PM (7 years ago)

Author:

Simon Fraser

Message:

Have WKWebView call _updateVisibleContentRects for the current transaction if possible, rather than always delaying
​https://bugs.webkit.org/show_bug.cgi?id=171619
Source/WebKit2:

Also fixes webkit.org/b/170153 and webkit.org/b/170195

Reviewed by Tim Horton.

In r214391 we started adding the pre-commit handler in a dispatch_async() to ensure that
the handler would always run, since we couldn't reliably test the phase of the current
transaction. Now that problem has been solved (rdar://problem/31253952) we can go back to
checking the transaction phase on newer iOS versions. If we're too late for the current transaction
we still need to dispatch_async() to get into the next one.

• UIProcess/API/Cocoa/WKWebView.mm:

(-[WKWebView _addUpdateVisibleContentRectPreCommitHandler]):
(-[WKWebView _scheduleVisibleContentRectUpdateAfterScrollInView:]):

Tools:

​https://bugs.webkit.org/show_bug.cgi?id=170195

Reviewed by Tim Horton.

Re-enable WebKit2.ResizeWithHiddenContentDoesNotHang.

• TestWebKitAPI/Tests/WebKit2Cocoa/AnimatedResize.mm:

(TEST):

LayoutTests:

​https://bugs.webkit.org/show_bug.cgi?id=170153

Reviewed by Tim Horton.

These tests need to wait a bit for the scrolling state of the document to be updated.

• fast/scrolling/ios/touch-scroll-pointer-events-none.html:
• fast/scrolling/ios/touch-scroll-visibility-hidden.html:
• platform/ios-wk2/TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/scrolling/ios/touch-scroll-pointer-events-none.html (modified) (1 diff)
• LayoutTests/fast/scrolling/ios/touch-scroll-visibility-hidden.html (modified) (1 diff)
• LayoutTests/platform/ios-wk2/TestExpectations (modified) (1 diff)
• Source/WebKit2/ChangeLog (modified) (1 diff)
• Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm (modified) (2 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKit2Cocoa/AnimatedResize.mm (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2017-05-03  Simon Fraser  <simon.fraser@apple.com>
+
+        Have WKWebView call _updateVisibleContentRects for the current transaction if possible, rather than always delaying
+        https://bugs.webkit.org/show_bug.cgi?id=171619
+        https://bugs.webkit.org/show_bug.cgi?id=170153
+
+        Reviewed by Tim Horton.
+
+        These tests need to wait a bit for the scrolling state of the document to be updated.
+
+        * fast/scrolling/ios/touch-scroll-pointer-events-none.html:
+        * fast/scrolling/ios/touch-scroll-visibility-hidden.html:
+        * platform/ios-wk2/TestExpectations:
+
 2017-05-03  Yoav Weiss  <yoav@yoav.ws>
 

trunk/LayoutTests/fast/scrolling/ios/touch-scroll-pointer-events-none.html

@@ -67 +67 @@
             if (testRunner.runUIScript) {
                 testRunner.runUIScript(getUIScript(), function() {
-                    debug("swipe complete");
-                    shouldBe("scroller.scrollTop", "0");
-                    shouldBe("document.scrollingElement.scrollTop", "90");
-                    finishJSTest();
+                    setTimeout(function() {
+                        debug("swipe complete");
+                        shouldBe("scroller.scrollTop", "0");
+                        shouldBe("document.scrollingElement.scrollTop", "90");
+                        finishJSTest();
+                    }, 0);
                 });
             }

trunk/LayoutTests/fast/scrolling/ios/touch-scroll-visibility-hidden.html

@@ -67 +67 @@
             if (testRunner.runUIScript) {
                 testRunner.runUIScript(getUIScript(), function() {
-                    debug("swipe complete");
-                    shouldBe("scroller.scrollTop", "0");
-                    shouldBe("document.scrollingElement.scrollTop", "90");
-                    finishJSTest();
+                    setTimeout(function() {
+                        debug("swipe complete");
+                        shouldBe("scroller.scrollTop", "0");
+                        shouldBe("document.scrollingElement.scrollTop", "90");
+                        finishJSTest();
+                    }, 0);
                 });
             }

trunk/LayoutTests/platform/ios-wk2/TestExpectations

@@ -1941 +1941 @@
 webkit.org/b/169558 fast/history/ios/history-scroll-restoration.html [ Pass Failure ]
 
-webkit.org/b/170153 fast/scrolling/ios/touch-scroll-pointer-events-none.html [ Failure ]
-webkit.org/b/170153 fast/scrolling/ios/touch-scroll-visibility-hidden.html [ Failure ]
-
 webkit.org/b/169719 fast/mediacapturefromelement/CanvasCaptureMediaStream-request-frame-events.html [ Pass Failure ]
 

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2017-05-03  Simon Fraser  <simon.fraser@apple.com>
+
+        Have WKWebView call _updateVisibleContentRects for the current transaction if possible, rather than always delaying
+        https://bugs.webkit.org/show_bug.cgi?id=171619
+        
+        Also fixes webkit.org/b/170153 and webkit.org/b/170195
+
+        Reviewed by Tim Horton.
+
+        In r214391 we started adding the pre-commit handler in a dispatch_async() to ensure that
+        the handler would always run, since we couldn't reliably test the phase of the current
+        transaction. Now that problem has been solved (rdar://problem/31253952) we can go back to
+        checking the transaction phase on newer iOS versions. If we're too late for the current transaction
+        we still need to dispatch_async() to get into the next one.
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _addUpdateVisibleContentRectPreCommitHandler]):
+        (-[WKWebView _scheduleVisibleContentRectUpdateAfterScrollInView:]):
+
 2017-05-03  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm

@@ -2312 +2312 @@
 }
 
+- (void)_addUpdateVisibleContentRectPreCommitHandler
+{
+    auto retainedSelf = retainPtr(self);
+    [CATransaction addCommitHandler:[retainedSelf] {
+        WKWebView *webView = retainedSelf.get();
+        [webView _updateVisibleContentRects];
+        webView->_hasScheduledVisibleRectUpdate = NO;
+    } forPhase:kCATransactionPhasePreCommit];
+}
+
 - (void)_scheduleVisibleContentRectUpdateAfterScrollInView:(UIScrollView *)scrollView
 {
@@ -2321 +2331 @@
     _hasScheduledVisibleRectUpdate = YES;
     
-    // FIXME: remove the dispatch_async() when we have a fix for rdar://problem/31253952.
+#if __IPHONE_OS_VERSION_MIN_REQUIRED >= 110000
+    CATransactionPhase transactionPhase = [CATransaction currentPhase];
+    if (transactionPhase == kCATransactionPhaseNull || transactionPhase == kCATransactionPhasePreLayout) {
+        [self _addUpdateVisibleContentRectPreCommitHandler];
+        return;
+    }
+#endif
+
     dispatch_async(dispatch_get_main_queue(), [retainedSelf = retainPtr(self)] {
-        [CATransaction addCommitHandler:[retainedSelf] {
-            WKWebView *webView = retainedSelf.get();
-            [webView _updateVisibleContentRects];
-            webView->_hasScheduledVisibleRectUpdate = NO;
-        } forPhase:kCATransactionPhasePreCommit];
+        WKWebView *webView = retainedSelf.get();
+        [webView _addUpdateVisibleContentRectPreCommitHandler];
     });
 }

trunk/Tools/ChangeLog

@@ +1 @@
+2017-05-03  Simon Fraser  <simon.fraser@apple.com>
+
+        Have WKWebView call _updateVisibleContentRects for the current transaction if possible, rather than always delaying
+        https://bugs.webkit.org/show_bug.cgi?id=171619
+        https://bugs.webkit.org/show_bug.cgi?id=170195
+
+        Reviewed by Tim Horton.
+
+        Re-enable WebKit2.ResizeWithHiddenContentDoesNotHang.
+
+        * TestWebKitAPI/Tests/WebKit2Cocoa/AnimatedResize.mm:
+        (TEST):
+
 2017-05-03  Michael Catanzaro  <mcatanzaro@igalia.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/AnimatedResize.mm

@@ -83 +83 @@
 }
 
-TEST(WebKit2, DISABLED_ResizeWithHiddenContentDoesNotHang)
+TEST(WebKit2, ResizeWithHiddenContentDoesNotHang)
 {
     auto webView = createAnimatedResizeWebView();