Fix focus to scroll input into view consistently

Focus gain causes virtual keyboard to show. Then virtual keyboard takes
the bottom inset and causes the window and view size to shrink, on
chrome and sometimes on webview.
At this point, the bottom inset may occlude the input form,
i.e. the input form is behind the keyboard and is not visible.

We have the following auto-scrolling logic to mitigate the inconvenience
here:
1) Check the size of visible display size after showing keyboard.
2) Wait for the view height change after showing keyboard.
3) Change the viewport size
4) Scroll focused editable node into view.

In the browser, this is in order. However, in the renderer, the order
of 3) and 4) sometimes gets reversed.
I suspect that this happens because the two messages are going through
different IPC channels.

Now we do the following:
1) Wait until SynchronizeVisualProperties() changes visual viewport.
2) Detect that blink's scrollable viewport size has changed
3) Comfirm this signal against the visible display size
4) Scroll focused editable node into view

This also adds some test and refactors ImeAdapterImpl.

Bug: 920061

Change-Id: Id237fa76c304c92b5459bda4fe6591c96cb804d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1562585Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Commit-Queue: Changwan Ryu <changwan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682991}

android_webview/javatests/src/org/chromium/android_webview/test/AwActivityTestRule.java

@@ -10,6 +10,7 @@ import android.content.Context;
 import android.support.test.InstrumentationRegistry;
 import android.support.test.rule.ActivityTestRule;
 import android.util.AndroidRuntimeException;
+import android.util.Base64;
 import android.view.ViewGroup;
 
 import org.junit.Assert;
@@ -236,6 +237,13 @@ public class AwActivityTestRule extends ActivityTestRule<AwTestRunnerActivity> {
                 currentCallCount, 1, WAIT_TIMEOUT_MS, TimeUnit.MILLISECONDS);
     }
 
+    public void loadHtmlSync(final AwContents awContents, CallbackHelper onPageFinishedHelper,
+            final String html) throws Throwable {
+        int currentCallCount = onPageFinishedHelper.getCallCount();
+        final String encodedData = Base64.encodeToString(html.getBytes(), Base64.NO_PADDING);
+        loadDataSync(awContents, onPageFinishedHelper, encodedData, "text/html", true);
+    }
+
     public void loadDataSyncWithCharset(final AwContents awContents,
             CallbackHelper onPageFinishedHelper, final String data, final String mimeType,
             final boolean isBase64Encoded, final String charset) throws Exception {

android_webview/javatests/src/org/chromium/android_webview/test/AwImeTest.java

@@ -4,13 +4,20 @@
 
 package org.chromium.android_webview.test;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.greaterThan;
+
 import android.content.Context;
+import android.graphics.Rect;
 import android.support.test.filters.SmallTest;
 import android.view.KeyEvent;
+import android.view.ViewGroup;
 import android.view.inputmethod.InputConnection;
 import android.view.inputmethod.InputMethodManager;
 import android.webkit.JavascriptInterface;
 import android.widget.EditText;
+import android.widget.LinearLayout;
+import android.widget.LinearLayout.LayoutParams;
 
 import org.junit.Before;
 import org.junit.Rule;
@@ -21,8 +28,10 @@ import org.chromium.base.test.util.CallbackHelper;
 import org.chromium.base.test.util.DisabledTest;
 import org.chromium.base.test.util.Feature;
 import org.chromium.content_public.browser.ImeAdapter;
+import org.chromium.content_public.browser.WebContents;
 import org.chromium.content_public.browser.test.util.Criteria;
 import org.chromium.content_public.browser.test.util.CriteriaHelper;
+import org.chromium.content_public.browser.test.util.DOMUtils;
 import org.chromium.content_public.browser.test.util.TestInputMethodManagerWrapper;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 
@@ -60,9 +69,21 @@ public class AwImeTest {
             // Use detached container view to avoid focus request.
             mTestContainerView =
                     mActivityTestRule.createDetachedAwTestContainerView(mContentsClient);
+            mTestContainerView.setLayoutParams(
+                    new LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT));
             mEditText = new EditText(mActivityTestRule.getActivity());
-            mActivityTestRule.getActivity().addView(mEditText);
-            mActivityTestRule.getActivity().addView(mTestContainerView);
+            LinearLayout linearLayout = new LinearLayout(mActivityTestRule.getActivity());
+            linearLayout.setOrientation(LinearLayout.VERTICAL);
+            linearLayout.setLayoutParams(
+                    new LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT));
+
+            // Ensures that we don't autofocus EditText.
+            linearLayout.setDescendantFocusability(ViewGroup.FOCUS_AFTER_DESCENDANTS);
+            linearLayout.setFocusableInTouchMode(true);
+
+            mActivityTestRule.getActivity().addView(linearLayout);
+            linearLayout.addView(mEditText);
+            linearLayout.addView(mTestContainerView);
             mTestContainerView.getAwContents().addJavascriptInterface(
                     mTestJavascriptInterface, "test");
             // Let's not test against real input method.
