Fix cause of viewport-related flakiness in iOS tests
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 Dec 2016 01:06:21 +0000 (01:06 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 Dec 2016 01:06:21 +0000 (01:06 +0000)
https://bugs.webkit.org/show_bug.cgi?id=165878

Reviewed by Tim Horton.

Source/WebKit2:

When TestController::platformConfigureViewForTest() changes the view size for a flexible
viewport test, the page would not have updated with the new scale by the time the load event
fired, causing flakiness depending on test order.

This was caused by code added in r170325 that delayed visible content rect updates until
after the UI process has received the transaction after didCommitLoad. So fix by overriding
this mechanism if the view has been resized.

* Shared/VisibleContentRectUpdateInfo.cpp:
(WebKit::VisibleContentRectUpdateInfo::encode):
(WebKit::VisibleContentRectUpdateInfo::decode):
(WebKit::operator<<):
* Shared/VisibleContentRectUpdateInfo.h:
(WebKit::VisibleContentRectUpdateInfo::VisibleContentRectUpdateInfo):
(WebKit::VisibleContentRectUpdateInfo::isFirstUpdateForNewViewSize):
(WebKit::operator==):
* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _frameOrBoundsChanged]):
(-[WKWebView _updateContentRectsWithState:]):
* UIProcess/DrawingAreaProxy.cpp:
(WebKit::DrawingAreaProxy::setSize):
* UIProcess/DrawingAreaProxy.h:
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::WebPageProxy):
* UIProcess/WebPageProxy.h:
* UIProcess/ios/WKContentView.h:
* UIProcess/ios/WKContentView.mm:
(-[WKContentView didUpdateVisibleRect:unobscuredRect:unobscuredRectInScrollViewCoordinates:obscuredInset:scale:minimumScale:inStableState:isChangingObscuredInsetsInteractively:enclosedInScrollableAncestorView:]):
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::updateVisibleContentRects):

LayoutTests:

Try un-flaking some viewport tests.

* platform/ios-simulator-wk2/TestExpectations:

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

13 files changed:
LayoutTests/ChangeLog
LayoutTests/platform/ios-simulator-wk2/TestExpectations
Source/WebKit2/ChangeLog
Source/WebKit2/Shared/VisibleContentRectUpdateInfo.cpp
Source/WebKit2/Shared/VisibleContentRectUpdateInfo.h
Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm
Source/WebKit2/UIProcess/DrawingAreaProxy.cpp
Source/WebKit2/UIProcess/DrawingAreaProxy.h
Source/WebKit2/UIProcess/WebPageProxy.cpp
Source/WebKit2/UIProcess/WebPageProxy.h
Source/WebKit2/UIProcess/ios/WKContentView.h
Source/WebKit2/UIProcess/ios/WKContentView.mm
Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm

index 7fcbfab..e955b8f 100644 (file)
@@ -1,3 +1,14 @@
+2016-12-14  Simon Fraser  <simon.fraser@apple.com>
+
+        Fix cause of viewport-related flakiness in iOS tests
+        https://bugs.webkit.org/show_bug.cgi?id=165878
+
+        Reviewed by Tim Horton.
+
+        Try un-flaking some viewport tests.
+
+        * platform/ios-simulator-wk2/TestExpectations:
+
 2016-12-12  Jon Lee  <jonlee@apple.com>
 
         Full Pass CSS Variables Test Suite
