[M123]Ensure we check readability for all URLs.
This change adds readability checks for redirects and onLoadStarted calls. Additionally, I added some per-entrypoint histograms tracking playback successes and failures and how often do we trigger playbacks without readability checks. I think some UI races are still possible, so this is most likely not the final fix, but definitely a step in the right direction
Bug: b/327453119
Bug: 328271376
(cherry picked from commit 99a85789670c89dfe9eeabbe71ad4008e4e81db0)
Change-Id: I43fe30f865a6230d137e2d89d785e25631627eaf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5341662
Reviewed-by: Ian Wells <iwells@chromium.org>
Reviewed-by: Calder Kitagawa <ckitagawa@chromium.org>
Commit-Queue: Basia Zimirska <basiaz@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1268687}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5356614
Auto-Submit: Basia Zimirska <basiaz@google.com>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/branch-heads/6312@{#558}
Cr-Branched-From: 6711dcdae48edaf98cbc6964f90fac85b7d9986e-refs/heads/main@{#1262506}
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/app/ChromeActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/app/ChromeActivity.java
index a0e1398..90bb2071 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/app/ChromeActivity.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/app/ChromeActivity.java
@@ -3171,7 +3171,7 @@
ReadAloudController readAloudController =
mRootUiCoordinator.getReadAloudControllerSupplier().get();
if (readAloudController != null) {
- readAloudController.playTab(currentTab);
+ readAloudController.playTab(currentTab, ReadAloudController.Entrypoint.OVERFLOW_MENU);
}
}
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/ChromeActivityUnitTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/ChromeActivityUnitTest.java
index 74d74ac..e6c3eabd 100644
--- a/chrome/android/junit/src/org/chromium/chrome/browser/ChromeActivityUnitTest.java
+++ b/chrome/android/junit/src/org/chromium/chrome/browser/ChromeActivityUnitTest.java
@@ -7,7 +7,6 @@
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.eq;
-import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -149,6 +148,7 @@
assertTrue(
chromeActivity.onMenuOrKeyboardAction(
R.id.readaloud_menu_id, /* fromMenu= */ true));
- verify(mReadAloudController, times(1)).playTab(eq(mActivityTab));
+ verify(mReadAloudController)
+ .playTab(eq(mActivityTab), eq(ReadAloudController.Entrypoint.OVERFLOW_MENU));
}
}
diff --git a/chrome/browser/readaloud/android/java/src/org/chromium/chrome/browser/readaloud/ReadAloudController.java b/chrome/browser/readaloud/android/java/src/org/chromium/chrome/browser/readaloud/ReadAloudController.java
index e52c3cb..1450f4e 100644
--- a/chrome/browser/readaloud/android/java/src/org/chromium/chrome/browser/readaloud/ReadAloudController.java
+++ b/chrome/browser/readaloud/android/java/src/org/chromium/chrome/browser/readaloud/ReadAloudController.java
@@ -11,6 +11,7 @@
import android.app.Activity;
import android.content.Intent;
+import androidx.annotation.IntDef;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;
@@ -53,6 +54,7 @@
import org.chromium.components.prefs.PrefService;
import org.chromium.components.user_prefs.UserPrefs;
import org.chromium.content_public.browser.GlobalRenderFrameHostId;
+import org.chromium.content_public.browser.NavigationHandle;
import org.chromium.content_public.browser.WebContents;
import org.chromium.ui.base.ActivityWindowAndroid;
import org.chromium.ui.base.WindowAndroid;
@@ -124,6 +126,21 @@
private boolean mOnUserLeaveHint;
+ /**
+ * ReadAloud entrypoint defined in readaloud/enums.xml.
+ *
+ * <p>Do not reorder or remove items, only add new items before NUM_ENTRIES.
+ */
+ @IntDef({Entrypoint.OVERFLOW_MENU, Entrypoint.MAGIC_TOOLBAR, Entrypoint.RESTORED_PLAYBACK})
+ public @interface Entrypoint {
+ int OVERFLOW_MENU = 0;
+ int MAGIC_TOOLBAR = 1;
+ int RESTORED_PLAYBACK = 2;
+
+ // Be sure to also update enums.xml when updating these values.
+ int NUM_ENTRIES = 3;
+ }
+
// Information about a tab playback necessary for resuming later. Does not
// include language or voice which should come from current tab state or
// settings respectively.
@@ -197,11 +214,10 @@
PlaybackData getPlaybackData() {
return mData;
}
-
/** Apply the saved playback state. */
void restore() {
maybeInitializePlaybackHooks();
- createTabPlayback(mTab, mDateModified)
+ createTabPlayback(mTab, mDateModified, Entrypoint.RESTORED_PLAYBACK)
.then(
playback -> {
if (mPlaying) {
@@ -359,6 +375,30 @@
ReadAloudMetrics.recordHighlightingEnabledOnStartup(mHighlightingEnabled.get());
mTabObserver =
new TabModelTabObserver(mTabModel) {
+
+ @Override
+ public void onLoadStarted(Tab tab, boolean toDifferentDocument) {
+ Log.d(TAG, "onLoadStarted");
+ if (tab != null
+ && tab.getUrl() != null
+ && tab.getUrl().isValid()
+ && toDifferentDocument) {
+ maybeCheckReadability(tab.getUrl());
+ maybeHandleTabReload(tab, tab.getUrl());
+ maybeStopPlayback(tab);
+ }
+ }
+
+ @Override
+ public void onDidRedirectNavigation(
+ Tab tab, NavigationHandle navigationHandle) {
+ Log.d(TAG, "onDidRedirectNavigation");
+ if (navigationHandle.getUrl() != null
+ && navigationHandle.getUrl().isValid()) {
+ maybeCheckReadability(navigationHandle.getUrl());
+ }
+ }
+
@Override
public void onPageLoadStarted(Tab tab, GURL url) {
Log.d(TAG, "onPageLoad called for %s", url.getPossiblyInvalidSpec());
@@ -550,21 +590,25 @@
*
* @param tab Tab to play.
*/
- public void playTab(Tab tab) {
+ public void playTab(Tab tab, @Entrypoint int entrypoint) {
+ if (!isReadable(tab)) {
+ ReadAloudMetrics.recordPlaybackWithoutReadabilityCheck(
+ entrypoint, Entrypoint.NUM_ENTRIES);
+ }
extractDateModified(tab)
.then(
timestamp -> {
ReadAloudMetrics.recordHasDateModified(true);
- playTabWithDateModified(tab, timestamp);
+ playTabWithDateModified(tab, timestamp, entrypoint);
},
exception -> {
ReadAloudMetrics.recordHasDateModified(false);
- playTabWithDateModified(tab, 0L);
+ playTabWithDateModified(tab, 0L, entrypoint);
});
}
- private void playTabWithDateModified(Tab tab, long dateModified) {
- createTabPlayback(tab, dateModified)
+ private void playTabWithDateModified(Tab tab, long dateModified, @Entrypoint int entrypoint) {
+ createTabPlayback(tab, dateModified, entrypoint)
.then(
playback -> {
mDateModified = dateModified;
@@ -597,7 +641,8 @@
}
}
- private Promise<Playback> createTabPlayback(Tab tab, long dateModified) {
+ private Promise<Playback> createTabPlayback(
+ Tab tab, long dateModified, @Entrypoint int entrypoint) {
assert tab.getUrl().isValid();
// only start a new playback if different URL or no active playback for that url
if (mCurrentlyPlayingTab != null && tab.getUrl().equals(mCurrentlyPlayingTab.getUrl())) {
@@ -627,7 +672,7 @@
var voices = mPlaybackHooks.getVoicesFor(playbackLanguage);
// TODO: Don't show entrypoints for unsupported languages
if (voices == null || voices.isEmpty()) {
- onCreatePlaybackFailed();
+ onCreatePlaybackFailed(entrypoint);
var promise = new Promise<Playback>();
promise.reject(new Exception("Unsupported language"));
return promise;
@@ -646,6 +691,7 @@
promise.then(
playback -> {
ReadAloudMetrics.recordIsTabPlaybackCreationSuccessful(true);
+ ReadAloudMetrics.recordTabCreationSuccess(entrypoint, Entrypoint.NUM_ENTRIES);
maybeSetUpHighlighter(playback.getMetadata());
updateVoiceMenu(
isTranslated
@@ -656,13 +702,14 @@
},
exception -> {
Log.e(TAG, exception.getMessage());
- onCreatePlaybackFailed();
+ onCreatePlaybackFailed(entrypoint);
});
return promise;
}
- private void onCreatePlaybackFailed() {
+ private void onCreatePlaybackFailed(@Entrypoint int entrypoint) {
ReadAloudMetrics.recordIsTabPlaybackCreationSuccessful(false);
+ ReadAloudMetrics.recordTabCreationFailure(entrypoint, Entrypoint.NUM_ENTRIES);
mPlayerCoordinator.playbackFailed();
}
diff --git a/chrome/browser/readaloud/android/java/src/org/chromium/chrome/browser/readaloud/ReadAloudControllerUnitTest.java b/chrome/browser/readaloud/android/java/src/org/chromium/chrome/browser/readaloud/ReadAloudControllerUnitTest.java
index 392bbe5..e5eb18d 100644
--- a/chrome/browser/readaloud/android/java/src/org/chromium/chrome/browser/readaloud/ReadAloudControllerUnitTest.java
+++ b/chrome/browser/readaloud/android/java/src/org/chromium/chrome/browser/readaloud/ReadAloudControllerUnitTest.java
@@ -90,6 +90,7 @@
import org.chromium.components.search_engines.TemplateUrlService;
import org.chromium.components.user_prefs.UserPrefsJni;
import org.chromium.content_public.browser.GlobalRenderFrameHostId;
+import org.chromium.content_public.browser.NavigationHandle;
import org.chromium.content_public.browser.RenderFrameHost;
import org.chromium.content_public.browser.WebContents;
import org.chromium.net.ConnectionType;
@@ -114,6 +115,7 @@
@DisableFeatures({ChromeFeatureList.READALOUD_IN_MULTI_WINDOW})
public class ReadAloudControllerUnitTest {
private static final GURL sTestGURL = JUnitTestGURLs.EXAMPLE_URL;
+ private static final GURL sTestRedirectGURL = JUnitTestGURLs.URL_1_WITH_PATH;
private static final long KNOWN_READABLE_TRIAL_PTR = 12345678L;
private MockTab mTab;
@@ -144,7 +146,7 @@
@Mock private TemplateUrlService mTemplateUrlService;
@Mock private ActivityWindowAndroid mActivityWindowAndroid;
@Mock private ActivityLifecycleDispatcher mActivityLifecycleDispatcher;
-
+ @Mock private NavigationHandle mNavigationHandle;
MockTabModelSelector mTabModelSelector;
@Captor ArgumentCaptor<ReadAloudReadabilityHooks.ReadabilityCallback> mCallbackCaptor;
@@ -217,7 +219,7 @@
mHighlightingEnabledOnStartupHistogram =
HistogramWatcher.newSingleRecordWatcher(
"ReadAloud.HighlightingEnabled.OnStartup", true);
-
+ when(mNavigationHandle.getUrl()).thenReturn(sTestRedirectGURL);
mController =
new ReadAloudController(
mActivity,
@@ -308,7 +310,7 @@
verify(mPlayback, never()).release();
// now start playing a tab
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks, times(1))
.createPlayback(Mockito.any(), mPlaybackCallbackCaptor.capture());
@@ -330,6 +332,52 @@
}
@Test
+ public void testOnLoadStarted_differentDocument() {
+ // start a successful playback
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
+ resolvePromises();
+ verify(mPlaybackHooks).createPlayback(Mockito.any(), mPlaybackCallbackCaptor.capture());
+ onPlaybackSuccess(mPlayback);
+ resolvePromises();
+
+ // Load new url
+ when(mTab.getUrl()).thenReturn(new GURL("https://en.wikipedia.org/wiki/Alphabet_Inc."));
+ mController.getTabModelTabObserverforTests().onLoadStarted(mTab, true);
+
+ verify(mHooksImpl).isPageReadable(eq(mTab.getUrl().getSpec()), any());
+ verify(mHighlighter).handleTabReloaded(eq(mTab));
+ verify(mPlayerCoordinator).dismissPlayers();
+ }
+
+ @Test
+ public void testOnLoadStarted_sameDocument() {
+ // start a successful playback
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
+ resolvePromises();
+ verify(mPlaybackHooks).createPlayback(Mockito.any(), mPlaybackCallbackCaptor.capture());
+ onPlaybackSuccess(mPlayback);
+ resolvePromises();
+
+ // Load the same document
+ mController.getTabModelTabObserverforTests().onLoadStarted(mTab, false);
+
+ // nothing should happen
+ verify(mHooksImpl, never()).isPageReadable(eq(mTab.getUrl().getSpec()), any());
+ verify(mHighlighter, never()).handleTabReloaded(eq(mTab));
+ verify(mPlayerCoordinator, never()).dismissPlayers();
+ }
+
+ @Test
+ public void testOnDidRedirectNavigation() {
+ mController
+ .getTabModelTabObserverforTests()
+ .onDidRedirectNavigation(mTab, mNavigationHandle);
+
+ // check readability for the redirect
+ verify(mHooksImpl).isPageReadable(eq(sTestRedirectGURL.getSpec()), any());
+ }
+
+ @Test
public void testOnActivityAttachmentChanged() {
// change tab attachment before any playback starts - tests null checks
mController
@@ -340,7 +388,7 @@
verify(mPlayback, never()).release();
// now start playing a tab
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks, times(1))
@@ -369,7 +417,7 @@
@Test
public void testOnActivityAttachmentChanged_saveAndRestoreState() {
// start playing a tab
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks, times(1))
@@ -415,7 +463,7 @@
@Test
public void testReloadPage_errorUiDismissed() {
// start a playback with an error
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks, times(1))
.createPlayback(Mockito.any(), mPlaybackCallbackCaptor.capture());
@@ -438,7 +486,7 @@
verify(mPlayback, never()).release();
// now start playing a tab
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks, times(1))
.createPlayback(Mockito.any(), mPlaybackCallbackCaptor.capture());
@@ -462,7 +510,7 @@
@Test
public void testClosingTab_errorUiDismissed() {
// start a playback with an error
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks, times(1))
.createPlayback(Mockito.any(), mPlaybackCallbackCaptor.capture());
@@ -618,7 +666,7 @@
public void testPlayTab() {
mFakeTranslateBridge.setCurrentLanguage("en");
mTab.setGurlOverrideForTesting(new GURL("https://en.wikipedia.org/wiki/Google"));
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks, times(1))
@@ -633,7 +681,7 @@
MockTab newTab = mTabModelSelector.addMockTab();
newTab.setGurlOverrideForTesting(new GURL("https://en.wikipedia.org/wiki/Alphabet_Inc."));
newTab.setWebContentsOverrideForTesting(mWebContents);
- mController.playTab(newTab);
+ mController.playTab(newTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlayback, times(1)).release();
}
@@ -642,7 +690,7 @@
public void testPlayTab_inMultiWindow() {
mFakeTranslateBridge.setCurrentLanguage("en");
mTab.setGurlOverrideForTesting(new GURL("https://en.wikipedia.org/wiki/Google"));
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks)
@@ -660,7 +708,7 @@
public void testPlayTab_inMultiWindow_flag() {
mFakeTranslateBridge.setCurrentLanguage("en");
mTab.setGurlOverrideForTesting(new GURL("https://en.wikipedia.org/wiki/Google"));
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks)
@@ -687,7 +735,7 @@
.getPlaybackVoiceList(any());
mTab.setGurlOverrideForTesting(new GURL("https://en.wikipedia.org/wiki/Google"));
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks, times(1)).initVoices();
@@ -712,7 +760,7 @@
mFakeTranslateBridge.setCurrentLanguage("");
mTab.setGurlOverrideForTesting(new GURL("https://en.wikipedia.org/wiki/Google"));
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks).createPlayback(mPlaybackArgsCaptor.capture(), any());
@@ -725,7 +773,7 @@
mFakeTranslateBridge.setCurrentLanguage("pl-PL");
mTab.setGurlOverrideForTesting(new GURL("https://en.wikipedia.org/wiki/Google"));
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks, never()).createPlayback(mPlaybackArgsCaptor.capture(), any());
@@ -740,7 +788,7 @@
mFakeTranslateBridge.setCurrentLanguage("und");
mTab.setGurlOverrideForTesting(new GURL("https://en.wikipedia.org/wiki/Google"));
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks).createPlayback(mPlaybackArgsCaptor.capture(), any());
@@ -755,7 +803,7 @@
mFakeTranslateBridge.setCurrentLanguage("fr");
mTab.setGurlOverrideForTesting(new GURL("https://en.wikipedia.org/wiki/Google"));
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks).createPlayback(mPlaybackArgsCaptor.capture(), any());
@@ -775,7 +823,7 @@
mFakeTranslateBridge.setCurrentLanguage("fr");
mTab.setGurlOverrideForTesting(new GURL("https://en.wikipedia.org/wiki/Google"));
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks)
@@ -800,7 +848,7 @@
mFakeTranslateBridge.setCurrentLanguage("fr");
mTab.setGurlOverrideForTesting(new GURL("https://en.wikipedia.org/wiki/Google"));
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks)
@@ -826,7 +874,7 @@
mFakeTranslateBridge.setCurrentLanguage("fr");
mTab.setGurlOverrideForTesting(new GURL("https://en.wikipedia.org/wiki/Google"));
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks)
@@ -841,7 +889,7 @@
public void testPlayTab_onFailure() {
mFakeTranslateBridge.setCurrentLanguage("en");
mTab.setGurlOverrideForTesting(new GURL("https://en.wikipedia.org/wiki/Google"));
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks, times(1))
@@ -857,7 +905,7 @@
// Play tab
mFakeTranslateBridge.setCurrentLanguage("en");
mTab.setGurlOverrideForTesting(new GURL("https://en.wikipedia.org/wiki/Google"));
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks, times(1))
@@ -879,7 +927,7 @@
.when(mPlaybackHooks)
.getVoicesFor(anyString());
// Subsequent playTab() should play without trying to release anything.
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks).createPlayback(any(), any());
verify(mPlayback, never()).release();
@@ -889,7 +937,7 @@
public void highlightsRequested() {
// set up the highlighter
mController.setTimepointsSupportedForTest(mTab.getUrl().getSpec(), true);
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks, times(1))
.createPlayback(Mockito.any(), mPlaybackCallbackCaptor.capture());
@@ -919,7 +967,7 @@
public void reloadingTab_highlightsCleared() {
// set up the highlighter
mController.setTimepointsSupportedForTest(mTab.getUrl().getSpec(), true);
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks, times(1))
.createPlayback(Mockito.any(), mPlaybackCallbackCaptor.capture());
@@ -936,7 +984,7 @@
public void reloadingTab_highlightsNotCleared() {
// set up the highlighter
mController.setTimepointsSupportedForTest(mTab.getUrl().getSpec(), true);
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks, times(1))
.createPlayback(Mockito.any(), mPlaybackCallbackCaptor.capture());
@@ -955,7 +1003,7 @@
public void stoppingPlaybackClearsHighlighter() {
// set up the highlighter
mController.setTimepointsSupportedForTest(mTab.getUrl().getSpec(), true);
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks, times(1))
.createPlayback(Mockito.any(), mPlaybackCallbackCaptor.capture());
@@ -992,7 +1040,7 @@
verify(mHighlighter, never()).handleTabReloaded(mTab);
mController.setTimepointsSupportedForTest(mTab.getUrl().getSpec(), true);
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks, times(1))
.createPlayback(Mockito.any(), mPlaybackCallbackCaptor.capture());
@@ -1018,7 +1066,7 @@
// First play tab.
mFakeTranslateBridge.setCurrentLanguage("en");
mTab.setGurlOverrideForTesting(new GURL("https://en.wikipedia.org/wiki/Google"));
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
// Verify the original voice list.
@@ -1063,7 +1111,7 @@
@Test
public void testSetVoiceWhilePaused() {
// Play tab.
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks).createPlayback(any(), mPlaybackCallbackCaptor.capture());
onPlaybackSuccess(mPlayback);
@@ -1269,7 +1317,7 @@
@Test
public void testRestorePlaybackState_whileLoading() {
// Request playback but don't succeed yet.
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks).createPlayback(any(), mPlaybackCallbackCaptor.capture());
reset(mPlaybackHooks);
@@ -1422,7 +1470,7 @@
public void testIsHighlightingSupported_pageTranslated() {
mFakeTranslateBridge.setIsPageTranslated(true);
mController.setTimepointsSupportedForTest(mTab.getUrl().getSpec(), true);
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
assertFalse(mController.isHighlightingSupported());
@@ -1432,7 +1480,7 @@
public void testIsHighlightingSupported_notSupported() {
mFakeTranslateBridge.setIsPageTranslated(false);
mController.setTimepointsSupportedForTest(mTab.getUrl().getSpec(), false);
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
assertFalse(mController.isHighlightingSupported());
@@ -1442,7 +1490,7 @@
public void testIsHighlightingSupported_supported() {
mFakeTranslateBridge.setIsPageTranslated(false);
mController.setTimepointsSupportedForTest(mTab.getUrl().getSpec(), true);
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
assertTrue(mController.isHighlightingSupported());
@@ -1679,10 +1727,9 @@
final String histogramName = ReadAloudMetrics.IS_TAB_PLAYBACK_CREATION_SUCCESSFUL;
var histogram = HistogramWatcher.newSingleRecordWatcher(histogramName, true);
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
- verify(mPlaybackHooks, times(1))
- .createPlayback(Mockito.any(), mPlaybackCallbackCaptor.capture());
+ verify(mPlaybackHooks).createPlayback(Mockito.any(), mPlaybackCallbackCaptor.capture());
onPlaybackSuccess(mPlayback);
histogram.assertExpected();
}
@@ -1692,12 +1739,56 @@
final String histogramName = ReadAloudMetrics.IS_TAB_PLAYBACK_CREATION_SUCCESSFUL;
var histogram = HistogramWatcher.newSingleRecordWatcher(histogramName, false);
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
+ resolvePromises();
+ verify(mPlaybackHooks).createPlayback(Mockito.any(), mPlaybackCallbackCaptor.capture());
+ mPlaybackCallbackCaptor.getValue().onFailure(new Exception("Very bad error"));
+ resolvePromises();
+ histogram.assertExpected();
+ }
+
+ @Test
+ public void testMetricRecorded_playbackWithoutReadabilityCheck() {
+ final String histogramName = ReadAloudMetrics.TAB_PLAYBACK_WITHOUT_READABILITY_CHECK_ERROR;
+ var histogram =
+ HistogramWatcher.newSingleRecordWatcher(
+ histogramName, ReadAloudController.Entrypoint.OVERFLOW_MENU);
+
+ mController.playTab(mTab, ReadAloudController.Entrypoint.OVERFLOW_MENU);
+ histogram.assertExpected();
+ }
+
+ @Test
+ public void testMetricRecorded_playbackSuccess() {
+ final String histogramName = ReadAloudMetrics.TAB_PLAYBACK_CREATION_SUCCESS;
+ var histogram =
+ HistogramWatcher.newSingleRecordWatcher(
+ histogramName, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
+
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
+ resolvePromises();
+ verify(mPlaybackHooks, times(1))
+ .createPlayback(Mockito.any(), mPlaybackCallbackCaptor.capture());
+
+ onPlaybackSuccess(mPlayback);
+
+ histogram.assertExpected();
+ }
+
+ @Test
+ public void testMetricRecorded_playbackFailure() {
+ final String histogramName = ReadAloudMetrics.TAB_PLAYBACK_CREATION_FAILURE;
+ var histogram =
+ HistogramWatcher.newSingleRecordWatcher(
+ histogramName, ReadAloudController.Entrypoint.OVERFLOW_MENU);
+ // Play tab to set up playbackhooks
+ mController.playTab(mTab, ReadAloudController.Entrypoint.OVERFLOW_MENU);
resolvePromises();
verify(mPlaybackHooks, times(1))
.createPlayback(Mockito.any(), mPlaybackCallbackCaptor.capture());
mPlaybackCallbackCaptor.getValue().onFailure(new Exception("Very bad error"));
resolvePromises();
+
histogram.assertExpected();
}
@@ -1707,7 +1798,7 @@
var histogram = HistogramWatcher.newBuilder().expectNoRecords(histogramName).build();
// Play tab to set up playbackhooks
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
// Preview a voice.
@@ -1736,7 +1827,7 @@
final String histogramName = "ReadAloud.HighlightingSupported";
var histogram = HistogramWatcher.newSingleRecordWatcher(histogramName, true);
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks, times(1))
@@ -1754,7 +1845,7 @@
final String histogramName = "ReadAloud.HighlightingSupported";
var histogram = HistogramWatcher.newSingleRecordWatcher(histogramName, false);
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks, times(1))
@@ -1772,7 +1863,7 @@
// Play tab.
mFakeTranslateBridge.setCurrentLanguage("en");
mTab.setGurlOverrideForTesting(new GURL("https://en.wikipedia.org/wiki/Google"));
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks).createPlayback(any(), mPlaybackCallbackCaptor.capture());
onPlaybackSuccess(mPlayback);
@@ -1857,7 +1948,7 @@
// Play tab
mFakeTranslateBridge.setCurrentLanguage("en");
mTab.setGurlOverrideForTesting(new GURL("https://en.wikipedia.org/wiki/Google"));
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks).createPlayback(any(), mPlaybackCallbackCaptor.capture());
onPlaybackSuccess(mPlayback);
@@ -1959,7 +2050,7 @@
// Try playing the tab.
mFakeTranslateBridge.setCurrentLanguage("en");
mTab.setGurlOverrideForTesting(new GURL("https://en.wikipedia.org/wiki/Google"));
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
// No playback request should be made.
verify(mPlaybackHooks, never()).createPlayback(any(), any());
@@ -2030,7 +2121,7 @@
mFakeTranslateBridge.setCurrentLanguage("en");
mTab.setGurlOverrideForTesting(new GURL("https://en.wikipedia.org/wiki/Google"));
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
@@ -2052,7 +2143,7 @@
mFakeTranslateBridge.setCurrentLanguage("en");
var histogram = HistogramWatcher.newSingleRecordWatcher("ReadAloud.HasDateModified", true);
mTab.setGurlOverrideForTesting(new GURL("https://en.wikipedia.org/wiki/Google"));
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
histogram.assertExpected();
@@ -2067,7 +2158,7 @@
var histogram = HistogramWatcher.newSingleRecordWatcher("ReadAloud.HasDateModified", false);
mTab.setGurlOverrideForTesting(new GURL("https://en.wikipedia.org/wiki/Google"));
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
histogram.assertExpected();
@@ -2078,7 +2169,7 @@
// play tab
mFakeTranslateBridge.setCurrentLanguage("en");
mTab.setGurlOverrideForTesting(new GURL("https://en.wikipedia.org/wiki/Google"));
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks, times(1))
.createPlayback(Mockito.any(), mPlaybackCallbackCaptor.capture());
@@ -2088,7 +2179,7 @@
private void requestAndStartPlayback() {
mFakeTranslateBridge.setCurrentLanguage("en");
mTab.setGurlOverrideForTesting(new GURL("https://en.wikipedia.org/wiki/Google"));
- mController.playTab(mTab);
+ mController.playTab(mTab, ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
resolvePromises();
verify(mPlaybackHooks, times(1))
diff --git a/chrome/browser/readaloud/android/java/src/org/chromium/chrome/browser/readaloud/ReadAloudMetrics.java b/chrome/browser/readaloud/android/java/src/org/chromium/chrome/browser/readaloud/ReadAloudMetrics.java
index cfd5d90..25c9fdb 100644
--- a/chrome/browser/readaloud/android/java/src/org/chromium/chrome/browser/readaloud/ReadAloudMetrics.java
+++ b/chrome/browser/readaloud/android/java/src/org/chromium/chrome/browser/readaloud/ReadAloudMetrics.java
@@ -20,6 +20,11 @@
public static String IS_TAB_PLAYBACK_CREATION_SUCCESSFUL =
"ReadAloud.IsTabPlaybackCreationSuccessful";
public static String HAS_TAP_TO_SEEK_FOUND_MATCH = "ReadAloud.HasTapToSeekFoundMatch";
+
+ public static String TAB_PLAYBACK_CREATION_SUCCESS = "ReadAloud.TabPlaybackCreationSuccess";
+ public static String TAB_PLAYBACK_CREATION_FAILURE = "ReadAloud.TabPlaybackCreationFailure";
+ public static String TAB_PLAYBACK_WITHOUT_READABILITY_CHECK_ERROR =
+ "ReadAloud.ReadAloudPlaybackWithoutReadabilityCheckError";
public static String VOICE_CHANGED = "ReadAloud.VoiceChanged.";
public static String VOICE_PREVIEWED = "ReadAloud.VoicePreviewed.";
public static String TIME_SPENT_LISTENING = "ReadAloud.DurationListened";
@@ -144,6 +149,21 @@
RecordHistogram.recordBooleanHistogram(HAS_TAP_TO_SEEK_FOUND_MATCH, matchFound);
}
+ public static void recordTabCreationSuccess(int entrypoint, int maxVal) {
+ RecordHistogram.recordEnumeratedHistogram(
+ TAB_PLAYBACK_CREATION_SUCCESS, entrypoint, maxVal);
+ }
+
+ public static void recordTabCreationFailure(int entrypoint, int maxVal) {
+ RecordHistogram.recordEnumeratedHistogram(
+ TAB_PLAYBACK_CREATION_FAILURE, entrypoint, maxVal);
+ }
+
+ public static void recordPlaybackWithoutReadabilityCheck(int entrypoint, int maxVal) {
+ RecordHistogram.recordEnumeratedHistogram(
+ TAB_PLAYBACK_WITHOUT_READABILITY_CHECK_ERROR, entrypoint, maxVal);
+ }
+
public static void recordSpeedChange(float speed) {
for (int i = 0; i < sPlaybackSpeeds.length; i++) {
if (speed == sPlaybackSpeeds[i]) {
diff --git a/chrome/browser/readaloud/android/java/src/org/chromium/chrome/browser/readaloud/ReadAloudToolbarButtonController.java b/chrome/browser/readaloud/android/java/src/org/chromium/chrome/browser/readaloud/ReadAloudToolbarButtonController.java
index 24cabd4..da6ddfe 100644
--- a/chrome/browser/readaloud/android/java/src/org/chromium/chrome/browser/readaloud/ReadAloudToolbarButtonController.java
+++ b/chrome/browser/readaloud/android/java/src/org/chromium/chrome/browser/readaloud/ReadAloudToolbarButtonController.java
@@ -67,7 +67,9 @@
}
RecordUserAction.record("MobileTopToolbarReadAloudButton");
- mControllerSupplier.get().playTab(mActiveTabSupplier.get());
+ mControllerSupplier
+ .get()
+ .playTab(mActiveTabSupplier.get(), ReadAloudController.Entrypoint.MAGIC_TOOLBAR);
}
@Override
@@ -81,6 +83,7 @@
@Override
protected boolean shouldShowButton(Tab tab) {
+
if (!super.shouldShowButton(tab) || tab == null || mControllerSupplier.get() == null) {
return false;
}
diff --git a/tools/metrics/histograms/metadata/readaloud/enums.xml b/tools/metrics/histograms/metadata/readaloud/enums.xml
index 7c8977d..7ace877 100644
--- a/tools/metrics/histograms/metadata/readaloud/enums.xml
+++ b/tools/metrics/histograms/metadata/readaloud/enums.xml
@@ -26,6 +26,12 @@
<enums>
+<enum name="ReadAloudEntrypoint">
+ <int value="0" label="OVERFLOW_MENU"/>
+ <int value="1" label="MAGIC_TOOLBAR"/>
+ <int value="2" label="RESTORED_PLAYBACK"/>
+</enum>
+
<enum name="ReadAloudErrorCode">
<int value="0" label="Message successfully sent"/>
<int value="1" label="CANCELLED"/>
diff --git a/tools/metrics/histograms/metadata/readaloud/histograms.xml b/tools/metrics/histograms/metadata/readaloud/histograms.xml
index bb1e9da..116e2db 100644
--- a/tools/metrics/histograms/metadata/readaloud/histograms.xml
+++ b/tools/metrics/histograms/metadata/readaloud/histograms.xml
@@ -202,6 +202,18 @@
<token key="Phase" variants="Phase"/>
</histogram>
+<histogram name="ReadAloud.ReadAloudPlaybackWithoutReadabilityCheckError"
+ enum="ReadAloudEntrypoint" expires_after="2024-08-04">
+ <owner>andreaxg@google.com</owner>
+ <owner>basiaz@google.com</owner>
+ <owner>iwells@chromium.org</owner>
+ <summary>
+ Android only. Records entrypoint from which a request to play a tab was
+ executed without checking for page readability readability first. Recorded
+ for each playback request without prior readability check.
+ </summary>
+</histogram>
+
<histogram name="ReadAloud.ReadAloudUnsuportedError.{Phase}"
enum="RejectionReason" expires_after="2024-08-04">
<owner>andreaxg@google.com</owner>
@@ -240,6 +252,30 @@
</summary>
</histogram>
+<histogram name="ReadAloud.TabPlaybackCreationFailure"
+ enum="ReadAloudEntrypoint" expires_after="2024-08-04">
+ <owner>andreaxg@google.com</owner>
+ <owner>basiaz@google.com</owner>
+ <owner>iwells@chromium.org</owner>
+ <summary>
+ Android only. Records when a a request to create playback from given
+ entrypoint fails. Emitted in the tab playback creation callback. Does not
+ mean playback has started.
+ </summary>
+</histogram>
+
+<histogram name="ReadAloud.TabPlaybackCreationSuccess"
+ enum="ReadAloudEntrypoint" expires_after="2024-08-04">
+ <owner>andreaxg@google.com</owner>
+ <owner>basiaz@google.com</owner>
+ <owner>iwells@chromium.org</owner>
+ <summary>
+ Android only. Records when a a request to create playback from given
+ entrypoint succeeds. Emitted in the tab playback creation callback. Does not
+ mean playback has started.
+ </summary>
+</histogram>
+
<histogram name="ReadAloud.VoiceChanged.{VoiceID}" enum="Boolean"
expires_after="2024-06-02">
<owner>andreaxg@google.com</owner>