Immediately unsubscribe from invalidations when Recent Tabs is closed

On Android, Sync only subscribes for SESSIONS invalidations while the
Recent Tabs page is open.

Before this CL, the corresponding *un*subscription was delayed by one
hour, to avoid subscription/unsubscription churn. However, this task
was not persisted, i.e. it got lost if Chrome was shut down within that
hour. The new FCM-based invalidations system does not do any cleanup on
startup (in contrast to the old Tango system), meaning such an
unsubscription would then never actually happen.

This CL removes the unregistration delay, as a quick fix to avoid most
of the "dangling subscriptions". It also adds a feature
"SyncUseSessionsUnregisterDelay" (disabled by default) to go back to
the previous behavior.

Bug: 1024817
Change-Id: Iaaf1afca3ba8719a3837dd760e4bd169d4d3c3c3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1919209Reviewed-by: Tim Schumann <tschumann@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719621}

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

@@ -335,6 +335,8 @@ public abstract class ChromeFeatureList {
     public static final String SWAP_PIXEL_FORMAT_TO_FIX_CONVERT_FROM_TRANSLUCENT =
             "SwapPixelFormatToFixConvertFromTranslucent";
     public static final String SYNC_MANUAL_START_ANDROID = "SyncManualStartAndroid";
+    public static final String SYNC_USE_SESSIONS_UNREGISTER_DELAY =
+            "SyncUseSessionsUnregisterDelay";
     public static final String TAB_ENGAGEMENT_REPORTING_ANDROID = "TabEngagementReportingAndroid";
     public static final String TAB_GROUPS_ANDROID = "TabGroupsAndroid";
     public static final String TAB_GROUPS_UI_IMPROVEMENTS_ANDROID =

chrome/android/java/src/org/chromium/chrome/browser/invalidation/SessionsInvalidationManager.java

@@ -11,6 +11,7 @@ import androidx.annotation.VisibleForTesting;
 import org.chromium.base.ApplicationState;
 import org.chromium.base.ApplicationStatus;
 import org.chromium.base.ThreadUtils;
+import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.ntp.ForeignSessionHelper;
 import org.chromium.chrome.browser.profiles.Profile;
 
@@ -34,6 +35,7 @@ public class SessionsInvalidationManager implements ApplicationStatus.Applicatio
      * The amount of time after the RecentTabsPage is closed to unregister for session sync
      * invalidations. The delay is long to avoid registering and unregistering a lot if the user
      * visits the RecentTabsPage a lot.
+     * Only applied if the feature SyncUseSessionsUnregisterDelay is enabled.
      */
     static final int UNREGISTER_FOR_SESSION_SYNC_INVALIDATIONS_DELAY_MS =
             (int) DateUtils.HOUR_IN_MILLIS;
@@ -97,8 +99,11 @@ public class SessionsInvalidationManager implements ApplicationStatus.Applicatio
     public void onRecentTabsPageClosed() {
         --mNumRecentTabPages;
         if (mNumRecentTabPages == 0) {
-            setSessionInvalidationsEnabled(
-                    false, UNREGISTER_FOR_SESSION_SYNC_INVALIDATIONS_DELAY_MS);
+            setSessionInvalidationsEnabled(false,
+                    ChromeFeatureList.isEnabled(
+                            ChromeFeatureList.SYNC_USE_SESSIONS_UNREGISTER_DELAY)
+                            ? UNREGISTER_FOR_SESSION_SYNC_INVALIDATIONS_DELAY_MS
+                            : 0);
         }
     }
 

chrome/browser/android/chrome_feature_list.cc

@@ -229,6 +229,7 @@ const base::Feature* kFeaturesExposedToJava[] = {
     &security_state::features::kMarkHttpAsFeature,
     &signin::kMiceFeature,
     &switches::kSyncManualStartAndroid,
+    &switches::kSyncUseSessionsUnregisterDelay,
     &subresource_filter::kSafeBrowsingSubresourceFilter,
 };
 

components/browser_sync/browser_sync_switches.cc

@@ -24,6 +24,9 @@ const char kLocalSyncBackendDir[] = "local-sync-backend-dir";
 #if defined(OS_ANDROID)
 const base::Feature kSyncManualStartAndroid{"SyncManualStartAndroid",
                                             base::FEATURE_ENABLED_BY_DEFAULT};
-#endif
+
+const base::Feature kSyncUseSessionsUnregisterDelay{
+    "SyncUseSessionsUnregisterDelay", base::FEATURE_DISABLED_BY_DEFAULT};
+#endif  // defined(OS_ANDROID)
 
 }  // namespace switches

components/browser_sync/browser_sync_switches.h

@@ -16,6 +16,7 @@ extern const char kLocalSyncBackendDir[];
 
 #if defined(OS_ANDROID)
 extern const base::Feature kSyncManualStartAndroid;
+extern const base::Feature kSyncUseSessionsUnregisterDelay;
 #endif
 
 }  // namespace switches