Refine autozoom scaling constants and logic

Source/web/WebViewImpl.cpp

Index: Source/web/WebViewImpl.cpp
diff --git a/Source/web/WebViewImpl.cpp b/Source/web/WebViewImpl.cpp
index 8836341267581ea88fede135a0af516a7b3bfb95..826921fb2b020ef54fddafc1123a0f9bffa669f3 100644
--- a/Source/web/WebViewImpl.cpp
+++ b/Source/web/WebViewImpl.cpp
@@ -182,8 +182,9 @@ static const float viewportAnchorYCoord = 0;
 
 // Constants for zooming in on a focused text field.
 static const double scrollAndScaleAnimationDurationInSeconds = 0.2;
-static const int minReadableCaretHeight = 18;
-static const float minScaleChangeToTriggerZoom = 1.05f;
+static const int minReadableCaretHeight = 16;
+static const float minScaleChangeToTriggerZoom = 1.35f;
+static const float minScaleChangeFromBlockScaleToTriggerZoom = 2.25f;
 static const float leftBoxRatio = 0.3f;
 static const int caretPadding = 10;
 
@@ -2530,7 +2531,28 @@ void WebViewImpl::computeScaleAndScrollForFocusedNode(Node* focusedNode, float&
     // the caret height will become minReadableCaretHeight (adjusted for dpi
     // and font scale factor).
     newScale = clampPageScaleFactorToLimits(legibleScale() * minReadableCaretHeight / caret.height);
-    const float deltaScale = newScale / pageScaleFactor();
+
+    // Avoiding zooming in further if already zoomed sufficiently within the focused block.
+    if (newScale > pageScaleFactor()) {
+        WebRect blockBounds = computeBlockBounds(textboxRect, false);
+
+        float blockScale;
+        WebPoint blockScroll;
+        computeScaleAndScrollForBlockRect(textboxRect.location(), blockBounds, touchPointPadding, minimumPageScaleFactor() * doubleTapZoomAlreadyLegibleRatio, blockScale, blockScroll);

[aelias_OOO_until_Jul13, 2014/03/14 07:16:10]

Hmm, this invokes more code than is necessary, particularly the computation of
scroll position which you then ignore.  And I don't like introducing a
dependency on double-tap zoom code because it makes it hard to change double-tap
zoom behavior without unexpected fallout on textbox behavior (or vice versa). 
If you want to keep this, could you rewrite it from first principles instead
(maybe start by copy-pasting the scale-computation code you're relying on and
then trim down any unnecessary parts?).

+        const float deltaScaleFromBlockScale = newScale / blockScale;
+        if (pageScaleFactor() > blockScale && deltaScaleFromBlockScale < minScaleChangeFromBlockScaleToTriggerZoom)
+            newScale = pageScaleFactor();
+    }
+
+    needAnimation = false;
+    // If we're sufficiently far from the target level, zoom in/out as necessary.
+    if (newScale / pageScaleFactor() > minScaleChangeToTriggerZoom
+        || pageScaleFactor() / newScale > minScaleChangeToTriggerZoom) {
+        needAnimation = true;
+    } else {
+        // Avoid changing the scale if it's already sufficiently close to the target.
+        newScale = pageScaleFactor();
+    }
 
     // Convert the rects to absolute space in the new scale.
     IntRect textboxRectInDocumentCoordinates = textboxRect;
@@ -2541,6 +2563,19 @@ void WebViewImpl::computeScaleAndScrollForFocusedNode(Node* focusedNode, float&
     int viewWidth = m_size.width / newScale;
     int viewHeight = m_size.height / newScale;
 
+    // If the caret is offscreen, then animate.
+    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)
+        return;
+
     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
@@ -2561,19 +2596,6 @@ void WebViewImpl::computeScaleAndScrollForFocusedNode(Node* focusedNode, float&
         // be offscreen, in which case bottom-align the caret.
         newScroll.setY(max<int>(textboxRectInDocumentCoordinates.y(), caretInDocumentCoordinates.y() + caretInDocumentCoordinates.height() + caretPadding - viewHeight));
     }
-
-    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, 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;
 }
 
 void WebViewImpl::advanceFocus(bool reverse)