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.
The fix is as follows:
- Do not send scroll as a separate message, but send it as a parameter
in SynchronizeVisualProperties. Luckily, we already have it.
- Change ImeAdapterImpl::UpdateAfterSizeChange() to
UpdateSizeChangeForScroll() which returns a boolean.
- Move SynchronizeVisualProperties() call from WCVA into RWHVA such that
we can pass shouldScrollIntoView as a parameter.
(Both OnSizeChanged() calls in WCVA and RWHVA come from ViewAndroid.)
- Minor plumbing to add a new parameter to the function.
Bug: 920061
Change-Id: Ibe4bc70b5f5710cc8622b3f24d0f71c4c29989c9
Reviewed-on: https://chromium-review.googlesource.com/c/1404077
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Changwan Ryu <changwan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623773}diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/AwImeTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/AwImeTest.java
index aafd221..866b71a9 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/AwImeTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/AwImeTest.java
@@ -5,12 +5,16 @@
package org.chromium.android_webview.test;
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;
@@ -22,8 +26,10 @@
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;
/**
@@ -60,9 +66,21 @@
// 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 +88,7 @@
imeAdapter.setInputMethodManagerWrapper(
TestInputMethodManagerWrapper.create(imeAdapter));
});
+ AwActivityTestRule.enableJavaScriptOnUiThread(mTestContainerView.getAwContents());
}
private void loadContentEditableBody() throws Exception {
@@ -81,6 +100,18 @@
mTestContainerView.getAwContents(), loadHelper, htmlDocument, mime, false);
}
+ private void loadBottomInputHtml() throws Throwable {
+ final String mime = "text/html";
+ // Shows an input at the bottom of the screen.
+ final String htmlDocument = "<html><head>"
+ + "<style>html, body{height:100%;} div{position:absolute;bottom:5;}</style>"
+ + "</head><body>Test<div id='footer'><input id='input_text'></div></body></html>";
+ final CallbackHelper loadHelper = mContentsClient.getOnPageFinishedHelper();
+
+ mActivityTestRule.loadDataSync(
+ mTestContainerView.getAwContents(), loadHelper, htmlDocument, mime, false);
+ }
+
private void focusOnEditTextAndShowKeyboard() {
ThreadUtils.runOnUiThreadBlocking(() -> {
mEditText.requestFocus();
@@ -94,7 +125,6 @@
private void focusOnWebViewAndEnableEditing() throws Exception {
ThreadUtils.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 +237,39 @@
}
});
}
+
+ private int getScrollY() throws Exception {
+ return Integer.parseInt(mActivityTestRule.executeJavaScriptAndWaitForResult(
+ mTestContainerView.getAwContents(), mContentsClient, "window.scrollY"));
+ }
+
+ // https://crbug.com/920061
+ @Test
+ @SmallTest
+ public void testFocusAndViewSizeChangeCausesScroll() throws Throwable {
+ loadBottomInputHtml();
+ Rect currentRect = new Rect();
+ mTestContainerView.getWindowVisibleDisplayFrame(currentRect);
+ WebContents webContents = mTestContainerView.getAwContents().getWebContents();
+
+ int initialScrollY = getScrollY();
+
+ DOMUtils.waitForNonZeroNodeBounds(webContents, "input_text");
+ DOMUtils.clickNode(webContents, "input_text");
+
+ // 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 window and view sizes.
+ ThreadUtils.runOnUiThreadBlocking(() -> {
+ mTestContainerView.setWindowVisibleDisplayFrameOverride(new Rect(
+ currentRect.left, currentRect.top, currentRect.right, currentRect.centerY()));
+ 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() > initialScrollY), "Scrolling did not happen");
+ }
}
diff --git a/android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java b/android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java
index c19edf3..7d28c3bb 100644
--- a/android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java
+++ b/android_webview/test/shell/src/org/chromium/android_webview/test/AwTestContainerView.java
@@ -44,6 +44,8 @@
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;
@@ -230,6 +232,19 @@
}
}
+ 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;
}
diff --git a/content/browser/android/ime_adapter_android.cc b/content/browser/android/ime_adapter_android.cc
index ef3b7a5..1750f18c 100644
--- a/content/browser/android/ime_adapter_android.cc
+++ b/content/browser/android/ime_adapter_android.cc
@@ -189,12 +189,12 @@
state.reply_to_request);
}
-void ImeAdapterAndroid::UpdateAfterViewSizeChanged() {
+bool ImeAdapterAndroid::UpdateSizeChangeForScroll() {
JNIEnv* env = AttachCurrentThread();
ScopedJavaLocalRef<jobject> obj = java_ime_adapter_.get(env);
if (obj.is_null())
- return;
- Java_ImeAdapterImpl_updateAfterViewSizeChanged(env, obj);
+ return false;
+ return Java_ImeAdapterImpl_updateSizeChangeForScroll(env, obj);
}
void ImeAdapterAndroid::UpdateOnTouchDown() {
diff --git a/content/browser/android/ime_adapter_android.h b/content/browser/android/ime_adapter_android.h
index 2b16c70..38a8600 100644
--- a/content/browser/android/ime_adapter_android.h
+++ b/content/browser/android/ime_adapter_android.h
@@ -105,7 +105,8 @@
}
void UpdateState(const TextInputState& state);
- void UpdateAfterViewSizeChanged();
+ // Update size change, and return whether we need to scroll input into view.
+ bool UpdateSizeChangeForScroll();
void UpdateOnTouchDown();
void AdvanceFocusInForm(JNIEnv*,
diff --git a/content/browser/renderer_host/render_widget_host_view_android.cc b/content/browser/renderer_host/render_widget_host_view_android.cc
index 575ff95..ac4aeb3 100644
--- a/content/browser/renderer_host/render_widget_host_view_android.cc
+++ b/content/browser/renderer_host/render_widget_host_view_android.cc
@@ -266,7 +266,8 @@
bool RenderWidgetHostViewAndroid::SynchronizeVisualProperties(
const cc::DeadlinePolicy& deadline_policy,
const base::Optional<viz::LocalSurfaceIdAllocation>&
- child_local_surface_id_allocation) {
+ child_local_surface_id_allocation,
+ bool scroll_focused_node_into_view) {
if (child_local_surface_id_allocation) {
local_surface_id_allocator_.UpdateFromChild(
*child_local_surface_id_allocation);
@@ -285,7 +286,7 @@
GetCompositorViewportPixelSize());
}
- return host()->SynchronizeVisualProperties();
+ return host()->SynchronizeVisualProperties(scroll_focused_node_into_view);
}
void RenderWidgetHostViewAndroid::SetSize(const gfx::Size& size) {
@@ -911,7 +912,8 @@
void RenderWidgetHostViewAndroid::EnsureSurfaceSynchronizedForWebTest() {
++latest_capture_sequence_number_;
SynchronizeVisualProperties(cc::DeadlinePolicy::UseInfiniteDeadline(),
- base::nullopt);
+ base::nullopt,
+ /* scroll_focused_node_into_view */ false);
}
uint32_t RenderWidgetHostViewAndroid::GetCaptureSequenceNumber() const {
@@ -1038,7 +1040,8 @@
bool RenderWidgetHostViewAndroid::RequestRepaintForTesting() {
return SynchronizeVisualProperties(cc::DeadlinePolicy::UseDefaultDeadline(),
- base::nullopt);
+ base::nullopt,
+ /* scroll_focused_node_into_view */ false);
}
void RenderWidgetHostViewAndroid::SynchronousFrameMetadata(
@@ -1351,7 +1354,8 @@
void RenderWidgetHostViewAndroid::OnDidUpdateVisualPropertiesComplete(
const cc::RenderFrameMetadata& metadata) {
SynchronizeVisualProperties(cc::DeadlinePolicy::UseDefaultDeadline(),
- metadata.local_surface_id_allocation);
+ metadata.local_surface_id_allocation,
+ /* scroll_focused_node_into_view */ false);
// We've just processed new RenderFrameMetadata and potentially embedded a
// new surface for that data. Check if we need to evict it.
EvictFrameIfNecessary();
@@ -1380,7 +1384,7 @@
? cc::DeadlinePolicy::UseSpecifiedDeadline(
ui::DelegatedFrameHostAndroid::FirstFrameTimeoutFrames())
: cc::DeadlinePolicy::UseDefaultDeadline(),
- base::nullopt);
+ base::nullopt, /* scroll_focused_node_into_view */ false);
}
host()->WasShown(false /* record_presentation_time */);
@@ -1972,7 +1976,7 @@
SynchronizeVisualProperties(
cc::DeadlinePolicy::UseSpecifiedDeadline(
ui::DelegatedFrameHostAndroid::ResizeTimeoutFrames()),
- base::nullopt);
+ base::nullopt, /* scroll_focused_node_into_view */ false);
}
if (!touch_selection_controller_) {
@@ -2058,8 +2062,13 @@
}
void RenderWidgetHostViewAndroid::OnSizeChanged() {
- if (ime_adapter_android_)
- ime_adapter_android_->UpdateAfterViewSizeChanged();
+ bool scroll_focused_node_into_view = false;
+ if (ime_adapter_android_) {
+ scroll_focused_node_into_view =
+ ime_adapter_android_->UpdateSizeChangeForScroll();
+ }
+ SynchronizeVisualProperties(cc::DeadlinePolicy::UseDefaultDeadline(),
+ base::nullopt, scroll_focused_node_into_view);
}
void RenderWidgetHostViewAndroid::OnPhysicalBackingSizeChanged() {
@@ -2067,7 +2076,7 @@
SynchronizeVisualProperties(
cc::DeadlinePolicy::UseSpecifiedDeadline(
ui::DelegatedFrameHostAndroid::ResizeTimeoutFrames()),
- base::nullopt);
+ base::nullopt, /* scroll_focused_node_into_view */ false);
}
void RenderWidgetHostViewAndroid::OnRootWindowVisibilityChanged(bool visible) {
@@ -2351,7 +2360,8 @@
void RenderWidgetHostViewAndroid::OnSynchronizedDisplayPropertiesChanged() {
SynchronizeVisualProperties(cc::DeadlinePolicy::UseDefaultDeadline(),
- base::nullopt);
+ base::nullopt,
+ /* scroll_focused_node_into_view */ false);
}
base::Optional<SkColor> RenderWidgetHostViewAndroid::GetBackgroundColor()
@@ -2373,10 +2383,12 @@
if (is_first_navigation_) {
SynchronizeVisualProperties(
cc::DeadlinePolicy::UseExistingDeadline(),
- local_surface_id_allocator_.GetCurrentLocalSurfaceIdAllocation());
+ local_surface_id_allocator_.GetCurrentLocalSurfaceIdAllocation(),
+ /* scroll_focused_node_into_view */ false);
} else {
SynchronizeVisualProperties(cc::DeadlinePolicy::UseExistingDeadline(),
- base::nullopt);
+ base::nullopt,
+ /* scroll_focused_node_into_view */ false);
}
}
delegated_frame_host_->DidNavigate();
@@ -2408,7 +2420,8 @@
// is no guarantee that they will occur after the eviction.
SynchronizeVisualProperties(
cc::DeadlinePolicy::UseExistingDeadline(),
- local_surface_id_allocator_.GetCurrentLocalSurfaceIdAllocation());
+ local_surface_id_allocator_.GetCurrentLocalSurfaceIdAllocation(),
+ /* scroll_focused_node_into_view */ false);
} else {
local_surface_id_allocator_.Invalidate();
}
diff --git a/content/browser/renderer_host/render_widget_host_view_android.h b/content/browser/renderer_host/render_widget_host_view_android.h
index a70fe2f9..96398ea 100644
--- a/content/browser/renderer_host/render_widget_host_view_android.h
+++ b/content/browser/renderer_host/render_widget_host_view_android.h
@@ -284,7 +284,8 @@
bool SynchronizeVisualProperties(
const cc::DeadlinePolicy& deadline_policy,
const base::Optional<viz::LocalSurfaceIdAllocation>&
- child_local_surface_id_allocation);
+ child_local_surface_id_allocation,
+ bool scroll_focused_node_into_view);
bool HasValidFrame() const;
diff --git a/content/browser/web_contents/web_contents_android.cc b/content/browser/web_contents/web_contents_android.cc
index b081bcf74..e62b6d1 100644
--- a/content/browser/web_contents/web_contents_android.cc
+++ b/content/browser/web_contents/web_contents_android.cc
@@ -851,8 +851,9 @@
if (rwhva) {
// |SendScreenRects()| indirectly calls GetViewSize() that asks Java layer.
web_contents_->SendScreenRects();
- rwhva->SynchronizeVisualProperties(cc::DeadlinePolicy::UseDefaultDeadline(),
- base::nullopt);
+ rwhva->SynchronizeVisualProperties(
+ cc::DeadlinePolicy::UseDefaultDeadline(), base::nullopt,
+ /* scroll_focused_node_into_view */ false);
}
}
diff --git a/content/browser/web_contents/web_contents_view_android.cc b/content/browser/web_contents/web_contents_view_android.cc
index 097eb03..fe293449 100644
--- a/content/browser/web_contents/web_contents_view_android.cc
+++ b/content/browser/web_contents/web_contents_view_android.cc
@@ -584,8 +584,6 @@
auto* rwhv = GetRenderWidgetHostViewAndroid();
if (rwhv) {
web_contents_->SendScreenRects();
- rwhv->SynchronizeVisualProperties(cc::DeadlinePolicy::UseDefaultDeadline(),
- base::nullopt);
}
}
diff --git a/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapterImpl.java b/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapterImpl.java
index c160562..531f984 100644
--- a/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapterImpl.java
+++ b/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapterImpl.java
@@ -527,7 +527,8 @@
}
@CalledByNative
- private void updateAfterViewSizeChanged() {
+ private boolean updateSizeChangeForScroll() {
+ boolean scrollFocusedEditable = false;
// Execute a delayed form focus operation because the OSK was brought up earlier.
if (!mFocusPreOSKViewportRect.isEmpty()) {
Rect rect = new Rect();
@@ -536,13 +537,14 @@
// Only assume the OSK triggered the onSizeChanged if width was preserved.
if (rect.width() == mFocusPreOSKViewportRect.width()) {
assert mWebContents != null;
- mWebContents.scrollFocusedEditableNodeIntoView();
+ scrollFocusedEditable = true;
}
// Zero the rect to prevent the above operation from issuing the delayed
// form focus event.
cancelRequestToScrollFocusedEditableNodeIntoView();
}
}
+ return scrollFocusedEditable;
}
@CalledByNative
