[Hub] Remove erroneous asserts

When Hub is enabled these two on layout changed runnable already set asserts are not valid. This is because if the Hub animation is cancelled the associated runnable no longer needs to run and can be dropped. Because either a hide or show runnable will trigger when the animation is cancelled we don't need to null out the runnables to avoid object leaks. Bug: b/321017523 Change-Id: I844ca137f5d854cd264fc19f95e03cc5338599da Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5212027 Reviewed-by: Sky Malice <[email protected]> Commit-Queue: Calder Kitagawa <[email protected]> Cr-Commit-Position: refs/heads/main@{#1248860}
jamlee-t · Jan 18, 2024 · 9066e49 · 9066e49
1 parent cee46f2
commit 9066e49
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 4 deletions.
diff --git a/.../tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabListCoordinator.java b/.../tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabListCoordinator.java
@@ -30,6 +30,7 @@
 import androidx.recyclerview.widget.RecyclerView.OnItemTouchListener;
 
 import org.chromium.base.Callback;
+import org.chromium.base.Log;
 import org.chromium.base.TraceEvent;
 import org.chromium.base.supplier.ObservableSupplier;
 import org.chromium.base.supplier.Supplier;
@@ -62,6 +63,8 @@
 /** Coordinator for showing UI for a list of tabs. Can be used in GRID or STRIP modes. */
 public class TabListCoordinator
         implements PriceMessageService.PriceWelcomeMessageProvider, DestroyObserver {
+    private static final String TAG = "TabListCoordinator";
+
     /**
      * Modes of showing the list of tabs.
      *
@@ -483,7 +486,15 @@ Size getThumbnailSize() {
     }
 
     void waitForLayoutWithTab(int tabId, Runnable r) {
-        assert mAwaitingLayoutRunnable == null;
+        // Very fast navigations to/from the tab list may not have time for a layout to reach a
+        // completed state. Since this is primarily used for cancellable or skippable animations
+        // where the runnable will not be serviced downstream, dropping the runnable altogether is
+        // safe.
+        if (mAwaitingLayoutRunnable != null) {
+            Log.d(TAG, "Dropping AwaitingLayoutRunnable for " + mAwaitingTabId);
+            mAwaitingLayoutRunnable = null;
+            mAwaitingTabId = Tab.INVALID_TAB_ID;
+        }
         int index = getIndexForTabId(tabId);
         if (index == TabModel.INVALID_TAB_INDEX) {
             r.run();

diff --git a/...tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabListRecyclerView.java b/...tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabListRecyclerView.java
@@ -40,6 +40,7 @@
 import androidx.recyclerview.widget.RecyclerView;
 
 import org.chromium.base.Log;
+import org.chromium.chrome.browser.hub.HubFieldTrial;
 import org.chromium.chrome.browser.tabmodel.TabModel;
 import org.chromium.chrome.tab_ui.R;
 import org.chromium.ui.base.ViewUtils;
@@ -182,9 +183,15 @@ protected void onLayout(boolean changed, int l, int t, int r, int b) {
      * @param runnable the runnable that executes on next layout.
      */
     void runAnimationOnNextLayout(Runnable runnable) {
-        assert mOnNextLayoutRunnable == null
-                : "TabListRecyclerView animation on next layout set multiple times without"
-                        + " running.";
+        // Very fast navigations to/from the tab list may not have time for a layout to reach a
+        // completed state. Since this is primarily used for cancellable or skippable animations
+        // where the runnable will not be serviced downstream, dropping the runnable altogether is
+        // safe for Hub.
+        if (!HubFieldTrial.isHubEnabled()) {
+            assert mOnNextLayoutRunnable == null
+                    : "TabListRecyclerView animation on next layout set multiple times without"
+                            + " running.";
+        }
         mOnNextLayoutRunnable =
                 () -> {
                     if (mDynamicView == null) {