iOS 15: History back sometimes scrolls to top of the page

https://bugs.webkit.org/show_bug.cgi?id=231563
<rdar://problem/84405417>

Reviewed by Devin Rousso.

Sometimes, going back in history would fail to restore the scroll position because
in -[WKWebView _restoreScrollAndZoomStateForTransaction:] the comparison of contentZoomScale() and _scaleToRestore
would fail. In the case I debugged, _scaleToRestore was 1.15 and contentZoomScale() was 1.15044, the latter deviating
via the round-trip through UIScrollView's zoomScale.

So make all the scale comparisons in both WKWebView and WebPageIOS a little more lenient, allowing a delta of 0.01.

* Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm:
(-[WKWebView _updateScrollViewForTransaction:]):
(-[WKWebView _restoreScrollAndZoomStateForTransaction:]):
(-[WKWebView _allowsDoubleTapGestures]):
* Source/WebKit/WebProcess/WebPage/WebPage.h:
* Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::scalesAreEssentiallyEqual):
(WebKit::WebPage::restorePageState):
(WebKit::WebPage::dynamicViewportSizeUpdate):
(WebKit::WebPage::scaleFromUIProcess const):
(WebKit::WebPage::updateVisibleContentRects):
(WebKit::areEssentiallyEqualAsFloat): Deleted.
* Tools/TestWebKitAPI/Tests/WebKit/simple-tall.html:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/RestoreScrollPosition.mm:
(TestWebKitAPI::TEST):

Canonical link: https://commits.webkit.org/253221@main

Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm

@@ -54,6 +54,7 @@
 #import "WKWebViewPrivateForTestingIOS.h"
 #import "WebBackForwardList.h"
 #import "WebIOSEventFactory.h"
+#import "WebPage.h"
 #import "WebPageProxy.h"
 #import "_WKActivatedElementInfoInternal.h"
 #import <WebCore/ColorCocoa.h>
