Use latest index when updating favicons in callback

Bug: 989319
Change-Id: I4ed0bb779e5bca4be0ebb6cc34db8ed74a1ed65a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1730190Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Commit-Queue: Yue Zhang <yuezhanggg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683092}

chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabListMediator.java

@@ -19,6 +19,7 @@ import android.view.View;
 import org.chromium.base.Callback;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.Log;
+import org.chromium.base.VisibleForTesting;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.base.metrics.RecordUserAction;
 import org.chromium.chrome.browser.compositor.layouts.content.TabContentManager;
@@ -918,7 +919,8 @@ class TabListMediator {
         return mTabModelSelector.getTabModelFilterProvider().getCurrentTabModelFilter().index();
     }
 
-    private void updateFaviconForTab(Tab tab, @Nullable Bitmap icon) {
+    @VisibleForTesting
+    void updateFaviconForTab(Tab tab, @Nullable Bitmap icon) {
         int modelIndex = mModel.indexFromId(tab.getId());
         if (modelIndex == Tab.INVALID_TAB_ID) return;
         // For tab group card in grid tab switcher, the favicon is set to be null.
@@ -935,8 +937,11 @@ class TabListMediator {
         }
         Callback<Drawable> faviconCallback = drawable -> {
             assert drawable != null;
-            if (drawable != null) {
-                mModel.get(modelIndex).set(TabProperties.FAVICON, drawable);
+            // Need to re-get the index because the original index can be stale when callback is
+            // triggered.
+            int index = mModel.indexFromId(tab.getId());
+            if (index != Tab.INVALID_TAB_ID && drawable != null) {
+                mModel.get(index).set(TabProperties.FAVICON, drawable);
             }
         };
         mTabListFaviconProvider.getFaviconForUrlAsync(

chrome/android/features/tab_ui/junit/src/org/chromium/chrome/browser/tasks/tab_management/TabListMediatorUnitTest.java

@@ -264,6 +264,25 @@ public class TabListMediatorUnitTest {
         assertNull(mModel.get(0).get(TabProperties.FAVICON));
     }
 
+    @Test
+    public void updateFavicon_StaleIndex() {
+        initAndAssertAllProperties();
+        mModel.get(0).set(TabProperties.FAVICON, null);
+        mModel.get(1).set(TabProperties.FAVICON, null);
+
+        mMediator.updateFaviconForTab(mTab2, null);
+        assertThat(mModel.indexFromId(TAB2_ID), equalTo(1));
+        // Before executing callback, there is a deletion in tab list model which makes the index
+        // stale.
+        mModel.removeAt(0);
+        assertThat(mModel.indexFromId(TAB2_ID), equalTo(0));
+
+        // Start to execute callback.
+        mCallbackCaptor.getValue().onResult(mFaviconDrawable);
+
+        assertThat(mModel.get(0).get(TabProperties.FAVICON), equalTo(mFaviconDrawable));
+    }
+
     @Test
     public void sendsSelectSignalCorrectly() {
         initAndAssertAllProperties();