Resize only the virtual viewport when the OSK triggers a resize.

content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java

Index: content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
index f3bd7259805dd3b53aa13dab667da64e6df83851..31f5a3c329b7c9500628ab622e16f770b969e22a 100644
--- a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
+++ b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
@@ -502,6 +502,7 @@ public class ContentViewCore implements
     private int mPhysicalBackingHeightPix;
     private int mTopControlsHeightPix;
     private boolean mTopControlsShrinkBlinkSize;
+    private int mViewportHeightWithoutOSKPix;

[bokan, 2015/10/08 22:38:21]

This is misleading since it implies we're excluding the OSK rect from the screen
rect. I think this should be ViewportHeightIncludingOSK or
ViewportHeightWithOSKHidden or something like that.

[ymalik, 2015/10/09 01:05:25]

Done.

 
     // Cached copy of all positions and scales as reported by the renderer.
     private final RenderCoordinates mRenderCoordinates;
@@ -1088,6 +1089,14 @@ public class ContentViewCore implements
     }
 
     /**
+     * @return Viewport height without the OSK in physical pixels as set from onSizeChanged.
+     */
+    @CalledByNative
+    public int getViewportHeightWithoutOSKPix() {
+        return mViewportHeightWithoutOSKPix;
+    }
+
+    /**
      * @return Width of underlying physical surface.
      */
     @CalledByNative
@@ -1619,13 +1628,44 @@ public class ContentViewCore implements
     public void onSizeChanged(int wPix, int hPix, int owPix, int ohPix) {

[bokan, 2015/10/08 22:38:21]

It seems we might now always get an onSizeChanged: crbug.com/294908

Jared might know if this is ok or what we can do about it.

         if (getViewportWidthPix() == wPix && getViewportHeightPix() == hPix) return;
 
+        boolean displayRectChanged = false, triggeredByOSKShow = false;

[bokan, 2015/10/08 22:38:21]

Nit: Variable declarations should get their own lines.

[ymalik, 2015/10/09 01:05:25]

Done.

+        if (!mFocusPreOSKViewportRect.isEmpty()) {
+            Rect rect = new Rect();
+            getContainerView().getWindowVisibleDisplayFrame(rect);
+            displayRectChanged = !rect.equals(mFocusPreOSKViewportRect);
+            // Only assume the OSK triggered the onSizeChanged if width was preserved.
+            triggeredByOSKShow = displayRectChanged
+                    && rect.width() == mFocusPreOSKViewportRect.width();
+        }
+
+        // triggeredByOSKShow is true if we are resizing to show the OSK. In this case,
+        // mViewportHeightWithoutOSK is what mViewportHeightPix was previously. Otherwise
+        // we are either resizing to hide the OSK or the onSizeChanged is not triggered by OSK,
+        // in which case the mViewportHeightWithoutOSK is hPix.

[bokan, 2015/10/08 22:38:21]

Comments should describe the "why?" not the "how". Your last sentence "in which
case the mViewportHeightWithoutOSK is hPix" adds nothing to understanding. Why
is it hPix? A: "there's no OSK to include in the height"

[ymalik, 2015/10/09 01:05:25]

Thanks. Done.

+        if (triggeredByOSKShow) {
+            mViewportHeightWithoutOSKPix = mViewportHeightPix;
+        } else {
+            mViewportHeightWithoutOSKPix = hPix;
+        }
+
         mViewportWidthPix = wPix;
         mViewportHeightPix = hPix;
         if (mNativeContentViewCore != 0) {
             nativeWasResized(mNativeContentViewCore);
         }
 
-        updateAfterSizeChanged();
+        mPopupZoomer.hide(false);
+
+        // Execute a delayed form focus operation because the OSK was brought
+        // up earlier.
+        if (triggeredByOSKShow) {
+            assert mWebContents != null;
+            mWebContents.scrollFocusedEditableNodeIntoView();
+        }
+        if (displayRectChanged) {
+            cancelRequestToScrollFocusedEditableNodeIntoView();
+        }
+        cancelRequestToScrollFocusedEditableNodeIntoView();

[bokan, 2015/10/08 22:38:21]

Is this accidentally duplicated? It makes the above 'if' nonsensical.

[ymalik, 2015/10/09 01:05:25]

Most definitely an accident.

     }
 
     /**
@@ -1647,27 +1687,8 @@ public class ContentViewCore implements
     public void onOverdrawBottomHeightChanged(int overdrawHeightPix) {
     }
 
-    private void updateAfterSizeChanged() {
-        mPopupZoomer.hide(false);
-
-        // Execute a delayed form focus operation because the OSK was brought
-        // up earlier.
-        if (!mFocusPreOSKViewportRect.isEmpty()) {
-            Rect rect = new Rect();
-            getContainerView().getWindowVisibleDisplayFrame(rect);
-            if (!rect.equals(mFocusPreOSKViewportRect)) {
-                // Only assume the OSK triggered the onSizeChanged if width was preserved.
-                if (rect.width() == mFocusPreOSKViewportRect.width()) {
-                    assert mWebContents != null;
-                    mWebContents.scrollFocusedEditableNodeIntoView();
-                }
-                cancelRequestToScrollFocusedEditableNodeIntoView();
-            }
-        }
-    }
-
     private void cancelRequestToScrollFocusedEditableNodeIntoView() {
-        // Zero-ing the rect will prevent |updateAfterSizeChanged()| from
+        // Zero-ing the rect will prevent |onSizeChanged()| from
         // issuing the delayed form focus event.
         mFocusPreOSKViewportRect.setEmpty();
     }