Skip to content

Commit

Permalink
M103: Feed: add Start-specific logging for feed launch cancel cases
Browse files Browse the repository at this point in the history 
* Voice search
  - FeedReliabilityLogger is plumbed down into TasksSurfaceMediator for this.
* Omnibox focused
  - FeedReliabilityLogger is added as an omnibox focus change listener for this.
* Navigate away
  - We detect this when the user does any navigation from Start Surface (except for feed card taps and background tabs).
* Navigate back
  - Detected in onBackPressed() just before mController handles the back press.

(cherry picked from commit 74a2a5b)

Bug: 1195940,1326142
Change-Id: Idb015603fd766baab5c7f978b52aa702b83824f6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3618351
Reviewed-by: Xi Han <hanxi@chromium.org>
Commit-Queue: Ian Wells <iwells@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1003407}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3650953
Auto-Submit: Ian Wells <iwells@chromium.org>
Commit-Queue: Xi Han <hanxi@chromium.org>
Cr-Commit-Position: refs/branch-heads/5060@{#56}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
  • Loading branch information
Ian Wells authored and Chromium LUCI CQ committed May 17, 2022
1 parent 1e48562 commit 5390ada
Show file tree
Hide file tree
Showing 8 changed files with 163 additions and 29 deletions.
5 changes: 5 additions & 0 deletions ...urface/java/src/org/chromium/chrome/features/start_surface/ExploreSurfaceCoordinator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.chromium.chrome.R;
import org.chromium.chrome.browser.app.feed.FeedActionDelegateImpl;
import org.chromium.chrome.browser.bookmarks.BookmarkBridge;
import org.chromium.chrome.browser.feed.FeedReliabilityLogger;
import org.chromium.chrome.browser.feed.FeedSurfaceCoordinator;
import org.chromium.chrome.browser.feed.FeedSurfaceDelegate;
import org.chromium.chrome.browser.feed.FeedSurfaceLifecycleManager;
Expand Down Expand Up @@ -128,6 +129,10 @@ public void enableSwipeRefresh(boolean isVisible) {
mFeedSurfaceCoordinator.enableSwipeRefresh(isVisible);
}

public FeedReliabilityLogger getFeedReliabilityLogger() {
return mFeedSurfaceCoordinator.getReliabilityLogger();
}

private class ExploreSurfaceActionDelegate extends FeedActionDelegateImpl {
ExploreSurfaceActionDelegate(
SnackbarManager snackbarManager, BookmarkBridge bookmarkBridge) {
Expand Down
7 changes: 4 additions & 3 deletions ..._surface/java/src/org/chromium/chrome/features/start_surface/StartSurfaceCoordinator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,8 @@ mParentTabSupplier, new ScrollableContainerDelegateImpl(), mSnackbarManager,
mDynamicResourceLoaderSupplier.get(), mSnackbarManager, mModalDialogManager);
}
if (mTasksSurface != null) {
mTasksSurface.onFinishNativeInitialization(mActivity, mOmniboxStubSupplier.get());
mTasksSurface.onFinishNativeInitialization(mActivity, mOmniboxStubSupplier.get(),
mStartSurfaceMediator.getFeedReliabilityLogger());
}

if (mIsInitPending) {
Expand All @@ -427,7 +428,7 @@ mParentTabSupplier, new ScrollableContainerDelegateImpl(), mSnackbarManager,
if (mIsSecondaryTaskInitPending) {
mIsSecondaryTaskInitPending = false;
mSecondaryTasksSurface.onFinishNativeInitialization(
mActivity, mOmniboxStubSupplier.get());
mActivity, mOmniboxStubSupplier.get(), /*feedReliabilityLogger=*/null);
mSecondaryTasksSurface.initialize();
}
}
Expand Down Expand Up @@ -634,7 +635,7 @@ private TabSwitcher.Controller initializeSecondaryTasksSurface() {
mShareDelegateSupplier, mMultiWindowModeStateDispatcher, mContainerView);
if (mIsInitializedWithNative) {
mSecondaryTasksSurface.onFinishNativeInitialization(
mActivity, mOmniboxStubSupplier.get());
mActivity, mOmniboxStubSupplier.get(), /*feedReliabilityLogger=*/null);
mSecondaryTasksSurface.initialize();
} else {
mIsSecondaryTaskInitPending = true;
Expand Down
49 changes: 43 additions & 6 deletions ...art_surface/java/src/org/chromium/chrome/features/start_surface/StartSurfaceMediator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import org.chromium.chrome.R;
import org.chromium.chrome.browser.back_press.BackPressManager;
import org.chromium.chrome.browser.browser_controls.BrowserControlsStateProvider;
import org.chromium.chrome.browser.feed.FeedReliabilityLogger;
import org.chromium.chrome.browser.flags.CachedFeatureFlags;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.lens.LensEntryPoint;
Expand Down Expand Up @@ -250,6 +251,17 @@ public void restoreCompleted() {

@Override
public void willAddTab(Tab tab, @TabLaunchType int type) {
if (mStartSurfaceState == StartSurfaceState.SHOWN_HOMEPAGE
&& type != TabLaunchType.FROM_LONGPRESS_BACKGROUND) {
// Log if the creation of this tab will hide the surface and there is an
// ongoing feed launch. If the tab creation is due to a feed card tap, "card
// tapped" should already have been logged marking the end of the launch.
FeedReliabilityLogger logger = getFeedReliabilityLogger();
if (logger != null) {
logger.onPageLoadStarted();
}
}

// When the tab model is empty and a new background tab is added, it is
// immediately selected, which normally causes the overview to hide. We
// don't want to hide the overview when creating a tab in the background, so
Expand Down Expand Up @@ -646,9 +658,7 @@ public void showOverview(boolean animate) {
&& mPropertyModel.get(EXPLORE_SURFACE_COORDINATOR) == null
&& !mActivityStateChecker.isFinishingOrDestroyed()
&& mExploreSurfaceCoordinatorFactory != null) {
mPropertyModel.set(EXPLORE_SURFACE_COORDINATOR,
mExploreSurfaceCoordinatorFactory.create(ColorUtils.inNightMode(mContext),
mHasFeedPlaceholderShown, mLaunchOrigin));
createAndSetExploreSurfaceCoordinator();
}
mTabModelSelector.addObserver(mTabModelSelectorObserver);

Expand Down Expand Up @@ -694,6 +704,13 @@ public boolean onBackPressed() {
}
}

if (isOnHomepage) {
FeedReliabilityLogger feedReliabilityLogger = getFeedReliabilityLogger();
if (feedReliabilityLogger != null) {
feedReliabilityLogger.onNavigateBack();
}
}

return mController.onBackPressed(isOnHomepage);
}

Expand Down Expand Up @@ -810,6 +827,10 @@ public void finishedHiding() {
private void destroyExploreSurfaceCoordinator() {
ExploreSurfaceCoordinator exploreSurfaceCoordinator =
mPropertyModel.get(EXPLORE_SURFACE_COORDINATOR);
FeedReliabilityLogger logger = getFeedReliabilityLogger();
if (logger != null) {
mOmniboxStub.removeUrlFocusChangeListener(logger);
}
if (exploreSurfaceCoordinator != null) exploreSurfaceCoordinator.destroy();
mPropertyModel.set(EXPLORE_SURFACE_COORDINATOR, null);
}
Expand Down Expand Up @@ -872,9 +893,7 @@ private void setExploreSurfaceVisibility(boolean isVisible) {
if (isVisible && mPropertyModel.get(IS_SHOWING_OVERVIEW)
&& mPropertyModel.get(EXPLORE_SURFACE_COORDINATOR) == null
&& !mActivityStateChecker.isFinishingOrDestroyed()) {
mPropertyModel.set(EXPLORE_SURFACE_COORDINATOR,
mExploreSurfaceCoordinatorFactory.create(ColorUtils.inNightMode(mContext),
mHasFeedPlaceholderShown, mLaunchOrigin));
createAndSetExploreSurfaceCoordinator();
}

mPropertyModel.set(IS_EXPLORE_SURFACE_VISIBLE, isVisible);
Expand Down Expand Up @@ -1139,4 +1158,22 @@ void setOnTabSelectingListener(StartSurface.OnTabSelectingListener onTabSelectin
private int getPixelSize(int id) {
return mContext.getResources().getDimensionPixelSize(id);
}

private void createAndSetExploreSurfaceCoordinator() {
ExploreSurfaceCoordinator exploreSurfaceCoordinator =
mExploreSurfaceCoordinatorFactory.create(
ColorUtils.inNightMode(mContext), mHasFeedPlaceholderShown, mLaunchOrigin);
mPropertyModel.set(EXPLORE_SURFACE_COORDINATOR, exploreSurfaceCoordinator);
FeedReliabilityLogger feedReliabilityLogger =
exploreSurfaceCoordinator.getFeedReliabilityLogger();
if (feedReliabilityLogger != null) {
mOmniboxStub.addUrlFocusChangeListener(feedReliabilityLogger);
}
}

FeedReliabilityLogger getFeedReliabilityLogger() {
if (mPropertyModel == null) return null;
ExploreSurfaceCoordinator coordinator = mPropertyModel.get(EXPLORE_SURFACE_COORDINATOR);
return coordinator != null ? coordinator.getFeedReliabilityLogger() : null;
}
}
4 changes: 3 additions & 1 deletion ...roid/features/start_surface/java/src/org/chromium/chrome/features/tasks/TasksSurface.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import org.chromium.base.supplier.Supplier;
import org.chromium.chrome.browser.compositor.layouts.Layout;
import org.chromium.chrome.browser.feed.FeedReliabilityLogger;
import org.chromium.chrome.browser.omnibox.OmniboxStub;
import org.chromium.chrome.browser.tasks.tab_management.TabSwitcher;
import org.chromium.chrome.browser.tasks.tab_management.TabSwitcherCustomViewManager;
Expand Down Expand Up @@ -80,7 +81,8 @@ public interface TasksSurface {
* Called when the native initialization is completed. Anything to construct a TasksSurface but
* require native initialization should be constructed here.
*/
void onFinishNativeInitialization(Context context, OmniboxStub omniboxStub);
void onFinishNativeInitialization(Context context, OmniboxStub omniboxStub,
@Nullable FeedReliabilityLogger feedReliabilityLogger);

/**
* @param onOffsetChangedListener Registers listener for the offset changes of the header view.
Expand Down
6 changes: 4 additions & 2 deletions ...es/start_surface/java/src/org/chromium/chrome/features/tasks/TasksSurfaceCoordinator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.chromium.chrome.R;
import org.chromium.chrome.browser.browser_controls.BrowserControlsStateProvider;
import org.chromium.chrome.browser.compositor.layouts.content.TabContentManager;
import org.chromium.chrome.browser.feed.FeedReliabilityLogger;
import org.chromium.chrome.browser.feedback.HelpAndFeedbackLauncherImpl;
import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
import org.chromium.chrome.browser.multiwindow.MultiWindowModeStateDispatcher;
Expand Down Expand Up @@ -234,13 +235,14 @@ public View getView() {
}

@Override
public void onFinishNativeInitialization(Context context, OmniboxStub omniboxStub) {
public void onFinishNativeInitialization(Context context, OmniboxStub omniboxStub,
@Nullable FeedReliabilityLogger feedReliabilityLogger) {
if (mTabSwitcher != null) {
mTabSwitcher.initWithNative(context, mTabContentManager,
mDynamicResourceLoaderSupplier.get(), mSnackbarManager, mModalDialogManager);
}

mMediator.initWithNative(omniboxStub);
mMediator.initWithNative(omniboxStub, feedReliabilityLogger);
}

@Override
Expand Down
7 changes: 6 additions & 1 deletion ...tures/start_surface/java/src/org/chromium/chrome/features/tasks/TasksSurfaceMediator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import androidx.annotation.Nullable;

import org.chromium.base.metrics.RecordUserAction;
import org.chromium.chrome.browser.feed.FeedReliabilityLogger;
import org.chromium.chrome.browser.lens.LensEntryPoint;
import org.chromium.chrome.browser.lens.LensMetrics;
import org.chromium.chrome.browser.ntp.IncognitoCookieControlsManager;
Expand Down Expand Up @@ -69,7 +70,8 @@ class TasksSurfaceMediator implements OverviewModeObserver {
mModel.set(IS_LENS_BUTTON_VISIBLE, false);
}

public void initWithNative(OmniboxStub omniboxStub) {
public void initWithNative(
OmniboxStub omniboxStub, @Nullable FeedReliabilityLogger feedReliabilityLogger) {
mOmniboxStub = omniboxStub;
assert mOmniboxStub != null;

Expand Down Expand Up @@ -102,6 +104,9 @@ public void afterTextChanged(Editable s) {
mModel.set(VOICE_SEARCH_BUTTON_CLICK_LISTENER, new View.OnClickListener() {
@Override
public void onClick(View v) {
if (feedReliabilityLogger != null) {
feedReliabilityLogger.onVoiceSearch();
}
mOmniboxStub.getVoiceRecognitionHandler().startVoiceRecognition(
VoiceRecognitionHandler.VoiceInteractionSource.TASKS_SURFACE);
RecordUserAction.record("TasksSurface.FakeBox.VoiceSearch");
Expand Down

0 comments on commit 5390ada

Please sign in to comment.