[Start] Use last visible time for showing Start at startup.

Logged the last background time synchronously was added in
https://crrev.com/c/3964629. However, the sync call causes an increase
of ANRs. In this CL, we remove the sync call, and record "the last time
when Chrome was showing in foreground" instead of "the last time when
Chrome went to background or closed". This allows to record the time
stamp asynchronously and isn't get lost due to shutdown. This would
slightly showing Start more frequently comparing with using the last
background time though.

Bug: 1410604
Change-Id: Idcc2f00ea23d7b4a74a6dc11b29d6356f19c067d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4198974
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Brandon Wylie <wylieb@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Weilun Shi <sweilun@chromium.org>
Commit-Queue: Xi Han <hanxi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1100749}

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

@@ -69,6 +69,7 @@
 import org.chromium.base.test.util.Feature;
 import org.chromium.base.test.util.Restriction;
 import org.chromium.chrome.R;
+import org.chromium.chrome.browser.ChromeInactivityTracker;
 import org.chromium.chrome.browser.ChromeTabbedActivity;
 import org.chromium.chrome.browser.feed.FeedPlaceholderLayout;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
@@ -609,9 +610,11 @@ private void startSurfaceRecordHistogramsTest(boolean isSingleTabSwitcher) {
             Assert.assertEquals(1,
                     RecordHistogram.getHistogramValueCountForTesting(
                             ReturnToChromeUtil.START_SHOW_STATE_UMA,
-
                             StartSurfaceState.SHOWING_HOMEPAGE));
         }
+        Assert.assertEquals(1,
+                RecordHistogram.getHistogramTotalCountForTesting(
+                        ChromeInactivityTracker.UMA_IS_LAST_VISIBLE_TIME_LOGGED));
     }
 
     @Test

chrome/android/java/src/org/chromium/chrome/browser/ChromeInactivityTracker.java

@@ -12,6 +12,7 @@
 import org.chromium.chrome.browser.lifecycle.DestroyObserver;
 import org.chromium.chrome.browser.lifecycle.PauseResumeWithNativeObserver;
 import org.chromium.chrome.browser.lifecycle.StartStopWithNativeObserver;
+import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
 import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
 
 /**
@@ -25,6 +26,9 @@ public class ChromeInactivityTracker
             "Startup.Android.DurationSinceLastBackgroundTime";
     private static final String UMA_IS_LAST_BACKGROUND_TIME_LOGGED =
             "Startup.Android.IsLastBackgroundTimeLogged";
+    @VisibleForTesting
+    public static final String UMA_IS_LAST_VISIBLE_TIME_LOGGED =
+            "Startup.Android.IsLastVisibleTimeLogged";
 
     private static final long UNKNOWN_LAST_BACKGROUNDED_TIME = -1;
 
@@ -54,20 +58,11 @@ public void register(ActivityLifecycleDispatcher lifecycleDispatcher) {
      * Updates the shared preferences to contain the given time. Used internally and for tests.
      * @param timeInMillis the time to record.
      */
-    @VisibleForTesting
+    @VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)
     public void setLastBackgroundedTimeInPrefs(long timeInMillis) {
         SharedPreferencesManager.getInstance().writeLong(mPrefName, timeInMillis);
     }
 
-    /**
-     * Updates the shared preferences to contain the given time synchronously. Used only in shutdown
-     * or moving Chrome to the background.
-     * @param timeInMillis the time to record.
-     */
-    public void setLastBackgroundedTimeInPrefsSync(long timeInMillis) {
-        SharedPreferencesManager.getInstance().writeLongSync(mPrefName, timeInMillis);
-    }
-
     /**
      * @return The last backgrounded time in millis.
      */
@@ -87,6 +82,31 @@ public long getTimeSinceLastBackgroundedMs() {
         return System.currentTimeMillis() - lastBackgroundedTimeMs;
     }
 
+    /**
+     * Sets the last time when Chrome was launching or moving to foreground and became visible to
+     * users and record histogram.
+     */
+    public void setLastVisibleTimeMsAndRecord(long timeInMillis) {
+        // We log the last visible time here to prevent losing the time stamp during the shutdown.
+        long lastVisibleTime = getLastVisibleTimeMs();
+        SharedPreferencesManager.getInstance().writeLong(
+                ChromePreferenceKeys.TABBED_ACTIVITY_LAST_VISIBLE_TIME_MS, timeInMillis);
+
+        Log.i(TAG, "Last visible time read from the SharedPreference is:" + lastVisibleTime + ".");
+        RecordHistogram.recordBooleanHistogram(
+                UMA_IS_LAST_VISIBLE_TIME_LOGGED, lastVisibleTime != UNKNOWN_LAST_BACKGROUNDED_TIME);
+    }
+
+    /**
+     * Gets the last time when Chrome was launching or moving to foreground and became visible to
+     * users.
+     */
+    public long getLastVisibleTimeMs() {
+        return SharedPreferencesManager.getInstance().readLong(
+                ChromePreferenceKeys.TABBED_ACTIVITY_LAST_VISIBLE_TIME_MS,
+                UNKNOWN_LAST_BACKGROUNDED_TIME);
+    }
+
     @Override
     public void onStartWithNative() {}
 

chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java

@@ -1038,6 +1038,8 @@ public void finishNativeInitialization() {
             ChromeAccessibilityUtil.get().addObserver(mLayoutManager);
             if (isTablet()) ChromeAccessibilityUtil.get().addObserver(mCompositorViewHolder);
             if (BackPressManager.isEnabled()) initializeBackPressHandlers();
+
+            mInactivityTracker.setLastVisibleTimeMsAndRecord(System.currentTimeMillis());
         }
     }
 
@@ -1074,6 +1076,8 @@ public void onResumeWithNative() {
 
         if (!isWarmOnResume()) {
             SuggestionsMetrics.recordArticlesListVisible();
+        } else {
+            mInactivityTracker.setLastVisibleTimeMsAndRecord(System.currentTimeMillis());
         }
 
         FeatureNotificationUtils.handleIntentIfApplicable(getIntent());
@@ -1089,7 +1093,7 @@ public void onPauseWithNative() {
 
         NavigationPredictorBridge.onPause();
         // Always track the last backgrounded time in case others are using the pref.
-        mInactivityTracker.setLastBackgroundedTimeInPrefsSync(System.currentTimeMillis());
+        mInactivityTracker.setLastBackgroundedTimeInPrefs(System.currentTimeMillis());
 
         super.onPauseWithNative();
     }

chrome/android/java/src/org/chromium/chrome/browser/tasks/ReturnToChromeUtil.java

@@ -146,16 +146,17 @@ public void destroy() {
 
     /**
      * Determine if we should show the tab switcher on returning to Chrome.
-     *   Returns true if enough time has elapsed since the app was last backgrounded.
-     *   The threshold time in milliseconds is set by experiment "enable-tab-switcher-on-return" or
-     *   from segmentation platform result if {@link ChromeFeatureList.START_SURFACE_RETURN_TIME} is
-     *   enabled.
+     *   Returns true if enough time has elapsed since the app was last backgrounded or foreground,
+     *   depending on which time is the max.
+     *   The threshold time in milliseconds is set by experiment "enable-start-surface-return-time"
+     *   or from segmentation platform result if {@link ChromeFeatureList.START_SURFACE_RETURN_TIME}
+     *   is enabled.
      *
-     * @param lastBackgroundedTimeMillis The last time the application was backgrounded. Set in
-     *                                   ChromeTabbedActivity::onStopWithNative
+     * @param lastTimeMillis The last time the application was backgrounded or foreground, depends
+     *                       on which time is the max. Set in ChromeTabbedActivity::onStopWithNative
      * @return true if past threshold, false if not past threshold or experiment cannot be loaded.
      */
-    public static boolean shouldShowTabSwitcher(final long lastBackgroundedTimeMillis) {
+    public static boolean shouldShowTabSwitcher(final long lastTimeMillis) {
         long tabSwitcherAfterMillis =
                 StartSurfaceConfiguration.START_SURFACE_RETURN_TIME_SECONDS.getValue()
                 * DateUtils.SECOND_IN_MILLIS;
@@ -165,7 +166,7 @@ public static boolean shouldShowTabSwitcher(final long lastBackgroundedTimeMilli
             tabSwitcherAfterMillis = getReturnTimeFromSegmentation();
         }
 
-        if (lastBackgroundedTimeMillis == -1) {
+        if (lastTimeMillis == -1) {
             // No last background timestamp set, use control behavior unless "immediate" was set.
             return tabSwitcherAfterMillis == 0;
         }
@@ -175,7 +176,7 @@ public static boolean shouldShowTabSwitcher(final long lastBackgroundedTimeMilli
             return false;
         }
 
-        return System.currentTimeMillis() - lastBackgroundedTimeMillis >= tabSwitcherAfterMillis;
+        return System.currentTimeMillis() - lastTimeMillis >= tabSwitcherAfterMillis;
     }
 
     /**
@@ -467,9 +468,11 @@ public static boolean shouldShowOverviewPageOnStart(Context context, Intent inte
         }
 
         // Checks whether to show the Start surface due to feature flag TAB_SWITCHER_ON_RETURN_MS.
-        long lastBackgroundedTimeMillis = inactivityTracker.getLastBackgroundedTimeMs();
+        long lastVisibleTimeMs = inactivityTracker.getLastVisibleTimeMs();
+        long lastBackgroundTimeMs = inactivityTracker.getLastBackgroundedTimeMs();
         boolean tabSwitcherOnReturn = IntentUtils.isMainIntentFromLauncher(intent)
-                && ReturnToChromeUtil.shouldShowTabSwitcher(lastBackgroundedTimeMillis);
+                && ReturnToChromeUtil.shouldShowTabSwitcher(
+                        Math.max(lastBackgroundTimeMs, lastVisibleTimeMs));
 
         // If the overview page won't be shown on startup, stops here.
         if (!tabSwitcherOnReturn) return false;