Commit e0fb41b1 authored by Stephane Zermatten's avatar Stephane Zermatten Committed by Commit Bot

[Autofill Assistant] Allow scrolling from non-touchable areas.

Before this change TouchEventFilter filtering out touch events that
happen outside of the allowed area would not only filter out taps, but
also scrolls.

This was inconvenient, as users would need to know that they need to
scroll starting from the highlighted, touchable are. Users could also
fling the highlighted area completely out of view, which left them
unable to do anything.

With this change, TouchEventFilter does the scrolling when a touch event
hits a non-allowed area. This requires dealing with two kinds of
scrolling. Once started, scrolling is allowed to continue until the end,
even if the touch event leaves the touchable or non-touchable area.

This change also agressivel triggers update of the touchable area while
scrolling. With browser-controlled scrolling, this corrects any drift
that result from the UI and the page having different scrolling speeds.

Bug: 806868
Change-Id: I85e3fd1339fadf9efe43b793c62ab6ce81c9247b
Reviewed-on: https://chromium-review.googlesource.com/c/1335928
Commit-Queue: Stephane Zermatten <szermatt@chromium.org>
Reviewed-by: default avatarGanggui Tang <gogerald@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608318}
parent 0d5495c3
......@@ -121,6 +121,16 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
mUiDelegateHolder.dismiss(R.string.autofill_assistant_maybe_give_up);
}
@Override
public void scrollBy(float distanceXRatio, float distanceYRatio) {
nativeScrollBy(mUiControllerAndroid, distanceXRatio, distanceYRatio);
}
@Override
public void updateTouchableArea() {
nativeUpdateTouchableArea(mUiControllerAndroid);
}
@Override
public void onScriptSelected(String scriptPath) {
nativeOnScriptSelected(mUiControllerAndroid, scriptPath);
......@@ -433,6 +443,9 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
String[] parameterValues, String locale, String countryCode);
private native void nativeStart(long nativeUiControllerAndroid, String initialUrl);
private native void nativeDestroy(long nativeUiControllerAndroid);
private native void nativeScrollBy(
long nativeUiControllerAndroid, float distanceXRatio, float distanceYRatio);
private native void nativeUpdateTouchableArea(long nativeUiControllerAndroid);
private native void nativeOnScriptSelected(long nativeUiControllerAndroid, String scriptPath);
private native void nativeOnAddressSelected(long nativeUiControllerAndroid, String guid);
private native void nativeOnCardSelected(long nativeUiControllerAndroid, String guid);
......
......@@ -53,6 +53,17 @@ public class TouchEventFilter
public interface Client {
/** Called after a certain number of unexpected taps. */
void onUnexpectedTaps();
/**
* Scroll the browser view by the given distance.
*
* <p>Distances are floats between -1.0 and 1.0, with 1 corresponding to the size of the
* visible viewport.
*/
void scrollBy(float distanceXRatio, float distanceYRatio);
/** Asks for an update of the touchable area. */
void updateTouchableArea();
}
private Client mClient;
......@@ -74,20 +85,23 @@ public class TouchEventFilter
private final RectF mDrawRect = new RectF();
/** Detects gestures that happen outside of the allowed area. */
private final GestureDetector mGestureDetector;
private final GestureDetector mNonBrowserGesture;
/** True while a gesture handled by {@link #mNonBrowserGesture} is in progress. */
private boolean mNonBrowserGestureInProgress;
/** Times, in millisecond, of unexpected taps detected outside of the allowed area. */
private final List<Long> mUnexpectedTapTimes = new ArrayList<>();
/** True while scrolling. */
private boolean mScrolling;
/** True while the browser is scrolling. */
private boolean mBrowserScrolling;
/**
* Scrolling offset to use while scrolling right after scrolling.
*
* <p>This value shifts the touchable area by that many pixels while scrolling.
*/
private int mScrollOffsetY;
private int mBrowserScrollOffsetY;
/**
* Offset reported at the beginning of a scroll.
......@@ -95,7 +109,7 @@ public class TouchEventFilter
* <p>This is used to interpret the offsets reported by subsequent calls to {@link
* #onScrollOffsetOrExtentChanged} or {@link #onScrollEnded}.
*/
private int mInitialScrollOffsetY;
private int mInitialBrowserScrollOffsetY;
/**
* Current offset that applies on mTouchableArea.
......@@ -129,13 +143,21 @@ public class TouchEventFilter
mClear = new Paint();
mClear.setXfermode(new PorterDuffXfermode(PorterDuff.Mode.CLEAR));
// Detect gestures in the unexpected area.
mGestureDetector = new GestureDetector(context, new SimpleOnGestureListener() {
// Handles gestures that happen outside of the allowed area.
mNonBrowserGesture = new GestureDetector(context, new SimpleOnGestureListener() {
@Override
public boolean onSingleTapUp(MotionEvent e) {
onUnexpectedTap(e);
return true;
}
@Override
public boolean onScroll(MotionEvent downEvent, MotionEvent moveEvent, float distanceX,
float distanceY) {
mClient.scrollBy(distanceX / getWidth(), distanceY / getVisualViewportHeight());
return true;
}
});
updateTouchableArea(false, Collections.emptyList());
......@@ -168,7 +190,7 @@ public class TouchEventFilter
clearTouchableArea();
}
if (!mScrolling && !mTouchableArea.equals(rectangles)) {
if (!mTouchableArea.equals(rectangles)) {
clearTouchableArea();
mTouchableArea.addAll(rectangles);
}
......@@ -177,6 +199,8 @@ public class TouchEventFilter
private void clearTouchableArea() {
mTouchableArea.clear();
mOffsetY = 0;
mInitialBrowserScrollOffsetY += mBrowserScrollOffsetY;
mBrowserScrollOffsetY = 0;
invalidate();
}
......@@ -187,10 +211,24 @@ public class TouchEventFilter
// Scrolling is always safe. Let it through.
return false;
case MotionEvent.ACTION_DOWN:
case MotionEvent.ACTION_MOVE:
case MotionEvent.ACTION_UP:
case MotionEvent.ACTION_CANCEL:
if (mNonBrowserGestureInProgress) {
// If a non-browser gesture, started with an ACTION_DOWN, is in progress, give
// it priority over everything else until the next ACTION_DOWN.
mNonBrowserGesture.onTouchEvent(event);
return true;
}
if (mBrowserScrolling) {
// Events sent to the browser triggered scrolling, which was reported to
// onScrollStarted. Direct all events to the browser until the next ACTION_DOWN
// to avoid interrupting that scroll.
return false;
}
// fallthrough
case MotionEvent.ACTION_DOWN:
if (!mEnabled) return false; // let everything through
// Only let through events if they're meant for the touchable area of the screen.
......@@ -201,9 +239,10 @@ public class TouchEventFilter
}
int height = yBottom - yTop;
boolean allowed = isInTouchableArea(((float) event.getX()) / getWidth(),
(((float) event.getY() - yTop + mScrollOffsetY + mOffsetY) / height));
(((float) event.getY() - yTop + mBrowserScrollOffsetY + mOffsetY)
/ height));
if (!allowed) {
mGestureDetector.onTouchEvent(event);
mNonBrowserGestureInProgress = mNonBrowserGesture.onTouchEvent(event);
return true; // handled
}
return false; // let it through
......@@ -236,9 +275,11 @@ public class TouchEventFilter
int height = yBottom - yTop;
for (RectF rect : mTouchableArea) {
mDrawRect.left = rect.left * width - mPaddingPx;
mDrawRect.top = yTop + rect.top * height - mPaddingPx - mScrollOffsetY - mOffsetY;
mDrawRect.top =
yTop + rect.top * height - mPaddingPx - mBrowserScrollOffsetY - mOffsetY;
mDrawRect.right = rect.right * width + mPaddingPx;
mDrawRect.bottom = yTop + rect.bottom * height + mPaddingPx - mScrollOffsetY - mOffsetY;
mDrawRect.bottom =
yTop + rect.bottom * height + mPaddingPx - mBrowserScrollOffsetY - mOffsetY;
if (mDrawRect.left <= 0 && mDrawRect.right >= width) {
// Rounded corners look strange in the case where the rectangle takes exactly the
// width of the screen.
......@@ -272,37 +313,40 @@ public class TouchEventFilter
invalidate();
}
/** Called at the beginning of a scroll gesture triggered by the browser. */
@Override
public void onScrollStarted(int scrollOffsetY, int scrollExtentY) {
mScrolling = true;
mInitialScrollOffsetY = scrollOffsetY;
mScrollOffsetY = 0;
mBrowserScrolling = true;
mInitialBrowserScrollOffsetY = scrollOffsetY;
mBrowserScrollOffsetY = 0;
invalidate();
}
/** Called during a scroll gesture triggered by the browser. */
@Override
public void onScrollOffsetOrExtentChanged(int scrollOffsetY, int scrollExtentY) {
if (!mScrolling) {
if (!mBrowserScrolling) {
// onScrollOffsetOrExtentChanged will be called alone, without onScrollStarted during a
// Javascript-initiated scroll.
//
// TODO(crbug.com/806868): Consider handling these as well, for smoother
// Javascript-initiated scrolling.
mClient.updateTouchableArea();
return;
}
mScrollOffsetY = scrollOffsetY - mInitialScrollOffsetY;
mBrowserScrollOffsetY = scrollOffsetY - mInitialBrowserScrollOffsetY;
invalidate();
mClient.updateTouchableArea();
}
/** Called at the end of a scroll gesture triggered by the browser. */
@Override
public void onScrollEnded(int scrollOffsetY, int scrollExtentY) {
if (!mScrolling) {
if (!mBrowserScrolling) {
return;
}
mOffsetY += (scrollOffsetY - mInitialScrollOffsetY);
mScrollOffsetY = 0;
mScrolling = false;
mOffsetY += (scrollOffsetY - mInitialBrowserScrollOffsetY);
mBrowserScrollOffsetY = 0;
mBrowserScrolling = false;
invalidate();
mClient.updateTouchableArea();
}
/** Considers whether to let the client know about unexpected taps. */
......@@ -347,4 +391,9 @@ public class TouchEventFilter
}
return getHeight() - bottomBarHeight;
}
/** Height of the visual viewport. */
private int getVisualViewportHeight() {
return getVisualViewportBottom() - getVisualViewportTop();
}
}
......@@ -152,6 +152,20 @@ void UiControllerAndroid::UpdateScripts(
delete[] script_highlights;
}
void UiControllerAndroid::ScrollBy(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj,
float distanceX,
float distanceY) {
ui_delegate_->ScrollBy(distanceX, distanceY);
}
void UiControllerAndroid::UpdateTouchableArea(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj) {
ui_delegate_->UpdateTouchableArea();
}
void UiControllerAndroid::OnScriptSelected(
JNIEnv* env,
const JavaParamRef<jobject>& jcaller,
......
......@@ -77,6 +77,12 @@ class UiControllerAndroid : public UiController,
const base::android::JavaParamRef<jobject>& jcaller,
const base::android::JavaParamRef<jstring>& initialUrlString);
void Destroy(JNIEnv* env, const base::android::JavaParamRef<jobject>& obj);
void ScrollBy(JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj,
float distanceX,
float distanceY);
void UpdateTouchableArea(JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj);
void OnScriptSelected(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller,
......
......@@ -325,6 +325,15 @@ void Controller::OnDestroy() {
delete this;
}
void Controller::ScrollBy(float distanceXRatio, float distanceYRatio) {
GetWebController()->ScrollBy(distanceXRatio, distanceYRatio);
touchable_element_area_.UpdatePositions();
}
void Controller::UpdateTouchableArea() {
touchable_element_area_.UpdatePositions();
}
void Controller::DidAttachInterstitialPage() {
GetUiController()->Shutdown();
}
......@@ -342,13 +351,6 @@ void Controller::DidGetUserInteraction(const blink::WebInputEvent::Type type) {
}
break;
case blink::WebInputEvent::kGestureScrollEnd:
case blink::WebInputEvent::kGesturePinchEnd:
// This speeds up update of the area in case where we know something has
// happened.
touchable_element_area_.UpdatePositions();
break;
default:
break;
}
......
......@@ -86,6 +86,8 @@ class Controller : public ScriptExecutorDelegate,
void Start(const GURL& initialUrl) override;
void OnClickOverlay() override;
void OnDestroy() override;
void ScrollBy(float distanceXRatio, float distanceYRatio) override;
void UpdateTouchableArea() override;
void OnScriptSelected(const std::string& script_path) override;
std::string GetDebugContext() override;
......
......@@ -23,6 +23,17 @@ class UiDelegate {
// detached from the associated activity.
virtual void OnDestroy() = 0;
// Scrolls the browser view.
//
// Distance is expressed as a float between -1.0 and 1.0, relative to the
// width or height of the visible viewport.
virtual void ScrollBy(float distanceXRatio, float distanceYRatio) = 0;
// Asks for updated coordinates for the touchable area. This is called to
// speed up update of the touchable areas when there are good reasons to think
// that the current coordinates are out of date, such as while scrolling.
virtual void UpdateTouchableArea() = 0;
// Called when a script was selected for execution.
virtual void OnScriptSelected(const std::string& script_path) = 0;
......
......@@ -8,8 +8,10 @@
#include <algorithm>
#include <utility>
#include "base/bind_helpers.h"
#include "base/callback.h"
#include "base/logging.h"
#include "base/strings/stringprintf.h"
#include "base/task/post_task.h"
#include "build/build_config.h"
#include "components/autofill/content/browser/content_autofill_driver.h"
......@@ -55,6 +57,10 @@ const char* const kScrollIntoViewScript =
node.scrollIntoViewIfNeeded();
})";
const char* const kScrollByScript =
R"(window.scrollBy(%f * window.visualViewport.width,
%f * window.visualViewport.height))";
// Javascript to select a value from a select box. Also fires a "change" event
// to trigger any listeners. Changing the index directly does not trigger this.
const char* const kSelectOptionScript =
......@@ -1148,6 +1154,12 @@ void WebController::GetElementPosition(
weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
}
void WebController::ScrollBy(float distanceXRatio, float distanceYRatio) {
devtools_client_->GetRuntime()->Evaluate(
base::StringPrintf(kScrollByScript, distanceXRatio, distanceYRatio),
base::DoNothing());
}
void WebController::OnFindElementForPosition(
base::OnceCallback<void(bool, const RectF&)> callback,
std::unique_ptr<FindElementResult> result) {
......
......@@ -131,6 +131,12 @@ class WebController {
const std::vector<std::string>& selectors,
base::OnceCallback<void(bool, const RectF&)> callback);
// Scroll the view by the given distance.
//
// Distances are floats between -1.0 and 1.0, with 1 corresponding to the size
// of the visible viewport.
virtual void ScrollBy(float distanceXRatio, float distanceYRatio);
protected:
friend class BatchElementChecker;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment