Bug 1532582 - Display autofill popup in correct location.

This autofill popover was being displayed in the incorrect place because the display rect we were providing to the `AutofillManager` was the rect for the `GeckoView` and not the rect for the HTML element that the autofill popover was relating to.

1. Add view dimensions to info passed to autofill in `GeckoViewAutoFill`.
2. Use those view dimensions to calculate the correct location on the screen using `pageToScreenMatrix` in `GeckoSession`.

The resulting locations were incorrect, as the values used by `pageToScreenMatrix` were out of date. The `GeckoSession` was only notified about updated metrics during first composite, which meant that when the metrics changed during zoom and scroll on soft keyboard presentation, `GeckoSession` was unaware of it.

3. Update `GeckoSession` with new screen metrics when they change and not only during first composite.

Despite this change ensuring that `GeckoSession` always had the correct values for the viewport size and location, the request to provide the autofill location was made before the zoom and scroll was complete, meaning that even then out of date values were used during the calculation. The intial solution was to fire an event once zoom was complete, but despite this event being fired after the new screen size had been calculcated in `AsyncCompositionManager`, `GeckoSession` did not receive the values until after the event had been processed (the calls were out by 0.024ms).

5. Call new method `onScreenMetricsUpdated` inside `SessionTextInput` after screen metrics have been updated. Call `AutofillManager#notifyViewEntered` from this function.

This was not my preferred solution to this, but timing issues meant I could not find/think of an alternative way of delaying the calculation of the autofill popover location until after `GeckoSession` had been updated.

This patch currently fixes things on GV apps. Occasionally, on Fennec, the autofill view is out of alignment slightly. This needs further work.

Differential Revision: https://phabricator.services.mozilla.com/D25406

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Emily Toop 2019-03-29 15:25:42 +00:00
parent de2d589e58
commit d75296da81
9 changed files with 101 additions and 28 deletions

View File