index 865cd19..9abb025 100644 (file)
@@ -1809,10 +1809,6 @@ webkit.org/b/155201 storage/domstorage/events/basic-body-attribute.html [ Pass F
 # Forcing always allow user scalable is not supported on certain OS version.
 webkit.org/b/155056 fast/viewport/ios/force-always-user-scalable.html [ Skip ]
 
-webkit.org/b/153110 fast/viewport/ios/width-is-device-width-overflowing.html [ Pass Failure ]
-webkit.org/b/153110 fast/viewport/ios/width-is-device-width-overflowing-body-overflow-hidden.html [ Pass Failure ]
-webkit.org/b/153110 fast/viewport/ios/width-is-device-width-overflowing-body-overflow-hidden-tall.html [ Pass Failure ]
-
 webkit.org/b/155501 animations/3d/transform-origin-vs-functions.html [ Pass Failure ]
 
 webkit.org/b/155495 compositing/visible-rect/animated-from-none.html [ Pass Failure ]
index 0406470..0c5e1d8 100644 (file)
@@ -1,3 +1,41 @@
+2016-12-14  Simon Fraser  <simon.fraser@apple.com>
+
+        Fix cause of viewport-related flakiness in iOS tests
+        https://bugs.webkit.org/show_bug.cgi?id=165878
+
+        Reviewed by Tim Horton.
+
+        When TestController::platformConfigureViewForTest() changes the view size for a flexible
+        viewport test, the page would not have updated with the new scale by the time the load event
+        fired, causing flakiness depending on test order.
+
+        This was caused by code added in r170325 that delayed visible content rect updates until
+        after the UI process has received the transaction after didCommitLoad. So fix by overriding
+        this mechanism if the view has been resized.
+
+        * Shared/VisibleContentRectUpdateInfo.cpp:
+        (WebKit::VisibleContentRectUpdateInfo::encode):
+        (WebKit::VisibleContentRectUpdateInfo::decode):
+        (WebKit::operator<<):
+        * Shared/VisibleContentRectUpdateInfo.h:
+        (WebKit::VisibleContentRectUpdateInfo::VisibleContentRectUpdateInfo):
+        (WebKit::VisibleContentRectUpdateInfo::isFirstUpdateForNewViewSize):
+        (WebKit::operator==):
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _frameOrBoundsChanged]):
+        (-[WKWebView _updateContentRectsWithState:]):
+        * UIProcess/DrawingAreaProxy.cpp:
+        (WebKit::DrawingAreaProxy::setSize):
+        * UIProcess/DrawingAreaProxy.h:
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::WebPageProxy):
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/ios/WKContentView.h:
+        * UIProcess/ios/WKContentView.mm:
+        (-[WKContentView didUpdateVisibleRect:unobscuredRect:unobscuredRectInScrollViewCoordinates:obscuredInset:scale:minimumScale:inStableState:isChangingObscuredInsetsInteractively:enclosedInScrollableAncestorView:]):
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::updateVisibleContentRects):
+
 2016-12-04  Sam Weinig  <sam@webkit.org>
 
         [WebIDL] Add support for converting dictionaries to JS
index e0f5f08..3c57105 100644 (file)
@@ -47,6 +47,7 @@ void VisibleContentRectUpdateInfo::encode(IPC::Encoder& encoder) const
     encoder << m_verticalVelocity;
     encoder << m_scaleChangeRate;
     encoder << m_inStableState;
+    encoder << m_isFirstUpdateForNewViewSize;
     encoder << m_isChangingObscuredInsetsInteractively;
     encoder << m_allowShrinkToFit;
     encoder << m_enclosedInScrollableAncestorView;
