Commit 8948cc53 authored by Stephane Zermatten's avatar Stephane Zermatten Committed by Commit Bot

[Autofill Assistant] Smoother scrolling for highlighted element.

This change improves the scrolling for highlighted elements.

Before this change, while scrolling, highlighted elements were updated
by the periodic update of the element position, which happens every
100ms. While scrolling, this produces noticeable jumps.

After this change, while scrolling, elements position is updated based
on the scroll position and the periodic updates are ignored. This means
that scrolling of the highlighted area follows the UI scrolling, which
is much smoother.

Note that there can still be a shift between the highlighted area and
the element, as the web page doesn't always scroll exactly at the same
speed as the UI elements, but the shift is much smaller and smoother.

Bug: 806868
Change-Id: Ide5f9b6f0ba472c93ced2dd26b4cbaf2ee14b59f
Reviewed-on: https://chromium-review.googlesource.com/c/1331401
Commit-Queue: Stephane Zermatten <szermatt@chromium.org>
Reviewed-by: default avatarMathias Carlen <mcarlen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607527}
parent c027688f
...@@ -275,7 +275,8 @@ class AutofillAssistantUiDelegate { ...@@ -275,7 +275,8 @@ class AutofillAssistantUiDelegate {
mOverlay = mFullContainer.findViewById(R.id.overlay); mOverlay = mFullContainer.findViewById(R.id.overlay);
mOverlay.setOnClickListener(unusedView -> mClient.onClickOverlay()); mOverlay.setOnClickListener(unusedView -> mClient.onClickOverlay());
mTouchEventFilter = (TouchEventFilter) mFullContainer.findViewById(R.id.touch_event_filter); mTouchEventFilter = (TouchEventFilter) mFullContainer.findViewById(R.id.touch_event_filter);
mTouchEventFilter.init(client, activity.getFullscreenManager()); mTouchEventFilter.init(client, activity.getFullscreenManager(),
activity.getActivityTab().getWebContents());
mBottomBar = mFullContainer.findViewById(R.id.bottombar); mBottomBar = mFullContainer.findViewById(R.id.bottombar);
mBottomBar.findViewById(R.id.close_button) mBottomBar.findViewById(R.id.close_button)
.setOnClickListener(unusedView -> mClient.onDismiss()); .setOnClickListener(unusedView -> mClient.onDismiss());
......
...@@ -22,6 +22,9 @@ import android.view.View; ...@@ -22,6 +22,9 @@ import android.view.View;
import org.chromium.base.ApiCompatibilityUtils; import org.chromium.base.ApiCompatibilityUtils;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.fullscreen.ChromeFullscreenManager; import org.chromium.chrome.browser.fullscreen.ChromeFullscreenManager;
import org.chromium.content_public.browser.GestureListenerManager;
import org.chromium.content_public.browser.GestureStateListener;
import org.chromium.content_public.browser.WebContents;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
...@@ -37,7 +40,8 @@ import java.util.List; ...@@ -37,7 +40,8 @@ import java.util.List;
* *
* <p>TODO(crbug.com/806868): Consider merging this view with the overlay. * <p>TODO(crbug.com/806868): Consider merging this view with the overlay.
*/ */
public class TouchEventFilter extends View implements ChromeFullscreenManager.FullscreenListener { public class TouchEventFilter
extends View implements ChromeFullscreenManager.FullscreenListener, GestureStateListener {
/** /**
* Complain after there's been {@link TAP_TRACKING_COUNT} taps within * Complain after there's been {@link TAP_TRACKING_COUNT} taps within
* {@link @TAP_TRACKING_DURATION_MS} in the unallowed area. * {@link @TAP_TRACKING_DURATION_MS} in the unallowed area.
...@@ -75,6 +79,32 @@ public class TouchEventFilter extends View implements ChromeFullscreenManager.Fu ...@@ -75,6 +79,32 @@ public class TouchEventFilter extends View implements ChromeFullscreenManager.Fu
/** Times, in millisecond, of unexpected taps detected outside of the allowed area. */ /** Times, in millisecond, of unexpected taps detected outside of the allowed area. */
private final List<Long> mUnexpectedTapTimes = new ArrayList<>(); private final List<Long> mUnexpectedTapTimes = new ArrayList<>();
/** True while scrolling. */
private boolean mScrolling;
/**
* 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;
/**
* Offset reported at the beginning of a scroll.
*
* <p>This is used to interpret the offsets reported by subsequent calls to {@link
* #onScrollOffsetOrExtentChanged} or {@link #onScrollEnded}.
*/
private int mInitialScrollOffsetY;
/**
* Current offset that applies on mTouchableArea.
*
* <p>This value shifts the touchable area by that many pixels after the end of a scroll and
* before the next update, which resets this value.
*/
private int mOffsetY;
public TouchEventFilter(Context context) { public TouchEventFilter(Context context) {
this(context, null, 0); this(context, null, 0);
} }
...@@ -112,10 +142,12 @@ public class TouchEventFilter extends View implements ChromeFullscreenManager.Fu ...@@ -112,10 +142,12 @@ public class TouchEventFilter extends View implements ChromeFullscreenManager.Fu
} }
/** Initializes dependencies. */ /** Initializes dependencies. */
public void init(Client client, ChromeFullscreenManager fullscreenManager) { public void init(
Client client, ChromeFullscreenManager fullscreenManager, WebContents webContents) {
mClient = client; mClient = client;
mFullscreenManager = fullscreenManager; mFullscreenManager = fullscreenManager;
mFullscreenManager.addListener(this); mFullscreenManager.addListener(this);
GestureListenerManager.fromWebContents(webContents).addListener(this);
} }
/** Sets the color to be used for unusable areas. */ /** Sets the color to be used for unusable areas. */
...@@ -130,10 +162,15 @@ public class TouchEventFilter extends View implements ChromeFullscreenManager.Fu ...@@ -130,10 +162,15 @@ public class TouchEventFilter extends View implements ChromeFullscreenManager.Fu
* @param rectangles rectangles defining the area that can be used, may be empty * @param rectangles rectangles defining the area that can be used, may be empty
*/ */
public void updateTouchableArea(boolean enabled, List<RectF> rectangles) { public void updateTouchableArea(boolean enabled, List<RectF> rectangles) {
if (mEnabled == enabled && mScrolling || mTouchableArea.equals(rectangles)) {
return;
}
mEnabled = enabled; mEnabled = enabled;
setAlpha(mEnabled ? 1.0f : 0.0f); setAlpha(mEnabled ? 1.0f : 0.0f);
mTouchableArea.clear(); mTouchableArea.clear();
mTouchableArea.addAll(rectangles); mTouchableArea.addAll(rectangles);
mOffsetY = 0;
invalidate(); invalidate();
} }
...@@ -158,7 +195,7 @@ public class TouchEventFilter extends View implements ChromeFullscreenManager.Fu ...@@ -158,7 +195,7 @@ public class TouchEventFilter extends View implements ChromeFullscreenManager.Fu
} }
int height = yBottom - yTop; int height = yBottom - yTop;
boolean allowed = isInTouchableArea(((float) event.getX()) / getWidth(), boolean allowed = isInTouchableArea(((float) event.getX()) / getWidth(),
((float) event.getY() - yTop) / height); (((float) event.getY() - yTop + mScrollOffsetY + mOffsetY) / height));
if (!allowed) { if (!allowed) {
mGestureDetector.onTouchEvent(event); mGestureDetector.onTouchEvent(event);
return true; // handled return true; // handled
...@@ -193,9 +230,9 @@ public class TouchEventFilter extends View implements ChromeFullscreenManager.Fu ...@@ -193,9 +230,9 @@ public class TouchEventFilter extends View implements ChromeFullscreenManager.Fu
int height = yBottom - yTop; int height = yBottom - yTop;
for (RectF rect : mTouchableArea) { for (RectF rect : mTouchableArea) {
mDrawRect.left = rect.left * width - mPaddingPx; mDrawRect.left = rect.left * width - mPaddingPx;
mDrawRect.top = yTop + rect.top * height - mPaddingPx; mDrawRect.top = yTop + rect.top * height - mPaddingPx - mScrollOffsetY - mOffsetY;
mDrawRect.right = rect.right * width + mPaddingPx; mDrawRect.right = rect.right * width + mPaddingPx;
mDrawRect.bottom = yTop + rect.bottom * height + mPaddingPx; mDrawRect.bottom = yTop + rect.bottom * height + mPaddingPx - mScrollOffsetY - mOffsetY;
if (mDrawRect.left <= 0 && mDrawRect.right >= width) { if (mDrawRect.left <= 0 && mDrawRect.right >= width) {
// Rounded corners look strange in the case where the rectangle takes exactly the // Rounded corners look strange in the case where the rectangle takes exactly the
// width of the screen. // width of the screen.
...@@ -229,6 +266,39 @@ public class TouchEventFilter extends View implements ChromeFullscreenManager.Fu ...@@ -229,6 +266,39 @@ public class TouchEventFilter extends View implements ChromeFullscreenManager.Fu
invalidate(); invalidate();
} }
@Override
public void onScrollStarted(int scrollOffsetY, int scrollExtentY) {
mScrolling = true;
mInitialScrollOffsetY = scrollOffsetY;
mScrollOffsetY = 0;
invalidate();
}
@Override
public void onScrollOffsetOrExtentChanged(int scrollOffsetY, int scrollExtentY) {
if (!mScrolling) {
// 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.
return;
}
mScrollOffsetY = scrollOffsetY - mInitialScrollOffsetY;
invalidate();
}
@Override
public void onScrollEnded(int scrollOffsetY, int scrollExtentY) {
if (!mScrolling) {
return;
}
mOffsetY += (scrollOffsetY - mInitialScrollOffsetY);
mScrollOffsetY = 0;
mScrolling = false;
invalidate();
}
/** Considers whether to let the client know about unexpected taps. */ /** Considers whether to let the client know about unexpected taps. */
private void onUnexpectedTap(MotionEvent e) { private void onUnexpectedTap(MotionEvent e) {
long eventTimeMs = e.getEventTime(); long eventTimeMs = e.getEventTime();
......
...@@ -98,13 +98,6 @@ void ElementArea::OnGetElementPosition(const std::vector<std::string>& selector, ...@@ -98,13 +98,6 @@ void ElementArea::OnGetElementPosition(const std::vector<std::string>& selector,
for (auto& position : element_positions_) { for (auto& position : element_positions_) {
if (position.selector == selector) { if (position.selector == selector) {
// found == false, has all coordinates set to 0.0, which clears the area. // found == false, has all coordinates set to 0.0, which clears the area.
if (position.rect.left == rect.left && position.rect.top == rect.top &&
position.rect.right == rect.right &&
position.rect.bottom == rect.bottom) {
// Avoid reporting unnecessary updates
return;
}
position.rect = rect; position.rect = rect;
ReportUpdate(); ReportUpdate();
return; return;
......
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