@@ -70,6 +91,7 @@ public class AwImeTest {
             imeAdapter.setInputMethodManagerWrapper(
                     TestInputMethodManagerWrapper.create(imeAdapter));
         });
+        AwActivityTestRule.enableJavaScriptOnUiThread(mTestContainerView.getAwContents());
     }
 
     private void loadContentEditableBody() throws Exception {
@@ -81,6 +103,18 @@ public class AwImeTest {
                 mTestContainerView.getAwContents(), loadHelper, htmlDocument, mime, false);
     }
 
+    private void loadBottomInputHtml() throws Throwable {
+        // Shows an input at the bottom of the screen.
+        final String htmlDocument = "<html><head>"
+                + "<style>html, body{background-color:beige} "
+                + "div{position:absolute;top:10000px;}</style></head>"
+                + "<body>Test<div id='footer'><input id='input_text'><br/></div></body></html>";
+        final CallbackHelper loadHelper = mContentsClient.getOnPageFinishedHelper();
+
+        mActivityTestRule.loadHtmlSync(
+                mTestContainerView.getAwContents(), loadHelper, htmlDocument);
+    }
+
     private void focusOnEditTextAndShowKeyboard() {
         TestThreadUtils.runOnUiThreadBlocking(() -> {
             mEditText.requestFocus();
@@ -94,7 +128,6 @@ public class AwImeTest {
     private void focusOnWebViewAndEnableEditing() throws Exception {
         TestThreadUtils.runOnUiThreadBlocking((Runnable) () -> mTestContainerView.requestFocus());
 
-        AwActivityTestRule.enableJavaScriptOnUiThread(mTestContainerView.getAwContents());
         // View focus may not have been propagated to the renderer process yet. If document is not
         // yet focused, and focusing on an element is an invalid operation. See crbug.com/622151
         // for details.
@@ -207,4 +240,60 @@ public class AwImeTest {
             }
         });
     }
+
+    private void scrollBottomOfNodeIntoView(String nodeId) throws Exception {
+        mActivityTestRule.executeJavaScriptAndWaitForResult(mTestContainerView.getAwContents(),
+                mContentsClient,
+                "document.getElementById('" + nodeId + "').scrollIntoView(false);");
+    }
+
+    private float getScrollY() throws Exception {
+        float res = Float.parseFloat(mActivityTestRule.executeJavaScriptAndWaitForResult(
+                mTestContainerView.getAwContents(), mContentsClient, "window.scrollY"));
+        return res;
+    }
+
+    // https://crbug.com/920061
+    @Test
+    @SmallTest
+    public void testFocusAndViewSizeChangeCausesScroll() throws Throwable {
+        loadBottomInputHtml();
+        Rect currentRect = new Rect();
+
+        TestThreadUtils.runOnUiThreadBlocking(
+                () -> { mTestContainerView.getWindowVisibleDisplayFrame(currentRect); });
+        WebContents webContents = mTestContainerView.getAwContents().getWebContents();
+
+        DOMUtils.waitForNonZeroNodeBounds(webContents, "input_text");
+
+        float initialScrollY = getScrollY();
+
+        scrollBottomOfNodeIntoView("footer");
+        // footer's offset is 10000 px from top. Scrolling this node into view should definitely
+        // change the scroll.
+        float scrollYAtBottom = getScrollY();
+        assertThat(scrollYAtBottom, greaterThan(initialScrollY));
+
+        DOMUtils.clickNode(webContents, "input_text", true /* goThroughAndroidRootView */,
+                false /* shouldScrollIntoView */);
+
+        TestThreadUtils.runOnUiThreadBlocking(() -> {
+            // Expect that we may have a size change.
+            ImeAdapter imeAdapter = ImeAdapter.fromWebContents(mTestContainerView.getWebContents());
+            imeAdapter.onShowKeyboardReceiveResult(InputMethodManager.RESULT_SHOWN);
+
+            // When a virtual keyboard shows up, the window and view size shrink. Note that we are
+            // not depend on the real virtual keyboard behavior here. We only emulate the behavior
+            // by shrinking the view height.
+            currentRect.bottom = currentRect.centerY();
+            mTestContainerView.setWindowVisibleDisplayFrameOverride(currentRect);
+            int width = mTestContainerView.getWidth();
+            int height = mTestContainerView.getHeight();
+            mTestContainerView.onSizeChanged(width, height / 2, width, height);
+        });
+
+        // Scrolling may take some time. Ensures that scrolling happened.
+        CriteriaHelper.pollInstrumentationThread(
+                () -> (getScrollY() > scrollYAtBottom), "Scrolling should happen.");
+    }
 }

android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java

@@ -48,6 +48,8 @@ public class AwTestContainerView extends FrameLayout {
     private HardwareView mHardwareView;
     private boolean mAttachedContents;
 
+    private Rect mWindowVisibleDisplayFrameOverride;
+
     private class HardwareView extends GLSurfaceView {
         private static final int MODE_DRAW = 0;
         private static final int MODE_PROCESS = 1;
@@ -281,6 +283,19 @@ public class AwTestContainerView extends FrameLayout {
         }
     }
 
+    public void setWindowVisibleDisplayFrameOverride(Rect rect) {
+        mWindowVisibleDisplayFrameOverride = rect;
+    }
+
+    @Override
+    public void getWindowVisibleDisplayFrame(Rect outRect) {
+        if (mWindowVisibleDisplayFrameOverride != null) {
+            outRect.set(mWindowVisibleDisplayFrameOverride);
+        } else {
+            super.getWindowVisibleDisplayFrame(outRect);
+        }
+    }
+
     public boolean isBackedByHardwareView() {
         return mHardwareView != null;
     }

content/browser/android/ime_adapter_android.cc

@@ -189,14 +189,6 @@ void ImeAdapterAndroid::UpdateState(const TextInputState& state) {
       state.composition_end, state.reply_to_request);
 }
 
-void ImeAdapterAndroid::UpdateAfterViewSizeChanged() {
-  JNIEnv* env = AttachCurrentThread();
-  ScopedJavaLocalRef<jobject> obj = java_ime_adapter_.get(env);
-  if (obj.is_null())
-    return;
-  Java_ImeAdapterImpl_updateAfterViewSizeChanged(env, obj);
-}
-
 void ImeAdapterAndroid::UpdateOnTouchDown() {
   JNIEnv* env = AttachCurrentThread();
   ScopedJavaLocalRef<jobject> obj = java_ime_adapter_.get(env);
@@ -232,6 +224,24 @@ void ImeAdapterAndroid::UpdateFrameInfo(
       insertion_marker_top, insertion_marker_bottom);
 }
 
+void ImeAdapterAndroid::OnRenderFrameMetadataChangedAfterActivation(
+    const gfx::SizeF& new_viewport_size) {
+  if (old_viewport_size_ == new_viewport_size)
+    return;
+
+  JNIEnv* env = AttachCurrentThread();
+  ScopedJavaLocalRef<jobject> obj = java_ime_adapter_.get(env);
+  if (obj.is_null())
+    return;
+
+  const jboolean surface_height_reduced =
+      new_viewport_size.width() == old_viewport_size_.width() &&
+      new_viewport_size.height() < old_viewport_size_.height();
+  old_viewport_size_ = new_viewport_size;
+  Java_ImeAdapterImpl_onResizeScrollableViewport(env, obj,
+                                                 surface_height_reduced);
+}
+
 bool ImeAdapterAndroid::SendKeyEvent(
     JNIEnv* env,
     const JavaParamRef<jobject>&,

content/browser/android/ime_adapter_android.h

@@ -13,6 +13,7 @@
 #include "content/browser/android/render_widget_host_connector.h"
 #include "content/common/content_export.h"
 #include "ui/gfx/geometry/rect_f.h"
+#include "ui/gfx/geometry/size.h"
 
 namespace ui {
 
@@ -99,6 +100,7 @@ class CONTENT_EXPORT ImeAdapterAndroid : public RenderWidgetHostConnector {
   void UpdateFrameInfo(const gfx::SelectionBound& selection_start,
                        float dip_scale,
                        float content_offset_ypix);
+  void OnRenderFrameMetadataChangedAfterActivation(const gfx::SizeF& new_size);
 
   // Called from native -> java
   void CancelComposition();
@@ -111,7 +113,6 @@ class CONTENT_EXPORT ImeAdapterAndroid : public RenderWidgetHostConnector {
   }
 
   void UpdateState(const TextInputState& state);
-  void UpdateAfterViewSizeChanged();
   void UpdateOnTouchDown();
 
   void AdvanceFocusInForm(JNIEnv*,
@@ -128,6 +129,8 @@ class CONTENT_EXPORT ImeAdapterAndroid : public RenderWidgetHostConnector {
       const base::android::JavaParamRef<jobject>& text,
       const base::string16& text16);
 
+  gfx::SizeF old_viewport_size_;
+
   // Current RenderWidgetHostView connected to this instance. Can be null.
   RenderWidgetHostViewAndroid* rwhva_;
   JavaObjectWeakGlobalRef java_ime_adapter_;

content/browser/renderer_host/render_widget_host_view_android.cc

@@ -559,6 +559,23 @@ void RenderWidgetHostViewAndroid::WriteContentBitmapToDiskAsync(
                   std::move(result_callback));
 }
 
+void RenderWidgetHostViewAndroid::
+    OnRenderFrameMetadataChangedAfterActivation() {
+  if (ime_adapter_android_) {
+    const cc::RenderFrameMetadata& metadata =
+        host()->render_frame_metadata_provider()->LastRenderFrameMetadata();
+    // We need to first wait for Blink's viewport size to change such that we
+    // can correctly scroll to the currently focused input.
+    // On Clank, only visible viewport size changes and device viewport size or
+    // viewport_size_in_pixels do not change according to the window/view size
+    /// change. Only scrollable viewport size changes both for Chrome and
+    // WebView.
+    ime_adapter_android_->OnRenderFrameMetadataChangedAfterActivation(
+        metadata.scrollable_viewport_size);
+  }
+  RenderWidgetHostViewBase::OnRenderFrameMetadataChangedAfterActivation();
+}
+
 void RenderWidgetHostViewAndroid::Focus() {
   if (view_.HasFocus())
     GotFocus();
@@ -2221,11 +2238,6 @@ bool RenderWidgetHostViewAndroid::RequiresDoubleTapGestureEvents() const {
   return true;
 }
 
-void RenderWidgetHostViewAndroid::OnSizeChanged() {
-  if (ime_adapter_android_)
-    ime_adapter_android_->UpdateAfterViewSizeChanged();
-}
-
 void RenderWidgetHostViewAndroid::OnPhysicalBackingSizeChanged() {
   EvictFrameIfNecessary();
   // We may need to update the background color to match pre-surface-sync

content/browser/renderer_host/render_widget_host_view_android.h

@@ -202,7 +202,6 @@ class CONTENT_EXPORT RenderWidgetHostViewAndroid
   bool OnMouseEvent(const ui::MotionEventAndroid& m) override;
   bool OnMouseWheelEvent(const ui::MotionEventAndroid& event) override;
   bool OnGestureEvent(const ui::GestureEventAndroid& event) override;
-  void OnSizeChanged() override;
   void OnPhysicalBackingSizeChanged() override;
 
   // ui::ViewAndroidObserver implementation:
@@ -346,6 +345,7 @@ class CONTENT_EXPORT RenderWidgetHostViewAndroid
   // RenderFrameMetadataProvider::Observer
   void OnRenderFrameMetadataChangedBeforeActivation(
       const cc::RenderFrameMetadata& metadata) override;
+  void OnRenderFrameMetadataChangedAfterActivation() override;
 
   void WasEvicted();
 

content/public/android/java/src/org/chromium/content/browser/input/ImeAdapterImpl.java

@@ -512,10 +512,7 @@ public class ImeAdapterImpl implements ImeAdapter, WindowEventObserver, UserData
         }
     }
 
-    /**
-     * Call this when we get result from ResultReceiver passed in calling showSoftInput().
-     * @param resultCode The result of showSoftInput() as defined in InputMethodManager.
-     */
+    @Override
     public void onShowKeyboardReceiveResult(int resultCode) {
         View containerView = getContainerView();
         if (resultCode == InputMethodManager.RESULT_SHOWN) {
@@ -534,25 +531,6 @@ public class ImeAdapterImpl implements ImeAdapter, WindowEventObserver, UserData
         }
     }
 
-    @CalledByNative
-    private void updateAfterViewSizeChanged() {
-        // 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();
-                }
-                // Zero the rect to prevent the above operation from issuing the delayed
-                // form focus event.
-                cancelRequestToScrollFocusedEditableNodeIntoView();
-            }
-        }
-    }
-
     @CalledByNative
     private void updateOnTouchDown() {
         cancelRequestToScrollFocusedEditableNodeIntoView();
@@ -996,6 +974,30 @@ public class ImeAdapterImpl implements ImeAdapter, WindowEventObserver, UserData
                 insertionMarkerTop, insertionMarkerBottom, getContainerView());
     }
 
