[Chromium] Correct zoom for focused node when using compositor scaling

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

Reviewed by Adam Barth.

When applyDeviceScaleFactorInCompositor, targetScale should exclude device scale factor.
When applyPageScaleFactorInCompositor, caret size and content sizes are in css pixels and they should be in the viewport of the new scale.

* src/WebViewImpl.cpp:
(WebKit::WebViewImpl::scrollFocusedNodeIntoRect):
(WebKit):
(WebKit::WebViewImpl::computeScaleAndScrollForFocusedNode): Extracted from scrollFocusedNodeIntoRect() to ease testing.
* src/WebViewImpl.h:
(WebViewImpl):
* tests/WebFrameTest.cpp: Updated test DivScrollEditableTest
* tests/data/get_scale_for_zoom_into_editable_test.html: Moved the logic of onload script (which seems not to work) into WebFrameTest.cpp.


Canonical link: https://commits.webkit.org/126444@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@141153 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/chromium/ChangeLog

@@ -1,3 +1,22 @@
+2013-01-29  Xianzhu Wang  <wangxianzhu@chromium.org>
+
+        [Chromium] Correct zoom for focused node when using compositor scaling
+        https://bugs.webkit.org/show_bug.cgi?id=107599
+
+        Reviewed by Adam Barth.
+
+        When applyDeviceScaleFactorInCompositor, targetScale should exclude device scale factor.
+        When applyPageScaleFactorInCompositor, caret size and content sizes are in css pixels and they should be in the viewport of the new scale.
+
+        * src/WebViewImpl.cpp:
+        (WebKit::WebViewImpl::scrollFocusedNodeIntoRect):
+        (WebKit):
+        (WebKit::WebViewImpl::computeScaleAndScrollForFocusedNode): Extracted from scrollFocusedNodeIntoRect() to ease testing.
+        * src/WebViewImpl.h:
+        (WebViewImpl):
+        * tests/WebFrameTest.cpp: Updated test DivScrollEditableTest
+        * tests/data/get_scale_for_zoom_into_editable_test.html: Moved the logic of onload script (which seems not to work) into WebFrameTest.cpp.
+
 2013-01-29  Alec Flett  <alecflett@chromium.org>
 
         IndexedDB: Pass metadata in to IDBOpenDBRequest.onUpgradeNeeded/onSuccess

Source/WebKit/chromium/src/WebViewImpl.cpp

@@ -2788,6 +2788,18 @@ void WebViewImpl::scrollFocusedNodeIntoRect(const WebRect& rect)
     }
 
 #if ENABLE(GESTURE_EVENTS)
