[CT][EngagementSignals] Add isDirectionUp param to scroll/fling start signal
This CL updates the GestureStateListener methods and the GestureListenerManager(Impl) code (both native and Java) to plumb the scroll direction value. However, there are no listeners that make use of it currently. Bug: 1340074 Change-Id: I0a5e53667968e6655650c7a2369d444c4f83a3da Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3726866 Reviewed-by: Colin Blundell <blundell@chromium.org> Commit-Queue: Sinan Sahin <sinansahin@google.com> Reviewed-by: Theresa Sullivan <twellington@chromium.org> Reviewed-by: Peter Beverloo <peter@chromium.org> Reviewed-by: Jinsuk Kim <jinsukkim@chromium.org> Cr-Commit-Position: refs/heads/main@{#1022668}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
c5e6820bb6
commit
b8c19c8dba
android_webview
java
src
org
chromium
android_webview
javatests
src
org
chromium
android_webview
chrome/android
java
src
org
chromium
chrome
browser
javatests
src
org
chromium
chrome
browser
fullscreen
components
autofill_assistant
android
java
src
org
chromium
components
autofill_assistant
browser_ui
banners
android
java
src
org
chromium
components
browser_ui
banners
content
browser
android
public
android
java
src
org
chromium
content
content_public
browser
javatests
src
org
chromium
content
weblayer/browser/java/org/chromium/weblayer_private
@ -810,7 +810,7 @@ public class AwContents implements SmartClipProvider {
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onScrollStarted(int scrollOffsetY, int scrollExtentY) {
|
||||
public void onScrollStarted(int scrollOffsetY, int scrollExtentY, boolean isDirectionUp) {
|
||||
mZoomControls.invokeZoomPicker();
|
||||
}
|
||||
|
||||
|
@ -144,7 +144,8 @@ public class PopupTouchHandleDrawable extends View implements DisplayAndroidObse
|
||||
mParentPositionListener = (x, y) -> updateParentPosition(x, y);
|
||||
mGestureStateListener = new GestureStateListenerWithScroll() {
|
||||
@Override
|
||||
public void onScrollStarted(int scrollOffsetX, int scrollOffsetY) {
|
||||
public void onScrollStarted(
|
||||
int scrollOffsetX, int scrollOffsetY, boolean isDirectionUp) {
|
||||
setIsScrolling(true);
|
||||
}
|
||||
@Override
|
||||
@ -152,7 +153,8 @@ public class PopupTouchHandleDrawable extends View implements DisplayAndroidObse
|
||||
setIsScrolling(false);
|
||||
}
|
||||
@Override
|
||||
public void onFlingStartGesture(int scrollOffsetY, int scrollExtentY) {
|
||||
public void onFlingStartGesture(
|
||||
int scrollOffsetY, int scrollExtentY, boolean isDirectionUp) {
|
||||
// Fling accounting is unreliable in WebView, as the embedder
|
||||
// can override onScroll() and suppress fling ticking. At best
|
||||
// we have to rely on the scroll offset changing to temporarily
|
||||
|
@ -729,7 +729,8 @@ public class AndroidScrollIntegrationTest {
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onFlingStartGesture(int scrollOffsetY, int scrollExtentY) {}
|
||||
public void onFlingStartGesture(
|
||||
int scrollOffsetY, int scrollExtentY, boolean isDirectionUp) {}
|
||||
|
||||
@Override
|
||||
public void onScrollUpdateGestureConsumed() {
|
||||
|
@ -121,7 +121,7 @@ public class ContextualSearchSelectionController {
|
||||
|
||||
private class ContextualSearchGestureStateListener extends GestureStateListener {
|
||||
@Override
|
||||
public void onScrollStarted(int scrollOffsetY, int scrollExtentY) {
|
||||
public void onScrollStarted(int scrollOffsetY, int scrollExtentY, boolean isDirectionUp) {
|
||||
mHandler.handleScrollStart();
|
||||
}
|
||||
|
||||
|
@ -46,7 +46,8 @@ public final class TabGestureStateListener extends TabWebContentsUserData {
|
||||
private int mLastScrollOffsetY;
|
||||
|
||||
@Override
|
||||
public void onFlingStartGesture(int scrollOffsetY, int scrollExtentY) {
|
||||
public void onFlingStartGesture(
|
||||
int scrollOffsetY, int scrollExtentY, boolean isDirectionUp) {
|
||||
onScrollingStateChanged();
|
||||
}
|
||||
|
||||
@ -56,7 +57,8 @@ public final class TabGestureStateListener extends TabWebContentsUserData {
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onScrollStarted(int scrollOffsetY, int scrollExtentY) {
|
||||
public void onScrollStarted(
|
||||
int scrollOffsetY, int scrollExtentY, boolean isDirectionUp) {
|
||||
onScrollingStateChanged();
|
||||
mLastScrollOffsetY = scrollOffsetY;
|
||||
}
|
||||
|
@ -363,7 +363,8 @@ public class FullscreenManagerTest {
|
||||
final CallbackHelper scrollStartCallback = new CallbackHelper();
|
||||
GestureStateListener scrollListener = new GestureStateListener() {
|
||||
@Override
|
||||
public void onScrollStarted(int scrollOffsetY, int scrollExtentY) {
|
||||
public void onScrollStarted(
|
||||
int scrollOffsetY, int scrollExtentY, boolean isDirectionUp) {
|
||||
scrollStartCallback.notifyCalled();
|
||||
}
|
||||
|
||||
@ -371,7 +372,6 @@ public class FullscreenManagerTest {
|
||||
public void onFlingEndGesture(int scrollOffsetY, int scrollExtentY) {
|
||||
flingEndCallback.notifyCalled();
|
||||
}
|
||||
|
||||
};
|
||||
|
||||
Tab tab = mActivityTestRule.getActivity().getActivityTab();
|
||||
|
@ -74,7 +74,7 @@ public class ScrollToHideGestureListener extends GestureStateListenerWithScroll
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onScrollStarted(int scrollOffsetY, int scrollExtentY) {
|
||||
public void onScrollStarted(int scrollOffsetY, int scrollExtentY, boolean isDirectionUp) {
|
||||
Callback<Integer> offsetController = mContent.getOffsetController();
|
||||
if (offsetController == null) return;
|
||||
|
||||
@ -153,10 +153,10 @@ public class ScrollToHideGestureListener extends GestureStateListenerWithScroll
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onFlingStartGesture(int scrollOffsetY, int scrollExtentY) {
|
||||
public void onFlingStartGesture(int scrollOffsetY, int scrollExtentY, boolean isDirectionUp) {
|
||||
// Flinging and scrolling are handled the same, the sheet follows the movement of the
|
||||
// browser page.
|
||||
onScrollStarted(scrollOffsetY, scrollExtentY);
|
||||
onScrollStarted(scrollOffsetY, scrollExtentY, isDirectionUp);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
@ -232,7 +232,8 @@ public abstract class SwipableOverlayView extends FrameLayout {
|
||||
private float mInitialExtentY;
|
||||
|
||||
@Override
|
||||
public void onFlingStartGesture(int scrollOffsetY, int scrollExtentY) {
|
||||
public void onFlingStartGesture(
|
||||
int scrollOffsetY, int scrollExtentY, boolean isDirectionUp) {
|
||||
if (!isAllowedToAutoHide() || !cancelCurrentAnimation()) return;
|
||||
resetInternalScrollState(scrollOffsetY, scrollExtentY);
|
||||
mGestureState = Gesture.FLINGING;
|
||||
@ -260,7 +261,8 @@ public abstract class SwipableOverlayView extends FrameLayout {
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onScrollStarted(int scrollOffsetY, int scrollExtentY) {
|
||||
public void onScrollStarted(
|
||||
int scrollOffsetY, int scrollExtentY, boolean isDirectionUp) {
|
||||
if (!isAllowedToAutoHide() || !cancelCurrentAnimation()) return;
|
||||
resetInternalScrollState(scrollOffsetY, scrollExtentY);
|
||||
mLastScrollOffsetY = scrollOffsetY;
|
||||
|
@ -163,9 +163,20 @@ void GestureListenerManager::GestureEventAck(
|
||||
ScopedJavaLocalRef<jobject> j_obj = java_ref_.get(env);
|
||||
if (j_obj.is_null())
|
||||
return;
|
||||
if (event.GetType() == blink::WebInputEvent::Type::kGestureScrollBegin) {
|
||||
Java_GestureListenerManagerImpl_onScrollBegin(
|
||||
env, j_obj, /*isDirectionUp*/ event.data.scroll_begin.delta_y_hint > 0);
|
||||
return;
|
||||
}
|
||||
bool consumed = ack_result == blink::mojom::InputEventResultState::kConsumed;
|
||||
if (event.GetType() == blink::WebInputEvent::Type::kGestureFlingStart &&
|
||||
consumed) {
|
||||
Java_GestureListenerManagerImpl_onFlingStart(
|
||||
env, j_obj, /*isDirectionUp*/ event.data.scroll_begin.delta_y_hint > 0);
|
||||
return;
|
||||
}
|
||||
Java_GestureListenerManagerImpl_onEventAck(
|
||||
env, j_obj, static_cast<int>(event.GetType()),
|
||||
ack_result == blink::mojom::InputEventResultState::kConsumed);
|
||||
env, j_obj, static_cast<int>(event.GetType()), consumed);
|
||||
}
|
||||
|
||||
void GestureListenerManager::DidStopFlinging() {
|
||||
|
@ -220,26 +220,14 @@ public class GestureListenerManagerImpl
|
||||
private void onEventAck(int event, boolean consumed) {
|
||||
switch (event) {
|
||||
case EventType.GESTURE_FLING_START:
|
||||
if (consumed) {
|
||||
// The view expects the fling velocity in pixels/s.
|
||||
mHasActiveFlingScroll = true;
|
||||
for (mIterator.rewind(); mIterator.hasNext();) {
|
||||
mIterator.next().onFlingStartGesture(
|
||||
verticalScrollOffset(), verticalScrollExtent());
|
||||
}
|
||||
} else {
|
||||
// If a scroll ends with a fling, a SCROLL_END event is never sent.
|
||||
// However, if that fling went unconsumed, we still need to let the
|
||||
// listeners know that scrolling has ended.
|
||||
updateOnScrollEnd();
|
||||
}
|
||||
break;
|
||||
case EventType.GESTURE_SCROLL_BEGIN:
|
||||
setGestureScrollInProgress(true);
|
||||
for (mIterator.rewind(); mIterator.hasNext();) {
|
||||
mIterator.next().onScrollStarted(
|
||||
verticalScrollOffset(), verticalScrollExtent());
|
||||
}
|
||||
// If we're here, then |consumed| is false as otherwise #onFlingStart() would have
|
||||
// been called by native instead.
|
||||
assert !consumed;
|
||||
|
||||
// If a scroll ends with a fling, a SCROLL_END event is never sent.
|
||||
// However, if that fling went unconsumed, we still need to let the
|
||||
// listeners know that scrolling has ended.
|
||||
updateOnScrollEnd();
|
||||
break;
|
||||
case EventType.GESTURE_SCROLL_UPDATE:
|
||||
if (!consumed) break;
|
||||
@ -274,6 +262,31 @@ public class GestureListenerManagerImpl
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Called when a gesture event ack happens for |EventType.GESTURE_SCROLL_BEGIN|.
|
||||
*/
|
||||
@CalledByNative
|
||||
private void onScrollBegin(boolean isDirectionUp) {
|
||||
setGestureScrollInProgress(true);
|
||||
for (mIterator.rewind(); mIterator.hasNext();) {
|
||||
mIterator.next().onScrollStarted(
|
||||
verticalScrollOffset(), verticalScrollExtent(), isDirectionUp);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Called when a gesture event ack happens for |EventType.GESTURE_FLING_START|.
|
||||
*/
|
||||
@CalledByNative
|
||||
private void onFlingStart(boolean isDirectionUp) {
|
||||
// The view expects the fling velocity in pixels/s.
|
||||
mHasActiveFlingScroll = true;
|
||||
for (mIterator.rewind(); mIterator.hasNext();) {
|
||||
mIterator.next().onFlingStartGesture(
|
||||
verticalScrollOffset(), verticalScrollExtent(), isDirectionUp);
|
||||
}
|
||||
}
|
||||
|
||||
private void destroyPastePopup() {
|
||||
SelectionPopupControllerImpl controller = getSelectionPopupController();
|
||||
if (controller != null) controller.destroyPastePopup();
|
||||
|
@ -56,7 +56,8 @@ public class SelectPopupDropdown implements SelectPopup.Ui {
|
||||
});
|
||||
GestureListenerManager.fromWebContents(webContents).addListener(new GestureStateListener() {
|
||||
@Override
|
||||
public void onScrollStarted(int scrollOffsetY, int scrollExtentY) {
|
||||
public void onScrollStarted(
|
||||
int scrollOffsetY, int scrollExtentY, boolean isDirectionUp) {
|
||||
hide(true);
|
||||
}
|
||||
});
|
||||
|
@ -22,7 +22,7 @@ public abstract class GestureStateListener {
|
||||
/**
|
||||
* Called when a fling starts.
|
||||
*/
|
||||
public void onFlingStartGesture(int scrollOffsetY, int scrollExtentY) {}
|
||||
public void onFlingStartGesture(int scrollOffsetY, int scrollExtentY, boolean isDirectionUp) {}
|
||||
|
||||
/**
|
||||
* Called when a fling has ended.
|
||||
@ -40,7 +40,7 @@ public abstract class GestureStateListener {
|
||||
/**
|
||||
* Called when a scroll gesture has started.
|
||||
*/
|
||||
public void onScrollStarted(int scrollOffsetY, int scrollExtentY) {}
|
||||
public void onScrollStarted(int scrollOffsetY, int scrollExtentY, boolean isDirectionUp) {}
|
||||
|
||||
/**
|
||||
* Called when a scroll gesture has stopped.
|
||||
|
@ -51,7 +51,7 @@ public class GestureListenerManagerTest {
|
||||
private Integer mLastScrollOffsetY;
|
||||
|
||||
@Override
|
||||
public void onScrollStarted(int scrollOffsetY, int scrollExtentY) {
|
||||
public void onScrollStarted(int scrollOffsetY, int scrollExtentY, boolean isDirectionUp) {
|
||||
org.chromium.base.Log.e("chrome", "!!!onScrollStarted " + scrollOffsetY);
|
||||
mGotStarted = true;
|
||||
mLastScrollOffsetY = null;
|
||||
|
@ -64,7 +64,8 @@ public final class WebContentsGestureStateTracker {
|
||||
|
||||
mGestureListener = new GestureStateListener() {
|
||||
@Override
|
||||
public void onFlingStartGesture(int scrollOffsetY, int scrollExtentY) {
|
||||
public void onFlingStartGesture(
|
||||
int scrollOffsetY, int scrollExtentY, boolean isDirectionUp) {
|
||||
onScrollingStateChanged();
|
||||
}
|
||||
|
||||
@ -74,7 +75,8 @@ public final class WebContentsGestureStateTracker {
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onScrollStarted(int scrollOffsetY, int scrollExtentY) {
|
||||
public void onScrollStarted(
|
||||
int scrollOffsetY, int scrollExtentY, boolean isDirectionUp) {
|
||||
onScrollingStateChanged();
|
||||
}
|
||||
|
||||
|
Reference in New Issue
Block a user