@@ -951,7 +952,7 @@ - (void)_updateScrollViewForTransaction:(const WebKit::RemoteLayerTreeTransactio
     bool scrollingEnabled = _page->scrollingCoordinatorProxy()->hasScrollableOrZoomedMainFrame() || hasDockedInputView || isZoomed || scrollingNeededToRevealUI;
     [_scrollView _setScrollEnabledInternal:scrollingEnabled];
 
-    if (!layerTreeTransaction.scaleWasSetByUIProcess() && ![_scrollView isZooming] && ![_scrollView isZoomBouncing] && ![_scrollView _isAnimatingZoom] && !WTF::areEssentiallyEqual<float>([_scrollView zoomScale], layerTreeTransaction.pageScaleFactor())) {
+    if (!layerTreeTransaction.scaleWasSetByUIProcess() && ![_scrollView isZooming] && ![_scrollView isZoomBouncing] && ![_scrollView _isAnimatingZoom] && !WebKit::scalesAreEssentiallyEqual([_scrollView zoomScale], layerTreeTransaction.pageScaleFactor())) {
         LOG_WITH_STREAM(VisibleRects, stream << " updating scroll view with pageScaleFactor " << layerTreeTransaction.pageScaleFactor());
         [_scrollView setZoomScale:layerTreeTransaction.pageScaleFactor()];
     }
@@ -970,7 +971,7 @@ - (BOOL)_restoreScrollAndZoomStateForTransaction:(const WebKit::RemoteLayerTreeT
         WebCore::FloatPoint scaledScrollOffset = _scrollOffsetToRestore.value();
         _scrollOffsetToRestore = std::nullopt;
 
-        if (WTF::areEssentiallyEqual<float>(contentZoomScale(self), _scaleToRestore)) {
+        if (WebKit::scalesAreEssentiallyEqual(contentZoomScale(self), _scaleToRestore)) {
             scaledScrollOffset.scale(_scaleToRestore);
             WebCore::FloatPoint contentOffsetInScrollViewCoordinates = scaledScrollOffset - WebCore::FloatSize(_obscuredInsetsWhenSaved.left(), _obscuredInsetsWhenSaved.top());
 
@@ -985,7 +986,7 @@ - (BOOL)_restoreScrollAndZoomStateForTransaction:(const WebKit::RemoteLayerTreeT
         WebCore::FloatPoint unobscuredCenterToRestore = _unobscuredCenterToRestore.value();
         _unobscuredCenterToRestore = std::nullopt;
 
-        if (WTF::areEssentiallyEqual<float>(contentZoomScale(self), _scaleToRestore)) {
+        if (WebKit::scalesAreEssentiallyEqual(contentZoomScale(self), _scaleToRestore)) {
             CGRect unobscuredRect = UIEdgeInsetsInsetRect(self.bounds, _obscuredInsets);
             WebCore::FloatSize unobscuredContentSizeAtNewScale = WebCore::FloatSize(unobscuredRect.size) / _scaleToRestore;
             WebCore::FloatPoint topLeftInDocumentCoordinates = unobscuredCenterToRestore - unobscuredContentSizeAtNewScale / 2;
@@ -1745,7 +1746,7 @@ - (BOOL)_allowsDoubleTapGestures
     // At this point, we have a page that asked for width = device-width. However,
     // if the content's width and height were large, we might have had to shrink it.
     // We'll enable double tap zoom whenever we're not at the actual initial scale.
-    return !WTF::areEssentiallyEqual<float>(contentZoomScale(self), _initialScaleFactor);
+    return !WebKit::scalesAreEssentiallyEqual(contentZoomScale(self), _initialScaleFactor);
 }
 
 #pragma mark UIScrollViewDelegate

Source/WebKit/WebProcess/WebPage/WebPage.h

@@ -2519,4 +2519,8 @@ inline void WebPage::prepareToRunModalJavaScriptDialog() { }
 inline bool WebPage::shouldAvoidComputingPostLayoutDataForEditorState() const { return false; }
 #endif
 
+#if PLATFORM(IOS_FAMILY)
+bool scalesAreEssentiallyEqual(float, float);
+#endif
+
 } // namespace WebKit

Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

@@ -194,6 +194,15 @@ static String plainTextForDisplay(const std::optional<SimpleRange>& range)
     return range ? plainTextForDisplay(*range) : emptyString();
 }
 
+// WebCore stores the page scale factor as float instead of double. When we get a scale from WebCore,
+// we need to ignore differences that are within a small rounding error, with enough leeway
+// to handle rounding differences that may result from round-tripping through UIScrollView.
+bool scalesAreEssentiallyEqual(float a, float b)
+{
+    const auto scaleFactorEpsilon = 0.01f;
+    return WTF::areEssentiallyEqual(a, b, scaleFactorEpsilon);
+}
+
 void WebPage::platformDetach()
 {
     [m_mockAccessibilityElement setWebPage:nullptr];
@@ -488,6 +497,7 @@ static inline FloatRect adjustExposedRectForNewScale(const FloatRect& exposedRec
             m_hasRestoredExposedContentRectAfterDidCommitLoad = true;
             scrollPosition = FloatPoint(historyItem.scrollPosition());
         }
+
         send(Messages::WebPageProxy::RestorePageState(scrollPosition, frameView.scrollOrigin(), historyItem.obscuredInsets(), boundedScale));
     } else {
         IntSize oldContentSize = historyItem.contentSize();
@@ -3602,13 +3612,6 @@ static void populateCaretContext(const HitTestResult& hitTestResult, const Inter
     }
 }
 
-// WebCore stores the page scale factor as float instead of double. When we get a scale from WebCore,
-// we need to ignore differences that are within a small rounding error on floats.
-static inline bool areEssentiallyEqualAsFloat(float a, float b)
-{
-    return WTF::areEssentiallyEqual(a, b);
-}
-
 void WebPage::setViewportConfigurationViewLayoutSize(const FloatSize& size, double scaleFactor, double minimumEffectiveDeviceWidth)
 {
     LOG_WITH_STREAM(VisibleRects, stream << "WebPage " << m_identifier << " setViewportConfigurationViewLayoutSize " << size << " scaleFactor " << scaleFactor << " minimumEffectiveDeviceWidth " << minimumEffectiveDeviceWidth);
@@ -3655,7 +3658,7 @@ static inline bool areEssentiallyEqualAsFloat(float a, float b)
     IntSize oldContentSize = frameView.contentsSize();
     float oldPageScaleFactor = m_page->pageScaleFactor();
     auto oldUnobscuredContentRect = frameView.unobscuredContentRect();
-    bool wasAtInitialScale = areEssentiallyEqualAsFloat(oldPageScaleFactor, m_viewportConfiguration.initialScale());
+    bool wasAtInitialScale = scalesAreEssentiallyEqual(oldPageScaleFactor, m_viewportConfiguration.initialScale());
 
     m_dynamicSizeUpdateHistory.add(std::make_pair(oldContentSize, oldPageScaleFactor), frameView.scrollPosition());
 
@@ -3708,7 +3711,7 @@ static inline bool areEssentiallyEqualAsFloat(float a, float b)
     FloatRect newUnobscuredContentRect = targetUnobscuredRect;
     FloatRect newExposedContentRect = targetExposedContentRect;
 
-    bool scaleChanged = !areEssentiallyEqualAsFloat(scale, targetScale);
+    bool scaleChanged = !scalesAreEssentiallyEqual(scale, targetScale);
     if (scaleChanged) {
         // The target scale the UI is using cannot be reached by the content. We need to compute new targets based
         // on the viewport constraint and report everything back to the UIProcess.
@@ -3780,7 +3783,7 @@ static inline bool areEssentiallyEqualAsFloat(float a, float b)
                 newExposedContentRect.setY(0);
             }
 
-            bool likelyResponsiveDesignViewport = newLayoutSize.width() == viewLayoutSize.width() && areEssentiallyEqualAsFloat(scale, 1);
+            bool likelyResponsiveDesignViewport = newLayoutSize.width() == viewLayoutSize.width() && scalesAreEssentiallyEqual(scale, 1);
             bool contentBleedsOutsideLayoutWidth = newContentSize.width() > newLayoutSize.width();
             bool originalScrollPositionWasOnTheLeftEdge = targetUnobscuredRect.x() <= 0;
             if (likelyResponsiveDesignViewport && contentBleedsOutsideLayoutWidth && originalScrollPositionWasOnTheLeftEdge) {
@@ -4160,7 +4163,7 @@ static inline void adjustVelocityDataForBoundedScale(VelocityData& velocityData,
     }
 
     scaleFromUIProcess = std::min<float>(m_viewportConfiguration.maximumScale(), std::max<float>(m_viewportConfiguration.minimumScale(), scaleFromUIProcess));
-    if (areEssentiallyEqualAsFloat(currentScale, scaleFromUIProcess))
+    if (scalesAreEssentiallyEqual(currentScale, scaleFromUIProcess))
         return std::nullopt;
 
     return scaleFromUIProcess;
@@ -4234,11 +4237,11 @@ static bool selectionIsInsideFixedPositionContainer(Frame& frame)
         if (!m_lastLayerTreeTransactionIdAndPageScaleBeforeScalingPage)
             return false;
 
-        if (areEssentiallyEqualAsFloat(scaleToUse, m_page->pageScaleFactor()))
+        if (scalesAreEssentiallyEqual(scaleToUse, m_page->pageScaleFactor()))
             return false;
 
         auto [transactionIdBeforeScalingPage, scaleBeforeScalingPage] = *m_lastLayerTreeTransactionIdAndPageScaleBeforeScalingPage;
-        if (!areEssentiallyEqualAsFloat(scaleBeforeScalingPage, scaleToUse))
+        if (!scalesAreEssentiallyEqual(scaleBeforeScalingPage, scaleToUse))
             return false;
 
         return transactionIdBeforeScalingPage >= visibleContentRectUpdateInfo.lastLayerTreeTransactionID();

Tools/TestWebKitAPI/Tests/WebKit/simple-tall.html

@@ -1,6 +1,8 @@
-<meta name='viewport' content='width=device-width, initial-scale=1'>
 <!DOCTYPE html>
 <html>
+<head>
+<meta name='viewport' content='width=device-width, initial-scale=1'>
+</head>
 <body>
   Simple and tall HTML file.
   <div style="height: 3000px;"></div>

Tools/TestWebKitAPI/Tests/WebKitCocoa/RestoreScrollPosition.mm

@@ -85,6 +85,33 @@
     EXPECT_EQ(1000, contentOffsetAfterBack.y);
 }
 
+TEST(RestoreScrollPositionTests, RestoreScrollPositionAfterBack)
+{
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 390, 664)]);
+
+    [webView synchronouslyLoadTestPageNamed:@"simple-tall"];
+    [webView _setViewScale:1.15]; // Simulate MobileSafari setting view scale on every didCommitNavigation.
+    [webView waitForNextPresentationUpdate];
+
+    [[webView scrollView] setContentOffset:CGPointMake(0, 1000)];
+    [webView waitForNextPresentationUpdate];
+
+    [webView synchronouslyLoadTestPageNamed:@"simple"];
+    [webView _setViewScale:1.15];
+    [webView waitForNextPresentationUpdate];
+
+    CGPoint contentOffsetInNewPage = [webView scrollView].contentOffset;
+    EXPECT_EQ(0, contentOffsetInNewPage.x);
+    EXPECT_EQ(0, contentOffsetInNewPage.y);
+
+    [webView synchronouslyGoBack];
+    [webView _setViewScale:1.15];
+    [webView waitForNextPresentationUpdate];
+
+    CGPoint contentOffsetAfterBack = [webView scrollView].contentOffset;
+    EXPECT_EQ(0, contentOffsetAfterBack.x);
+    EXPECT_TRUE(WTF::areEssentiallyEqual<float>(contentOffsetAfterBack.y, 1000.0f, 1.0f)); // It can be 999.5 but that's OK.
+}
 #endif
 
 }