@@ -78,6 +79,8 @@ bool VisibleContentRectUpdateInfo::decode(IPC::Decoder& decoder, VisibleContentR
         return false;
     if (!decoder.decode(result.m_inStableState))
         return false;
+    if (!decoder.decode(result.m_isFirstUpdateForNewViewSize))
+        return false;
     if (!decoder.decode(result.m_isChangingObscuredInsetsInteractively))
         return false;
     if (!decoder.decode(result.m_allowShrinkToFit))
@@ -111,6 +114,7 @@ TextStream& operator<<(TextStream& ts, const VisibleContentRectUpdateInfo& info)
 
     ts.dumpProperty("scale", info.scale());
     ts.dumpProperty("inStableState", info.inStableState());
+    ts.dumpProperty("isFirstUpdateForNewViewSize", info.isFirstUpdateForNewViewSize());
     if (info.isChangingObscuredInsetsInteractively())
         ts.dumpProperty("isChangingObscuredInsetsInteractively", info.isChangingObscuredInsetsInteractively());
     if (info.enclosedInScrollableAncestorView())
index dd3a87a..05d3ead 100644 (file)
@@ -23,8 +23,7 @@
  * THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#ifndef VisibleContentRectUpdateInfo_h
-#define VisibleContentRectUpdateInfo_h
+#pragma once
 
 #include <WebCore/FloatRect.h>
 #include <wtf/MonotonicTime.h>
@@ -47,7 +46,7 @@ public:
 
     VisibleContentRectUpdateInfo(const WebCore::FloatRect& exposedContentRect, const WebCore::FloatRect& unobscuredContentRect,
         const WebCore::FloatRect& unobscuredRectInScrollViewCoordinates, const WebCore::FloatRect& customFixedPositionRect,
-        const WebCore::FloatSize& obscuredInset, double scale, bool inStableState, bool isChangingObscuredInsetsInteractively, bool allowShrinkToFit, bool enclosedInScrollableAncestorView,
+        const WebCore::FloatSize& obscuredInset, double scale, bool inStableState, bool isFirstUpdateForNewViewSize, bool isChangingObscuredInsetsInteractively, bool allowShrinkToFit, bool enclosedInScrollableAncestorView,
         MonotonicTime timestamp, double horizontalVelocity, double verticalVelocity, double scaleChangeRate, uint64_t lastLayerTreeTransactionId)
         : m_exposedContentRect(exposedContentRect)
         , m_unobscuredContentRect(unobscuredContentRect)
@@ -61,6 +60,7 @@ public:
         , m_verticalVelocity(verticalVelocity)
         , m_scaleChangeRate(scaleChangeRate)
         , m_inStableState(inStableState)
+        , m_isFirstUpdateForNewViewSize(isFirstUpdateForNewViewSize)
         , m_isChangingObscuredInsetsInteractively(isChangingObscuredInsetsInteractively)
         , m_allowShrinkToFit(allowShrinkToFit)
         , m_enclosedInScrollableAncestorView(enclosedInScrollableAncestorView)
@@ -75,6 +75,7 @@ public:
 
     double scale() const { return m_scale; }
     bool inStableState() const { return m_inStableState; }
+    bool isFirstUpdateForNewViewSize() const { return m_isFirstUpdateForNewViewSize; }
     bool isChangingObscuredInsetsInteractively() const { return m_isChangingObscuredInsetsInteractively; }
     bool allowShrinkToFit() const { return m_allowShrinkToFit; }
     bool enclosedInScrollableAncestorView() const { return m_enclosedInScrollableAncestorView; }
@@ -104,6 +105,7 @@ private:
     double m_verticalVelocity { 0 };
     double m_scaleChangeRate { 0 };
     bool m_inStableState { false };
+    bool m_isFirstUpdateForNewViewSize { false };
     bool m_isChangingObscuredInsetsInteractively { false };
     bool m_allowShrinkToFit { false };
     bool m_enclosedInScrollableAncestorView { false };
@@ -121,6 +123,7 @@ inline bool operator==(const VisibleContentRectUpdateInfo& a, const VisibleConte
         && a.verticalVelocity() == b.verticalVelocity()
         && a.scaleChangeRate() == b.scaleChangeRate()
         && a.inStableState() == b.inStableState()
+        && a.isFirstUpdateForNewViewSize() == b.isFirstUpdateForNewViewSize()
         && a.allowShrinkToFit() == b.allowShrinkToFit()
         && a.enclosedInScrollableAncestorView() == b.enclosedInScrollableAncestorView();
 }
@@ -128,5 +131,3 @@ inline bool operator==(const VisibleContentRectUpdateInfo& a, const VisibleConte
 WebCore::TextStream& operator<<(WebCore::TextStream&, const VisibleContentRectUpdateInfo&);
 
 } // namespace WebKit
-
-#endif // VisibleContentRectUpdateInfo_h
index 121552d..4a91cf6 100644 (file)
@@ -2025,8 +2025,13 @@ static WebCore::FloatPoint constrainContentOffset(WebCore::FloatPoint contentOff
             _page->setViewportConfigurationMinimumLayoutSize(WebCore::FloatSize(bounds.size));
         if (!_overridesMaximumUnobscuredSize)
             _page->setMaximumUnobscuredSize(WebCore::FloatSize(bounds.size));
+        
+        BOOL sizeChanged = NO;
         if (auto drawingArea = _page->drawingArea())
-            drawingArea->setSize(WebCore::IntSize(bounds.size), WebCore::IntSize(), WebCore::IntSize());
+            sizeChanged = drawingArea->setSize(WebCore::IntSize(bounds.size), WebCore::IntSize(), WebCore::IntSize());
+        
+        if (sizeChanged & [self usesStandardContentView])
+            [_contentView setSizeChangedSinceLastVisibleContentRectUpdate:YES];
     }
 
     [_customContentView web_setMinimumSize:bounds.size];
@@ -2105,7 +2110,7 @@ static bool scrollViewCanScroll(UIScrollView *scrollView)
     }
 
     if (_dynamicViewportUpdateMode != DynamicViewportUpdateMode::NotResizing
-        || _needsResetViewStateAfterCommitLoadForMainFrame
+        || (_needsResetViewStateAfterCommitLoadForMainFrame && ![_contentView sizeChangedSinceLastVisibleContentRectUpdate])
         || [_scrollView isZoomBouncing]
         || _currentlyAdjustingScrollViewInsetsForKeyboard)
         return;
