[gbb-cct] Logging Dismissal in Page Insights Hub with the right conditions (for both peek and expanded mode)
Bug: 326394951
Bug: b/326395321
(cherry picked from commit 3513b987b4a5e180e3726ded018935ba6d293224)
Change-Id: Ie110d6c7497e065c234270bf9787e25dfe19d7df
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5317638
Reviewed-by: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: Edmund Wright <edmundw@google.com>
Commit-Queue: Kamal Choudhury <kamalchoudhury@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1265243}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5328518
Auto-Submit: Edmund Wright <edmundw@google.com>
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Commit-Position: refs/branch-heads/6312@{#255}
Cr-Branched-From: 6711dcdae48edaf98cbc6964f90fac85b7d9986e-refs/heads/main@{#1262506}
diff --git a/chrome/browser/ui/android/page_insights/java/src/org/chromium/chrome/browser/page_insights/PageInsightsMediator.java b/chrome/browser/ui/android/page_insights/java/src/org/chromium/chrome/browser/page_insights/PageInsightsMediator.java
index ec43ef31..cbc5995 100644
--- a/chrome/browser/ui/android/page_insights/java/src/org/chromium/chrome/browser/page_insights/PageInsightsMediator.java
+++ b/chrome/browser/ui/android/page_insights/java/src/org/chromium/chrome/browser/page_insights/PageInsightsMediator.java
@@ -66,7 +66,9 @@
import org.chromium.ui.util.ColorUtils;
import org.chromium.url.GURL;
+import java.util.Arrays;
import java.util.HashMap;
+import java.util.List;
import java.util.function.BooleanSupplier;
/**
@@ -90,6 +92,11 @@
"page_insights_can_autotrigger_while_in_motion";
static final String PAGE_INSIGHTS_CAN_RETURN_TO_PEEK_AFTER_EXPANSION =
"page_insights_can_return_to_peek_after_expansion";
+ private static final List<Integer> USER_DISMISSAL_REASONS =
+ Arrays.asList(
+ StateChangeReason.SWIPE,
+ StateChangeReason.BACK_PRESS,
+ StateChangeReason.TAP_SCRIM);
private final PageInsightsSheetContent mSheetContent;
private final ManagedBottomSheetController mSheetController;
@@ -411,7 +418,7 @@
mSheetContent.showFeedPage();
mIsShowingChildView = false;
} else if (!mSheetController.collapseSheet(true)) {
- mSheetController.hideContent(mSheetContent, true);
+ mSheetController.hideContent(mSheetContent, true, StateChangeReason.BACK_PRESS);
}
return true;
}
@@ -674,7 +681,7 @@
mWillHandleBackPressSupplier.set(false);
if (mResizeInSync) mSheetInset.set(0);
setBottomControlsHeight(mSheetController.getCurrentOffset());
- handleDismissal(mOldPihState);
+ handleDismissal(mOldPihState, reason);
} else if (newPihState == PageInsightsSheetState.PEEK) {
mWillHandleBackPressSupplier.set(false);
if (mResizeInSync) mSheetInset.set(0);
@@ -711,7 +718,8 @@
}
}
- private void handleDismissal(@PageInsightsSheetState int oldState) {
+ private void handleDismissal(
+ @PageInsightsSheetState int oldState, @StateChangeReason int reason) {
mIsShowingChildView = false;
if (mCurrentFeedView != null) {
@@ -723,13 +731,14 @@
mCurrentChildView = null;
}
- if (oldState == PageInsightsSheetState.PEEK) {
- logPageInsightsEvent(PageInsightsEvent.DISMISS_PEEK);
- getSurfaceRenderer().onEvent(DISMISSED_FROM_PEEKING_STATE);
- } else if (oldState == PageInsightsSheetState.EXPANDED) {
- logPageInsightsEvent(PageInsightsEvent.DISMISS_EXPANDED);
+ if (USER_DISMISSAL_REASONS.contains(reason)) {
+ if (oldState == PageInsightsSheetState.PEEK) {
+ logPageInsightsEvent(PageInsightsEvent.DISMISS_PEEK);
+ getSurfaceRenderer().onEvent(DISMISSED_FROM_PEEKING_STATE);
+ } else if (oldState == PageInsightsSheetState.EXPANDED) {
+ logPageInsightsEvent(PageInsightsEvent.DISMISS_EXPANDED);
+ }
}
-
getSurfaceRenderer().onSurfaceClosed();
}
diff --git a/chrome/browser/ui/android/page_insights/java/src/org/chromium/chrome/browser/page_insights/PageInsightsMediatorTest.java b/chrome/browser/ui/android/page_insights/java/src/org/chromium/chrome/browser/page_insights/PageInsightsMediatorTest.java
index 4022f3b..16b2673 100644
--- a/chrome/browser/ui/android/page_insights/java/src/org/chromium/chrome/browser/page_insights/PageInsightsMediatorTest.java
+++ b/chrome/browser/ui/android/page_insights/java/src/org/chromium/chrome/browser/page_insights/PageInsightsMediatorTest.java
@@ -1200,7 +1200,10 @@
assertFalse(handled);
verify(mBottomSheetController, never())
- .hideContent(eq(mMediator.getSheetContent()), anyBoolean());
+ .hideContent(
+ eq(mMediator.getSheetContent()),
+ anyBoolean(),
+ eq(StateChangeReason.BACK_PRESS));
}
@Test
@@ -1216,7 +1219,10 @@
assertFalse(handled);
verify(mBottomSheetController, never())
- .hideContent(eq(mMediator.getSheetContent()), anyBoolean());
+ .hideContent(
+ eq(mMediator.getSheetContent()),
+ anyBoolean(),
+ eq(StateChangeReason.BACK_PRESS));
}
@Test
@@ -1233,7 +1239,8 @@
assertTrue(handled);
verify(mBottomSheetController).collapseSheet(true);
- verify(mBottomSheetController, never()).hideContent(any(), anyBoolean());
+ verify(mBottomSheetController, never())
+ .hideContent(any(), anyBoolean(), eq(StateChangeReason.BACK_PRESS));
}
@Test
@@ -1249,7 +1256,8 @@
boolean handled = mMediator.getSheetContent().handleBackPress();
assertTrue(handled);
- verify(mBottomSheetController).hideContent(mMediator.getSheetContent(), true);
+ verify(mBottomSheetController)
+ .hideContent(mMediator.getSheetContent(), true, StateChangeReason.BACK_PRESS);
}
@Test
@@ -1283,7 +1291,10 @@
.findViewById(R.id.page_insights_feed_header)
.getVisibility());
verify(mBottomSheetController, never())
- .hideContent(eq(mMediator.getSheetContent()), anyBoolean());
+ .hideContent(
+ eq(mMediator.getSheetContent()),
+ anyBoolean(),
+ eq(StateChangeReason.BACK_PRESS));
}
@Test