+    @CalledByNative
+    private void onResizeScrollableViewport(boolean contentsHeightReduced) {
+        if (!contentsHeightReduced) {
+            cancelRequestToScrollFocusedEditableNodeIntoView();
+            return;
+        }
+
+        // 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();
+                }
+                // Zero the rect to prevent the above operation from issuing the delayed
+                // form focus event.
+                cancelRequestToScrollFocusedEditableNodeIntoView();
+            }
+        }
+    }
+
     private int getUnderlineColorForSuggestionSpan(SuggestionSpan suggestionSpan) {
         try {
             Method getUnderlineColorMethod = SuggestionSpan.class.getMethod("getUnderlineColor");

content/public/android/java/src/org/chromium/content_public/browser/ImeAdapter.java

@@ -93,4 +93,11 @@ public interface ImeAdapter {
      */
     @VisibleForTesting
     void setComposingTextForTest(final CharSequence text, final int newCursorPosition);
+
+    /**
+     * Call this when we get result from ResultReceiver passed in calling showSoftInput().
+     * @param resultCode The result of showSoftInput() as defined in InputMethodManager.
+     */
+    @VisibleForTesting
+    void onShowKeyboardReceiveResult(int resultCode);
 }

content/public/test/android/javatests/src/org/chromium/content_public/browser/test/util/DOMUtils.java

@@ -253,7 +253,22 @@ public class DOMUtils {
      */
     public static boolean clickNode(final WebContents webContents, String nodeId,
             boolean goThroughRootAndroidView) throws InterruptedException, TimeoutException {
-        scrollNodeIntoView(webContents, nodeId);
+        return clickNode(
+                webContents, nodeId, goThroughRootAndroidView, true /* shouldScrollIntoView */);
+    }
+
+    /**
+     * Click a DOM node by its id.
+     * @param webContents The WebContents in which the node lives.
+     * @param nodeId The id of the node.
+     * @param goThroughRootAndroidView Whether the input should be routed through the Root View for
+     *        the CVC.
+     * @param shouldScrollIntoView Whether to scroll the node into view first.
+     */
+    public static boolean clickNode(final WebContents webContents, String nodeId,
+            boolean goThroughRootAndroidView, boolean shouldScrollIntoView)
+            throws InterruptedException, TimeoutException {
+        if (shouldScrollIntoView) scrollNodeIntoView(webContents, nodeId);
         int[] clickTarget = getClickTargetForNode(webContents, nodeId);
         if (goThroughRootAndroidView) {
             return TouchCommon.singleClickView(