@ -6,7 +6,6 @@
#include "mozilla/layers/AsyncCompositionManager.h"
#include <stdint.h> // for uint32_t
#include "FrameMetrics.h" // for FrameMetrics
#include "LayerManagerComposite.h" // for LayerManagerComposite, etc
#include "Layers.h" // for Layer, ContainerLayer, etc
#include "gfxPoint.h" // for gfxPoint, gfxSize
@ -1041,18 +1040,25 @@ bool AsyncCompositionManager::ApplyAsyncContentTransformToTree(
compositor->GetCompositorBridgeParent()) {
AndroidDynamicToolbarAnimator* animator =
bridge->GetAndroidDynamicToolbarAnimator();
if (mIsFirstPaint) {
LayersId rootLayerTreeId = bridge->RootLayerTreeId();
if (mIsFirstPaint || FrameMetricsHaveUpdated(metrics)) {
if (animator) {
animator->UpdateRootFrameMetrics(metrics);
} else if (RefPtr<UiCompositorControllerParent> uiController =
UiCompositorControllerParent::
GetFromRootLayerTreeId(rootLayerTreeId)) {
uiController->NotifyUpdateScreenMetrics(metrics);
}
mLastMetrics = metrics;
}
if (mIsFirstPaint) {
if (animator) {
animator->FirstPaint();
}
LayersId rootLayerTreeId = bridge->RootLayerTreeId();
if (RefPtr<UiCompositorControllerParent> uiController =
UiCompositorControllerParent::
GetFromRootLayerTreeId(rootLayerTreeId)) {
if (!animator) {
uiController->NotifyUpdateScreenMetrics(metrics);
}
uiController->NotifyFirstPaint();
}
mIsFirstPaint = false;
@ -1231,6 +1237,13 @@ bool AsyncCompositionManager::ApplyAsyncContentTransformToTree(
return appliedTransform;
}
#if defined(MOZ_WIDGET_ANDROID)
bool AsyncCompositionManager::FrameMetricsHaveUpdated(const FrameMetrics& aMetrics) {
return RoundedToInt(mLastMetrics.GetScrollOffset()) != RoundedToInt(aMetrics.GetScrollOffset())
|| mLastMetrics.GetZoom() != aMetrics.GetZoom();;
}
#endif
static bool LayerIsScrollbarTarget(const LayerMetricsWrapper& aTarget,
Layer* aScrollbar) {
if (!aTarget.GetApzc()) {

View File

@ -8,6 +8,7 @@
#define GFX_ASYNCCOMPOSITIONMANAGER_H
#include "Units.h" // for ScreenPoint, etc
#include "FrameMetrics.h" // for FrameMetrics
#include "mozilla/layers/LayerManagerComposite.h" // for LayerManagerComposite
#include "mozilla/Attributes.h" // for final, etc
#include "mozilla/RefPtr.h" // for RefCounted
@ -238,6 +239,11 @@ class AsyncCompositionManager final {
void SetFixedLayerMargins(ScreenIntCoord aTop, ScreenIntCoord aBottom);
private:
// This calculates whether frame metrics should be sent to Java.
bool FrameMetricsHaveUpdated(const FrameMetrics& aMetrics);
// This holds the most recent scroll/zoom metrics sent to Java, and is used
// to send new updates when it changes.
FrameMetrics mLastMetrics;
// The following two fields are only needed on Fennec with C++ APZ, because
// then we need to reposition the gecko scrollbar to deal with the
// dynamic toolbar shifting content around.

View File

@ -165,7 +165,6 @@ class GeckoViewContentChild extends GeckoViewChildModule {
.exitFullscreen();
}
break;
case "GeckoView:ZoomToInput": {
let dwu = content.windowUtils;

View File

@ -6,10 +6,10 @@
<body>
<form>
<input type="text" id="user1" value="foo">
<input type="password" id="pass1", value="foo">
<input type="email" id="email1", value="@">
<input type="number" id="number1", value="0">
<input type="tel" id="tel1", value="0">
<input type="password" id="pass1" value="foo">
<input type="email" id="email1" value="@">
<input type="number" id="number1" value="0">
<input type="tel" id="tel1" value="0">
</form>
<input type="Text" id="user2" value="foo">
<input type="PassWord" id="pass2" maxlength="8" value="foo">

View File

@ -407,6 +407,7 @@ class ContentDelegateTest : BaseSessionTest() {
countAutoFillNodes({ it.isFocused }), equalTo(0))
}
@WithDisplay(height = 100, width = 100)
@WithDevToolsAPI
@Test fun autofill_userpass() {
if (Build.VERSION.SDK_INT < 26) {

View File

@ -1447,6 +1447,7 @@ import android.view.inputmethod.EditorInfo;
if (DEBUG) {
Log.d(LOGTAG, "restartInput(" + reason + ", " + toggleSoftInput + ')');
}
if (toggleSoftInput) {
mSoftInputReentrancyGuard.incrementAndGet();
}
@ -1595,6 +1596,7 @@ import android.view.inputmethod.EditorInfo;
// show/hide the keyboard because the find box has the focus and is taking input from
// the keyboard.
final GeckoSession session = mSession.get();
if (session == null) {
return;
}
@ -1613,15 +1615,9 @@ import android.view.inputmethod.EditorInfo;
return;
}
if (state == SessionTextInput.EditableListener.IME_STATE_DISABLED) {
if (DEBUG) {
Log.d(LOGTAG, "hideSoftInput");
}
session.getTextInput().getDelegate().hideSoftInput(session);
return;
}
if (DEBUG) {
Log.d(LOGTAG, "showSoftInput");
}
session.getEventDispatcher().dispatch("GeckoView:ZoomToInput", null);
session.getTextInput().getDelegate().showSoftInput(session);
}

View File

@ -4598,6 +4598,8 @@ public class GeckoSession implements Parcelable {
mViewportLeft = scrollX;
mViewportTop = scrollY;
mViewportZoom = zoom;
mTextInput.onScreenMetricsUpdated();
}
/* protected */ void onWindowBoundsChanged() {

View File

@ -5,6 +5,7 @@
package org.mozilla.geckoview;
import org.mozilla.gecko.GeckoAppShell;
import org.mozilla.gecko.InputMethods;
import org.mozilla.gecko.annotation.WrapForJNI;
import org.mozilla.gecko.IGeckoEditableParent;
@ -18,6 +19,7 @@ import org.mozilla.gecko.util.ThreadUtils;
import android.annotation.TargetApi;
import android.app.Activity;
import android.content.Context;
import android.graphics.Matrix;
import android.graphics.Rect;
import android.graphics.RectF;
import android.os.Build;
@ -245,6 +247,41 @@ public final class SessionTextInput {
}
}
private Rect displayRectForId(@NonNull final GeckoSession session,
@NonNull final int virtualId,
@Nullable final View view) {
final SessionTextInput textInput = session.getTextInput();
Rect contentRect;
if (textInput.mAutoFillNodes.indexOfKey(virtualId) >= 0) {
GeckoBundle element = textInput.mAutoFillNodes
.get(virtualId);
GeckoBundle bounds = element.getBundle("bounds");
contentRect = new Rect(bounds.getInt("left"),
bounds.getInt("top"),
bounds.getInt("right"),
bounds.getInt("bottom"));
final Matrix matrix = new Matrix();
final RectF rectF = new RectF(contentRect);
if (GeckoAppShell.isFennec()) {
session.getClientToScreenMatrix(matrix);
} else {
session.getPageToScreenMatrix(matrix);
}
matrix.mapRect(rectF);
rectF.roundOut(contentRect);
if (DEBUG) {
Log.d(LOGTAG, "Displaying autofill rect at (" + contentRect.left + ", " +
contentRect.top + "), (" + contentRect.right + ", " +
contentRect.bottom + ")");
}
} else {
contentRect = getDummyAutoFillRect(session, true, view);
}
return contentRect;
}
@Override
public void notifyAutoFill(@NonNull final GeckoSession session,
@AutoFillNotification final int notification,
@ -273,9 +310,7 @@ public final class SessionTextInput {
manager.cancel();
break;
case AUTO_FILL_NOTIFY_VIEW_ENTERED:
// Use a dummy rect for the View.
manager.notifyViewEntered(view, virtualId, getDummyAutoFillRect(
session, /* screen */ true, view));
manager.notifyViewEntered(view, virtualId, displayRectForId(session, virtualId, view));
break;
case AUTO_FILL_NOTIFY_VIEW_EXITED:
manager.notifyViewExited(view, virtualId);
@ -503,6 +538,13 @@ public final class SessionTextInput {
return mDelegate;
}
/*package*/ void onScreenMetricsUpdated() {
if (mAutoFillFocusedId != View.NO_ID) {
getDelegate().notifyAutoFill(
mSession, GeckoSession.TextInputDelegate.AUTO_FILL_NOTIFY_VIEW_ENTERED, mAutoFillFocusedId);
}
}
/**
* Fill the specified {@link ViewStructure} with auto-fill fields from the current page.
*
@ -521,8 +563,7 @@ public final class SessionTextInput {
structure.setEnabled(true);
structure.setVisibility(View.VISIBLE);
final Rect rect = getDummyAutoFillRect(mSession, /* screen */ false,
/* view */ null);
final Rect rect = getDummyAutoFillRect(mSession, false, null);
structure.setDimens(rect.left, rect.top, 0, 0, rect.width(), rect.height());
if (mAutoFillRoots == null) {
@ -720,9 +761,8 @@ public final class SessionTextInput {
if (DEBUG) {
Log.d(LOGTAG, "addAutoFill(" + id + ')');
}
mAutoFillRoots.append(id, callback);
mAutoFillNodes.append(id, message);
populateAutofillNodes(message);
if (initializing) {
getDelegate().notifyAutoFill(
@ -733,6 +773,19 @@ public final class SessionTextInput {
}
}
private void populateAutofillNodes(final GeckoBundle bundle) {
final int id = bundle.getInt("id");
mAutoFillNodes.append(id, bundle);
final GeckoBundle[] children = bundle.getBundleArray("children");
if (children != null) {
for (GeckoBundle child : children) {
populateAutofillNodes(child);
}
}
}
/* package */ void clearAutoFill() {
if (mAutoFillRoots == null) {
return;
@ -777,10 +830,6 @@ public final class SessionTextInput {
mAutoFillFocusedId = id;
mAutoFillFocusedRoot = root;
if (id != View.NO_ID) {
getDelegate().notifyAutoFill(
mSession, GeckoSession.TextInputDelegate.AUTO_FILL_NOTIFY_VIEW_ENTERED, id);
}
}
/* package */ static Rect getDummyAutoFillRect(@NonNull final GeckoSession session,

View File

@ -84,6 +84,7 @@ class GeckoViewAutoFill {
if (info) {
return info;
}
const bounds = element.getBoundingClientRect();
info = {
id: ++this._autoFillId,
parent,
@ -101,6 +102,12 @@ class GeckoViewAutoFill {
.map(attr => ({[attr.localName]: attr.value}))),
origin: element.ownerDocument.location.origin,
autofillhint: "",
bounds: {
left: bounds.left,
top: bounds.top,
right: bounds.right,
bottom: bounds.bottom,
},
};
if (element === usernameField) {