Skip to content

Commit

Permalink
[M115] Check if TextBubble has been correctly displayed
Browse files Browse the repository at this point in the history
TextBubble may not be displayed for various reasons:
1. it has already been showing
2. it does reach the minimize size requirement of AnchorPopupWindow
3. the anchor view is not attached to window

This CL adds an assertion to check that and skips recording duration
 metrics if the text bubble has been displayed.

(cherry picked from commit f732ca5)

Bug: 1448335
Change-Id: I767c851b2203986cf97b40a34cfd6242a1cc9e5a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4563505
Commit-Queue: Lijin Shen <lazzzis@google.com>
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Peter Williamson <petewil@chromium.org>
Reviewed-by: Theresa Sullivan <twellington@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1148835}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4582407
Cr-Commit-Position: refs/branch-heads/5790@{#236}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
Lijin Shen authored and Chromium LUCI CQ committed Jun 1, 2023
1 parent cfd6e01 commit 1b30156
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 3 deletions.
Expand Up @@ -50,6 +50,7 @@
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.ui.appmenu.AppMenuHandler;
import org.chromium.chrome.browser.ui.messages.snackbar.SnackbarManager;
import org.chromium.components.browser_ui.widget.textbubble.TextBubble;
import org.chromium.components.feature_engagement.FeatureConstants;
import org.chromium.components.feature_engagement.Tracker;
import org.chromium.components.feature_engagement.TriggerDetails;
Expand Down Expand Up @@ -174,10 +175,13 @@ private void resetWebFeedFollowIntroController() {
mTabSupplier, new View(mActivity), mFeedLauncher, mDialogManager, mSnackbarManager);
mEmptyTabObserver = mWebFeedFollowIntroController.getEmptyTabObserverForTesting();
mWebFeedFollowIntroController.setClockForTesting(mClock);
// TextBubble is impossible to show in a junit.
TextBubble.setSkipShowCheckForTesting(true);
}

@After
public void tearDown() {
TextBubble.setSkipShowCheckForTesting(false);
TrackerFactory.setTrackerForTests(null);
}

Expand Down
Expand Up @@ -22,6 +22,7 @@
import androidx.annotation.DrawableRes;
import androidx.annotation.Nullable;
import androidx.annotation.StringRes;
import androidx.annotation.VisibleForTesting;
import androidx.appcompat.content.res.AppCompatResources;

import org.chromium.base.ApiCompatibilityUtils;
Expand Down Expand Up @@ -58,6 +59,9 @@ public class TextBubble implements AnchoredPopupWindow.LayoutObserver {
private static final ObservableSupplierImpl<Integer> sCountSupplier =
new ObservableSupplierImpl<>();

/** Disable assert error if it fails to be displayed. */
private static boolean sSkipShowCheckForTesting;

protected final Context mContext;
private final Handler mHandler;
private final boolean mInverseColor;
Expand Down Expand Up @@ -352,6 +356,9 @@ public void show() {
}

mPopupWindow.show();
assert sSkipShowCheckForTesting || mPopupWindow.isShowing() : "TextBubble is not presented";
if (!mPopupWindow.isShowing()) return;

sBubbles.add(this);
sCountSupplier.set(sBubbles.size());
mBubbleShowStartTime = System.currentTimeMillis();
Expand All @@ -362,13 +369,13 @@ public void show() {
* @see PopupWindow#dismiss()
*/
public void dismiss() {
mPopupWindow.dismiss();

if (mBubbleShowStartTime != 0) {
if (mPopupWindow.isShowing() && mBubbleShowStartTime != 0) {
RecordHistogram.recordTimesHistogram("InProductHelp.TextBubble.ShownTime",
System.currentTimeMillis() - mBubbleShowStartTime);
mBubbleShowStartTime = 0;
}

mPopupWindow.dismiss();
}

/**
Expand Down Expand Up @@ -563,4 +570,9 @@ public static Set<TextBubble> getTextBubbleSetForTesting() {
public View getTextBubbleContentViewForTesting() {
return mContentView;
}

@VisibleForTesting
public static void setSkipShowCheckForTesting(boolean skip) {
sSkipShowCheckForTesting = skip;
}
}

0 comments on commit 1b30156

Please sign in to comment.