index b45f227..a5f75f0 100644 (file)
@@ -55,15 +55,16 @@ DrawingAreaProxy::~DrawingAreaProxy()
     m_webPageProxy.process().removeMessageReceiver(Messages::DrawingAreaProxy::messageReceiverName(), m_webPageProxy.pageID());
 }
 
-void DrawingAreaProxy::setSize(const IntSize& size, const IntSize& layerPosition, const IntSize& scrollOffset)
+bool DrawingAreaProxy::setSize(const IntSize& size, const IntSize& layerPosition, const IntSize& scrollOffset)
 { 
     if (m_size == size && m_layerPosition == layerPosition && scrollOffset.isZero())
-        return;
+        return false;
 
     m_size = size;
     m_layerPosition = layerPosition;
     m_scrollOffset += scrollOffset;
     sizeDidChange();
+    return true;
 }
 
 #if PLATFORM(COCOA)
index 117f71e..3df188f 100644 (file)
@@ -68,7 +68,7 @@ public:
     virtual void waitForBackingStoreUpdateOnNextPaint() { }
 
     const WebCore::IntSize& size() const { return m_size; }
-    void setSize(const WebCore::IntSize&, const WebCore::IntSize&, const WebCore::IntSize& scrollOffset);
+    bool setSize(const WebCore::IntSize&, const WebCore::IntSize&, const WebCore::IntSize& scrollOffset);
 
     // The timeout we use when waiting for a DidUpdateGeometry message.
     static constexpr Seconds didUpdateBackingStoreStateTimeout() { return Seconds::fromMilliseconds(500); }