+    float scale;
+    IntPoint scroll;
+    bool needAnimation;
+    computeScaleAndScrollForFocusedNode(focusedNode, scale, scroll, needAnimation);
+    if (needAnimation)
+        startPageScaleAnimation(scroll, false, scale, scrollAndScaleAnimationDurationInSeconds);
+#endif
+}
+
+#if ENABLE(GESTURE_EVENTS)
+void WebViewImpl::computeScaleAndScrollForFocusedNode(Node* focusedNode, float& newScale, IntPoint& newScroll, bool& needAnimation)
+{
     focusedNode->document()->updateLayoutIgnorePendingStylesheets();
 
     // 'caret' is rect encompassing the blinking cursor.
@@ -2798,61 +2810,66 @@ void WebViewImpl::scrollFocusedNodeIntoRect(const WebRect& rect)
     // Pick a scale which is reasonably readable. This is the scale at which
     // the caret height will become minReadableCaretHeight (adjusted for dpi
     // and font scale factor).
-    float targetScale = deviceScaleFactor();
+    float targetScale = settingsImpl()->applyDeviceScaleFactorInCompositor() ? 1 : deviceScaleFactor();
 #if ENABLE(TEXT_AUTOSIZING)
     if (page() && page()->settings())
         targetScale *= page()->settings()->textAutosizingFontScaleFactor();
 #endif
-    const float newScale = clampPageScaleFactorToLimits(pageScaleFactor() * minReadableCaretHeight * targetScale / caret.height);
+    const float caretHeight = settingsImpl()->applyPageScaleFactorInCompositor() ? caret.height : caret.height / pageScaleFactor();
+    newScale = clampPageScaleFactorToLimits(minReadableCaretHeight * targetScale / caretHeight);
     const float deltaScale = newScale / pageScaleFactor();
 
     // Convert the rects to absolute space in the new scale.
     IntRect textboxRectInDocumentCoordinates = textboxRect;
     textboxRectInDocumentCoordinates.move(mainFrame()->scrollOffset());
-    textboxRectInDocumentCoordinates.scale(deltaScale);
     IntRect caretInDocumentCoordinates = caret;
     caretInDocumentCoordinates.move(mainFrame()->scrollOffset());
-    caretInDocumentCoordinates.scale(deltaScale);
 
-    IntPoint newOffset;
-    if (textboxRectInDocumentCoordinates.width() <= m_size.width) {
+    int viewWidth = m_size.width;
+    int viewHeight = m_size.height;
+    if (settingsImpl()->applyPageScaleFactorInCompositor()) {
+        viewWidth /= newScale;
+        viewHeight /= newScale;
+    } else {
+        textboxRectInDocumentCoordinates.scale(deltaScale);
+        caretInDocumentCoordinates.scale(deltaScale);
+    }
+
+    if (textboxRectInDocumentCoordinates.width() <= viewWidth) {
         // Field is narrower than screen. Try to leave padding on left so field's
         // label is visible, but it's more important to ensure entire field is
         // onscreen.
-        int idealLeftPadding = m_size.width * leftBoxRatio;
-        int maxLeftPaddingKeepingBoxOnscreen = m_size.width - textboxRectInDocumentCoordinates.width();
-        newOffset.setX(textboxRectInDocumentCoordinates.x() - min<int>(idealLeftPadding, maxLeftPaddingKeepingBoxOnscreen));
+        int idealLeftPadding = viewWidth * leftBoxRatio;
+        int maxLeftPaddingKeepingBoxOnscreen = viewWidth - textboxRectInDocumentCoordinates.width();
+        newScroll.setX(textboxRectInDocumentCoordinates.x() - min<int>(idealLeftPadding, maxLeftPaddingKeepingBoxOnscreen));
     } else {
         // Field is wider than screen. Try to left-align field, unless caret would
         // be offscreen, in which case right-align the caret.
-        newOffset.setX(max<int>(textboxRectInDocumentCoordinates.x(), caretInDocumentCoordinates.x() + caretInDocumentCoordinates.width() + caretPadding - m_size.width));
+        newScroll.setX(max<int>(textboxRectInDocumentCoordinates.x(), caretInDocumentCoordinates.x() + caretInDocumentCoordinates.width() + caretPadding - viewWidth));
     }
-    if (textboxRectInDocumentCoordinates.height() <= m_size.height) {
+    if (textboxRectInDocumentCoordinates.height() <= viewHeight) {
         // Field is shorter than screen. Vertically center it.
-        newOffset.setY(textboxRectInDocumentCoordinates.y() - (m_size.height - textboxRectInDocumentCoordinates.height()) / 2);
+        newScroll.setY(textboxRectInDocumentCoordinates.y() - (viewHeight - textboxRectInDocumentCoordinates.height()) / 2);
     } else {
         // Field is taller than screen. Try to top align field, unless caret would
         // be offscreen, in which case bottom-align the caret.
-        newOffset.setY(max<int>(textboxRectInDocumentCoordinates.y(), caretInDocumentCoordinates.y() + caretInDocumentCoordinates.height() + caretPadding - m_size.height));
+        newScroll.setY(max<int>(textboxRectInDocumentCoordinates.y(), caretInDocumentCoordinates.y() + caretInDocumentCoordinates.height() + caretPadding - viewHeight));
     }
 
-    bool needAnimation = false;
+    needAnimation = false;
     // If we are at less than the target zoom level, zoom in.
     if (deltaScale > minScaleChangeToTriggerZoom)
         needAnimation = true;
     // If the caret is offscreen, then animate.
-    IntRect sizeRect(0, 0, m_size.width, m_size.height);
+    IntRect sizeRect(0, 0, viewWidth, viewHeight);
     if (!sizeRect.contains(caret))
         needAnimation = true;
     // If the box is partially offscreen and it's possible to bring it fully
     // onscreen, then animate.
     if (sizeRect.contains(textboxRectInDocumentCoordinates.width(), textboxRectInDocumentCoordinates.height()) && !sizeRect.contains(textboxRect))
         needAnimation = true;
-
-    if (needAnimation)
-        startPageScaleAnimation(newOffset, false, newScale, scrollAndScaleAnimationDurationInSeconds);
-#endif
 }
+#endif
 
 void WebViewImpl::advanceFocus(bool reverse)
 {

Source/WebKit/chromium/src/WebViewImpl.h

@@ -579,6 +579,7 @@ class WebViewImpl : public WebView
     void computeScaleAndScrollForHitRect(const WebRect& hitRect, AutoZoomType, float& scale, WebPoint& scroll, bool& isAnchor);
     WebCore::Node* bestTouchLinkNode(const WebGestureEvent& touchEvent);
     void enableTouchHighlight(const WebGestureEvent& touchEvent);
+    void computeScaleAndScrollForFocusedNode(WebCore::Node* focusedNode, float& scale, WebCore::IntPoint& scroll, bool& needAnimation);
 #endif
     void animateZoomAroundPoint(const WebCore::IntPoint&, AutoZoomType);
 

Source/WebKit/chromium/tests/WebFrameTest.cpp

@@ -928,22 +928,21 @@ TEST_F(WebFrameTest, DivAutoZoomScaleFontScaleFactorTestCompositorScaling)
 }
 #endif
 
-// This test depends on code that is compiled conditionally. We likely need to
-// add the proper ifdef when re-enabling it. See
-// https://bugs.webkit.org/show_bug.cgi?id=98558
-TEST_F(WebFrameTest, DISABLED_DivScrollIntoEditableTest)
+TEST_F(WebFrameTest, DivScrollIntoEditableTest)
 {
     registerMockedHttpURLLoad("get_scale_for_zoom_into_editable_test.html");
 
-    int viewportWidth = 640;
-    int viewportHeight = 480;
+    int viewportWidth = 450;
+    int viewportHeight = 300;
     float leftBoxRatio = 0.3f;
     int caretPadding = 10;
-    int minReadableCaretHeight = 18;
+    float minReadableCaretHeight = 18.0f;
     WebKit::WebView* webView = FrameTestHelpers::createWebViewAndLoad(m_baseURL + "get_scale_for_zoom_into_editable_test.html");
+    webView->settings()->setApplyDeviceScaleFactorInCompositor(true);
+    webView->settings()->setApplyPageScaleFactorInCompositor(true);
     webView->enableFixedLayoutMode(true);
     webView->resize(WebSize(viewportWidth, viewportHeight));
-    webView->setPageScaleFactorLimits(1, 10);
+    webView->setPageScaleFactorLimits(1, 4);
     webView->layout();
     webView->setDeviceScaleFactor(1.5f);
     webView->settings()->setAutoZoomFocusedNodeToLegibleScale(true);
@@ -956,46 +955,60 @@ TEST_F(WebFrameTest, DISABLED_DivScrollIntoEditableTest)
 
     // Test scrolling the focused node
     // The edit box is shorter and narrower than the viewport when legible.
+    webView->advanceFocus(false);
+    // Set the caret to the end of the input box.
+    webView->mainFrame()->document().getElementById("EditBoxWithText").to<WebInputElement>().setSelectionRange(1000, 1000);
     setScaleAndScrollAndLayout(webView, WebPoint(0, 0), 1);
     WebRect rect, caret;
     webViewImpl->selectionBounds(caret, rect);
-    webView->scrollFocusedNodeIntoRect(rect);
+
+    float scale;
+    WebCore::IntPoint scroll;
+    bool needAnimation;
+    webViewImpl->computeScaleAndScrollForFocusedNode(webViewImpl->focusedWebCoreNode(), scale, scroll, needAnimation);
+    EXPECT_TRUE(needAnimation);
     // The edit box should be left aligned with a margin for possible label.
-    int hScroll = editBoxWithText.x * webView->pageScaleFactor() - leftBoxRatio * viewportWidth;
-    EXPECT_EQ(hScroll, webView->mainFrame()->scrollOffset().width);
-    int vScroll = editBoxWithText.y * webView->pageScaleFactor() - (viewportHeight - editBoxWithText.height * webView->pageScaleFactor()) / 2;
-    EXPECT_EQ(vScroll, webView->mainFrame()->scrollOffset().height);
-    EXPECT_FLOAT_EQ(webView->deviceScaleFactor() * minReadableCaretHeight / caret.height, webView->pageScaleFactor());
+    int hScroll = editBoxWithText.x - leftBoxRatio * viewportWidth / scale;
+    EXPECT_NEAR(hScroll, scroll.x(), 1);
+    int vScroll = editBoxWithText.y - (viewportHeight / scale - editBoxWithText.height * scale) / 2;
+    EXPECT_NEAR(vScroll, scroll.y(), 1);
+    EXPECT_NEAR(minReadableCaretHeight / caret.height, scale, 0.1);
 
     // The edit box is wider than the viewport when legible.
-    webView->setDeviceScaleFactor(4);
+    viewportWidth = 200;
+    viewportHeight = 150;
+    webView->resize(WebSize(viewportWidth, viewportHeight));
     setScaleAndScrollAndLayout(webView, WebPoint(0, 0), 1);
     webViewImpl->selectionBounds(caret, rect);
-    webView->scrollFocusedNodeIntoRect(rect);
+    webViewImpl->computeScaleAndScrollForFocusedNode(webViewImpl->focusedWebCoreNode(), scale, scroll, needAnimation);
+    EXPECT_TRUE(needAnimation);
     // The caret should be right aligned since the caret would be offscreen when the edit box is left aligned.
-    hScroll = (caret.x + caret.width) * webView->pageScaleFactor() + caretPadding - viewportWidth;
-    EXPECT_EQ(hScroll, webView->mainFrame()->scrollOffset().width);
-    EXPECT_FLOAT_EQ(webView->deviceScaleFactor() * minReadableCaretHeight / caret.height, webView->pageScaleFactor());
+    hScroll = caret.x + caret.width + caretPadding - viewportWidth / scale;
+    EXPECT_NEAR(hScroll, scroll.x(), 1);
+    EXPECT_NEAR(minReadableCaretHeight / caret.height, scale, 0.1);
 
     setScaleAndScrollAndLayout(webView, WebPoint(0, 0), 1);
     // Move focus to edit box with text.
     webView->advanceFocus(false);
     webViewImpl->selectionBounds(caret, rect);
-    webView->scrollFocusedNodeIntoRect(rect);
+    webViewImpl->computeScaleAndScrollForFocusedNode(webViewImpl->focusedWebCoreNode(), scale, scroll, needAnimation);
+    EXPECT_TRUE(needAnimation);
     // The edit box should be left aligned.
-    hScroll = editBoxWithNoText.x * webView->pageScaleFactor();
-    EXPECT_EQ(hScroll, webView->mainFrame()->scrollOffset().width);
-    vScroll = editBoxWithNoText.y * webView->pageScaleFactor() - (viewportHeight - editBoxWithNoText.height * webView->pageScaleFactor()) / 2;
-    EXPECT_EQ(vScroll, webView->mainFrame()->scrollOffset().height);
-    EXPECT_FLOAT_EQ(webView->deviceScaleFactor() * minReadableCaretHeight / caret.height, webView->pageScaleFactor());
+    hScroll = editBoxWithNoText.x;
+    EXPECT_NEAR(hScroll, scroll.x(), 1);
+    vScroll = editBoxWithNoText.y - (viewportHeight / scale - editBoxWithNoText.height) / 2;
+    EXPECT_NEAR(vScroll, scroll.y(), 1);
+    EXPECT_NEAR(minReadableCaretHeight / caret.height, scale, 0.1);
+
+    setScaleAndScrollAndLayout(webViewImpl, scroll, scale);
 
     // Move focus back to the first edit box.
     webView->advanceFocus(true);
-    webViewImpl->selectionBounds(caret, rect);
+    webViewImpl->computeScaleAndScrollForFocusedNode(webViewImpl->focusedWebCoreNode(), scale, scroll, needAnimation);
     // The position should have stayed the same since this box was already on screen with the right scale.
-    EXPECT_EQ(vScroll, webView->mainFrame()->scrollOffset().height);
-    EXPECT_EQ(hScroll, webView->mainFrame()->scrollOffset().width);
+    EXPECT_FALSE(needAnimation);
 }
+
 #endif
 
 class TestReloadDoesntRedirectWebFrameClient : public WebFrameClient {

Source/WebKit/chromium/tests/data/get_scale_for_zoom_into_editable_test.html

@@ -1,16 +1,8 @@
 <html>
 <head>
-<script>
-function getfocus()
-{
-var textfield = document.getElementById('EditBoxWithText');
-textfield.focus();
-textfield.setSelectionRange(textfield.value.length,textfield.value.length);
-}
-</script>
 </head>
-  <body onload="getfocus()">
-      <input id="EditBoxWithText" style=" position: absolute; left: 200px; top: 200px; width: 250; height:20" value = "Long text to fill the edit box" type="text">
-      <input id="EditBoxWithNoText" style=" position: absolute; left: 200px; top: 250px; width: 250; height:20" type="text">
+  <body>
+      <input id="EditBoxWithText" style=" position: absolute; left: 200px; top: 200px; width: 250px; height:20" value = "Long long long long text to fill the edit box" type="text">
+      <input id="EditBoxWithNoText" style=" position: absolute; left: 200px; top: 250px; width: 250px; height:20" type="text">
   </body>
 </html>