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

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

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.content.browser;
 
 import android.annotation.SuppressLint;
 import android.annotation.TargetApi;
 import android.app.Activity;
 import android.app.SearchManager;
@@ skipping 484 matching lines @@
     private PastePopupMenu mPastePopupMenu;
     private boolean mWasPastePopupShowingOnInsertionDragStart;
 
     // Size of the viewport in physical pixels as set from onSizeChanged.
     private int mViewportWidthPix;
     private int mViewportHeightPix;
     private int mPhysicalBackingWidthPix;
     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;
 
     // Provides smooth gamepad joystick-driven scrolling.
     private final JoystickScrollProvider mJoystickScrollProvider;
 
     private boolean mIsMobileOptimizedHint;
 
     // Tracks whether a selection is currently active.  When applied to selected text, indicates
@@ skipping 566 matching lines @@
 
     /**
      * @return Viewport height in physical pixels as set from onSizeChanged.
      */
     @CalledByNative
     public int getViewportHeightPix() {
         return mViewportHeightPix;
     }
 
     /**
+     * @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
     public int getPhysicalBackingWidthPix() {
         return mPhysicalBackingWidthPix;
     }
 
     /**
      * @return Height of underlying physical surface.
      */
@@ skipping 508 matching lines @@
         } finally {
             mCurrentConfig = newConfig;
             TraceEvent.end("ContentViewCore.onConfigurationChanged");
         }
     }
 
     /**
      * @see View#onSizeChanged(int, int, int, int)
      */
     @SuppressWarnings("javadoc")
     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.

     }
 
     /**
      * Called when the underlying surface the compositor draws to changes size.
      * This may be larger than the viewport size.
      */
     public void onPhysicalBackingSizeChanged(int wPix, int hPix) {
         if (mPhysicalBackingWidthPix == wPix && mPhysicalBackingHeightPix == hPix) return;
 
         mPhysicalBackingWidthPix = wPix;
         mPhysicalBackingHeightPix = hPix;
 
         if (mNativeContentViewCore != 0) {
             nativeWasResized(mNativeContentViewCore);
         }
     }
 
     /* TODO(aelias): Remove this after downstream callers disappear. */
     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();
     }
 
     /**
      * @see View#onWindowFocusChanged(boolean)
      */
     public void onWindowFocusChanged(boolean hasWindowFocus) {
         if (!hasWindowFocus) resetGestureDetection();
         if (mActionMode != null) mActionMode.onWindowFocusChanged(hasWindowFocus);
@@ skipping 1672 matching lines @@
     private native void nativeSetTextTrackSettings(long nativeContentViewCoreImpl,
             boolean textTracksEnabled, String textTrackBackgroundColor, String textTrackFontFamily,
             String textTrackFontStyle, String textTrackFontVariant, String textTrackTextColor,
             String textTrackTextShadow, String textTrackTextSize);
 
     private native void nativeExtractSmartClipData(long nativeContentViewCoreImpl,
             int x, int y, int w, int h);
 
     private native void nativeSetBackgroundOpaque(long nativeContentViewCoreImpl, boolean opaque);
 }