[Instant Start] Record the late FeedContentFirstLoadedTime UMA.

The ContentChangedListener::onAddFinished is not guaranteed to come
before the onOverviewShownAtLaunch(). To fix it, we cache the event of
onOverviewShownAtLaunch() if the feed content is still loading. The UMA
will be recorded when onAddFinished() is called if it is missed on
startup.

Bug: 1107000
Change-Id: I23336149addb7ee4ee986083ce09212e982b1d6d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2316667
Commit-Queue: Xi Han <hanxi@chromium.org>
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#794997}

chrome/android/features/start_surface/internal/javatests/src/org/chromium/chrome/features/start_surface/StartSurfaceTest.java

@@ -75,7 +75,7 @@ import org.chromium.base.test.util.Restriction;
 import org.chromium.chrome.browser.ChromeTabbedActivity;
 import org.chromium.chrome.browser.DeferredStartupHandler;
 import org.chromium.chrome.browser.compositor.layouts.phone.StackLayout;
-import org.chromium.chrome.browser.feed.FeedSurfaceCoordinator;
+import org.chromium.chrome.browser.feed.FeedSurfaceMediator;
 import org.chromium.chrome.browser.flags.CachedFeatureFlags;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.flags.ChromeSwitches;
@@ -988,7 +988,7 @@ public class StartSurfaceTest {
         Assert.assertEquals(expectedRecordCount,
                 RecordHistogram.getHistogramTotalCountForTesting(
                         StartSurfaceConfiguration.getHistogramName(
-                                FeedSurfaceCoordinator.FEED_CONTENT_FIRST_LOADED_TIME_MS_UMA,
+                                FeedSurfaceMediator.FEED_CONTENT_FIRST_LOADED_TIME_MS_UMA,
                                 isInstantStart)));
         Assert.assertEquals(isInstantReturn() ? 1 : 0,
                 RecordHistogram.getHistogramTotalCountForTesting(

chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/FeedSurfaceCoordinator.java

@@ -47,7 +47,6 @@ import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tabmodel.TabModelSelector;
 import org.chromium.chrome.browser.ui.messages.snackbar.SnackbarManager;
 import org.chromium.chrome.browser.user_education.UserEducationHelper;
-import org.chromium.chrome.features.start_surface.StartSurfaceConfiguration;
 import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
 import org.chromium.components.browser_ui.util.GlobalDiscardableReferencePool;
 import org.chromium.components.browser_ui.widget.displaystyle.UiConfig;
@@ -64,9 +63,6 @@ import java.util.List;
  * Provides a surface that displays an interest feed rendered list of content suggestions.
  */
 public class FeedSurfaceCoordinator implements FeedSurfaceProvider {
-    @VisibleForTesting
-    public static final String FEED_CONTENT_FIRST_LOADED_TIME_MS_UMA = "FeedContentFirstLoadedTime";
-
     private final Activity mActivity;
     private final SnackbarManager mSnackbarManager;
     @Nullable
@@ -502,9 +498,7 @@ public class FeedSurfaceCoordinator implements FeedSurfaceProvider {
     }
 
     public void onOverviewShownAtLaunch(long activityCreationTimeMs) {
-        StartSurfaceConfiguration.recordHistogram(FEED_CONTENT_FIRST_LOADED_TIME_MS_UMA,
-                mMediator.getContentFirstAvailableTimeMs() - activityCreationTimeMs,
-                mIsPlaceholderShown);
+        mMediator.onOverviewShownAtLaunch(activityCreationTimeMs, mIsPlaceholderShown);
     }
 
     Tracker getFeatureEngagementTracker() {

chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/FeedSurfaceMediator.java

@@ -39,6 +39,7 @@ import org.chromium.chrome.browser.signin.PersonalizedSigninPromoView;
 import org.chromium.chrome.browser.signin.SigninManager;
 import org.chromium.chrome.browser.signin.SigninPromoUtil;
 import org.chromium.chrome.browser.suggestions.SuggestionsMetrics;
+import org.chromium.chrome.features.start_surface.StartSurfaceConfiguration;
 import org.chromium.components.browser_ui.widget.listmenu.ListMenu;
 import org.chromium.components.browser_ui.widget.listmenu.ListMenuItemProperties;
 import org.chromium.components.feature_engagement.Tracker;
@@ -56,10 +57,14 @@ import org.chromium.ui.mojom.WindowOpenDisposition;
  * A mediator for the {@link FeedSurfaceCoordinator} responsible for interacting with the
  * native library and handling business logic.
  */
-class FeedSurfaceMediator implements NewTabPageLayout.ScrollDelegate,
-                                     ContextMenuManager.TouchEnabledDelegate,
-                                     TemplateUrlServiceObserver, ListMenu.Delegate,
-                                     HomepagePromoStateListener, IdentityManager.Observer {
+@VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)
+public class FeedSurfaceMediator
+        implements NewTabPageLayout.ScrollDelegate, ContextMenuManager.TouchEnabledDelegate,
+                   TemplateUrlServiceObserver, ListMenu.Delegate, HomepagePromoStateListener,
+                   IdentityManager.Observer {
+    @VisibleForTesting
+    public static final String FEED_CONTENT_FIRST_LOADED_TIME_MS_UMA = "FeedContentFirstLoadedTime";
+
     private static final float IPH_TRIGGER_BAR_TRANSITION_FRACTION = 1.0f;
     private static final float IPH_STREAM_MIN_SCROLL_FRACTION = 0.10f;
     private static final float IPH_FEED_HEADER_MAX_POS_FRACTION = 0.35f;
@@ -85,7 +90,17 @@ class FeedSurfaceMediator implements NewTabPageLayout.ScrollDelegate,
     private int mThumbnailWidth;
     private int mThumbnailHeight;
     private int mThumbnailScrollY;
+
+    /** Whether the Feed content is loading. */
+    private boolean mIsLoadingFeed;
+    /** Cached parameters for recording the histogram of "FeedContentFirstLoadedTime". */
+    private boolean mIsInstantStart;
+    private long mActivityCreationTimeMs;
     private long mContentFirstAvailableTimeMs;
+    // Whether missing a histogram record when onOverviewShownAtLaunch() is called. It is possible
+    // that Feed content is still loading at that time and the {@link mContentFirstAvailableTimeMs}
+    // hasn't been set yet.
+    private boolean mHasPendingUmaRecording;
 
     /**
      * @param coordinator The {@link FeedSurfaceCoordinator} that interacts with this class.
@@ -145,6 +160,7 @@ class FeedSurfaceMediator implements NewTabPageLayout.ScrollDelegate,
         }
 
         if (mFeedEnabled) {
+            mIsLoadingFeed = true;
             mCoordinator.createStream();
             if (mSnapScrollHelper != null) {
                 mSnapScrollHelper.setView(mCoordinator.getStream().getView());
@@ -191,7 +207,12 @@ class FeedSurfaceMediator implements NewTabPageLayout.ScrollDelegate,
             public void onAddFinished() {
                 if (mContentFirstAvailableTimeMs == 0) {
                     mContentFirstAvailableTimeMs = SystemClock.elapsedRealtime();
+                    if (mHasPendingUmaRecording) {
+                        maybeRecordContentLoadingTime();
+                        mHasPendingUmaRecording = false;
+                    }
                 }
+                mIsLoadingFeed = false;
             }
         };
         stream.addOnContentChangedListener(mStreamContentChangedListener);
@@ -569,10 +590,6 @@ class FeedSurfaceMediator implements NewTabPageLayout.ScrollDelegate,
         updateSectionHeader();
     }
 
-    long getContentFirstAvailableTimeMs() {
-        return mContentFirstAvailableTimeMs;
-    }
-
     /**
      * The {@link SignInPromo} for the Feed.
      * TODO(huayinz): Update content and visibility through a ModelChangeProcessor.
@@ -618,4 +635,22 @@ class FeedSurfaceMediator implements NewTabPageLayout.ScrollDelegate,
     SignInPromo getSignInPromoForTesting() {
         return mSignInPromo;
     }
+
+    void onOverviewShownAtLaunch(long activityCreationTimeMs, boolean isInstantStart) {
+        assert mActivityCreationTimeMs == 0;
+        mActivityCreationTimeMs = activityCreationTimeMs;
+        mIsInstantStart = isInstantStart;
+
+        if (!maybeRecordContentLoadingTime() && mIsLoadingFeed) {
+            mHasPendingUmaRecording = true;
+        }
+    }
+
+    private boolean maybeRecordContentLoadingTime() {
+        if (mActivityCreationTimeMs == 0 || mContentFirstAvailableTimeMs == 0) return false;
+
+        StartSurfaceConfiguration.recordHistogram(FEED_CONTENT_FIRST_LOADED_TIME_MS_UMA,
+                mContentFirstAvailableTimeMs - mActivityCreationTimeMs, mIsInstantStart);
+        return true;
+    }
 }