index e5d3960..c93fd74 100644 (file)
@@ -346,16 +346,6 @@ WebPageProxy::WebPageProxy(PageClient& pageClient, WebProcessProxy& process, uin
 #if ENABLE(FULLSCREEN_API)
     , m_fullscreenClient(std::make_unique<API::FullscreenClient>())
 #endif
-#if PLATFORM(IOS)
-    , m_hasReceivedLayerTreeTransactionAfterDidCommitLoad(true)
-    , m_firstLayerTreeTransactionIdAfterDidCommitLoad(0)
-    , m_deviceOrientation(0)
-    , m_dynamicViewportSizeUpdateWaitingForTarget(false)
-    , m_dynamicViewportSizeUpdateWaitingForLayerTreeCommit(false)
-    , m_dynamicViewportSizeUpdateLayerTreeTransactionID(0)
-    , m_layerTreeTransactionIdAtLastTouchStart(0)
-    , m_hasNetworkRequestsOnSuspended(false)
-#endif
     , m_geolocationPermissionRequestManager(*this)
     , m_notificationPermissionRequestManager(*this)
     , m_activityState(ActivityState::NoFlags)
index 9f7074d..ff02f4a 100644 (file)
@@ -1655,15 +1655,15 @@ private:
 #endif
 #if PLATFORM(IOS)
     VisibleContentRectUpdateInfo m_lastVisibleContentRectUpdate;
-    bool m_hasReceivedLayerTreeTransactionAfterDidCommitLoad;
-    uint64_t m_firstLayerTreeTransactionIdAfterDidCommitLoad;
-    int32_t m_deviceOrientation;
-    bool m_dynamicViewportSizeUpdateWaitingForTarget;
-    bool m_dynamicViewportSizeUpdateWaitingForLayerTreeCommit;
-    uint64_t m_dynamicViewportSizeUpdateLayerTreeTransactionID;
-    uint64_t m_layerTreeTransactionIdAtLastTouchStart;
+    bool m_hasReceivedLayerTreeTransactionAfterDidCommitLoad { true };
+    uint64_t m_firstLayerTreeTransactionIdAfterDidCommitLoad { 0 };
+    int32_t m_deviceOrientation { 0 };
+    bool m_dynamicViewportSizeUpdateWaitingForTarget { false };
+    bool m_dynamicViewportSizeUpdateWaitingForLayerTreeCommit { false };
+    uint64_t m_dynamicViewportSizeUpdateLayerTreeTransactionID { 0 };
+    uint64_t m_layerTreeTransactionIdAtLastTouchStart { 0 };
     uint64_t m_currentDynamicViewportSizeUpdateID { 0 };
-    bool m_hasNetworkRequestsOnSuspended;
+    bool m_hasNetworkRequestsOnSuspended { false };
     bool m_isKeyboardAnimatingIn { false };
     bool m_isScrollingOrZooming { false };
 #endif
index 9066520..381d106 100644 (file)
@@ -63,6 +63,7 @@ class WebProcessPool;
 @property (nonatomic, getter=isShowingInspectorIndication) BOOL showingInspectorIndication;
 @property (nonatomic, readonly) BOOL isBackground;
 @property (nonatomic, readonly, getter=isResigningFirstResponder) BOOL resigningFirstResponder;
+@property (nonatomic) BOOL sizeChangedSinceLastVisibleContentRectUpdate;
 
 - (instancetype)initWithFrame:(CGRect)frame processPool:(WebKit::WebProcessPool&)processPool configuration:(Ref<API::PageConfiguration>&&)configuration webView:(WKWebView *)webView;
 
index 1238e62..9d432d3 100644 (file)
@@ -393,6 +393,7 @@ private:
         WebCore::FloatSize(obscuredInset),
         zoomScale,
         isStableState,
+        _sizeChangedSinceLastVisibleContentRectUpdate,
         isChangingObscuredInsetsInteractively,
         _webView._allowsViewportShrinkToFit,
         enclosedInScrollableAncestorView,
@@ -406,6 +407,8 @@ private:
 
     _page->updateVisibleContentRects(visibleContentRectUpdateInfo);
 
+    _sizeChangedSinceLastVisibleContentRectUpdate = NO;
+
     FloatRect fixedPositionRect = _page->computeCustomFixedPositionRect(_page->unobscuredContentRect(), _page->customFixedPositionRect(), zoomScale, WebPageProxy::UnobscuredRectConstraint::Unconstrained, scrollingCoordinator->visualViewportEnabled());
     scrollingCoordinator->viewportChangedViaDelegatedScrolling(scrollingCoordinator->rootScrollingNodeID(), fixedPositionRect, zoomScale);
 
index 9a041af..17fde78 100644 (file)
@@ -3007,12 +3007,12 @@ static inline FloatRect adjustExposedRectForBoundedScale(const FloatRect& expose
 
 void WebPage::updateVisibleContentRects(const VisibleContentRectUpdateInfo& visibleContentRectUpdateInfo, MonotonicTime oldestTimestamp)
 {
+    LOG_WITH_STREAM(VisibleRects, stream << "\nWebPage::updateVisibleContentRects " << visibleContentRectUpdateInfo);
+
     // Skip any VisibleContentRectUpdate that have been queued before DidCommitLoad suppresses the updates in the UIProcess.
-    if (visibleContentRectUpdateInfo.lastLayerTreeTransactionID() < m_mainFrame->firstLayerTreeTransactionIDAfterDidCommitLoad())
+    if (visibleContentRectUpdateInfo.lastLayerTreeTransactionID() < m_mainFrame->firstLayerTreeTransactionIDAfterDidCommitLoad() && !visibleContentRectUpdateInfo.isFirstUpdateForNewViewSize())
         return;
 
-    LOG_WITH_STREAM(VisibleRects, stream << "\nWebPage::updateVisibleContentRects " << visibleContentRectUpdateInfo);
-
     m_hasReceivedVisibleContentRectsAfterDidCommitLoad = true;
     m_isInStableState = visibleContentRectUpdateInfo.inStableState();