[Tab Switcher] Update thumbnail in response to onUrlUpdated
onUrlUpdated triggers TabContentManager to invalidate the thumbnail if
the cached thumbnail's URL does not match the current URL. This update
isn't reflected correctly in the tab switcher as the old thumbnail
remains.
Change this behavior because it can result in inconsistent UI for
Tab Group Sync & Data Sharing.
Fixed: 367195874
Change-Id: I7bf43d823afe29311a07f7f6e301952762f19f39
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5905227
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1363300}
diff --git a/chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabListMediator.java b/chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabListMediator.java
index 6f3a4c59..afc7c73 100644
--- a/chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabListMediator.java
+++ b/chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabListMediator.java
@@ -564,19 +564,25 @@
int index = mModel.indexFromId(updatedTab.getId());
+ @Nullable Tab tab = null;
+ @Nullable PropertyModel model = null;
if (index != TabModel.INVALID_TAB_INDEX) {
- mModel.get(index)
- .model
- .set(TabProperties.URL_DOMAIN, getDomainForTab(updatedTab));
+ tab = updatedTab;
+ model = mModel.get(index).model;
} else if (mActionsOnAllRelatedTabs) {
@Nullable
Pair<Integer, Tab> indexAndTab =
getIndexAndTabForRootId(updatedTab.getRootId());
- if (indexAndTab == null) return;
- Tab tab = indexAndTab.second;
- PropertyModel model = mModel.get(indexAndTab.first).model;
-
+ if (indexAndTab != null) {
+ tab = indexAndTab.second;
+ model = mModel.get(indexAndTab.first).model;
+ }
+ }
+ if (tab != null && model != null) {
model.set(TabProperties.URL_DOMAIN, getDomainForTab(tab));
+ // Changing URL will result in a thumbnail invalidation if the on-disk
+ // thumbnail doesn't match.
+ updateThumbnailFetcher(model, tab.getId());
}
}
};
diff --git a/chrome/android/features/tab_ui/junit/src/org/chromium/chrome/browser/tasks/tab_management/TabListMediatorUnitTest.java b/chrome/android/features/tab_ui/junit/src/org/chromium/chrome/browser/tasks/tab_management/TabListMediatorUnitTest.java
index 83dc564..1ef054b6 100644
--- a/chrome/android/features/tab_ui/junit/src/org/chromium/chrome/browser/tasks/tab_management/TabListMediatorUnitTest.java
+++ b/chrome/android/features/tab_ui/junit/src/org/chromium/chrome/browser/tasks/tab_management/TabListMediatorUnitTest.java
@@ -2830,10 +2830,14 @@
.getDomainAndRegistry(eq(NEW_URL), anyBoolean());
doReturn(new GURL(NEW_URL)).when(mTab1).getUrl();
+
+ var oldFetcher = mModel.get(POSITION1).model.get(TabProperties.THUMBNAIL_FETCHER);
mTabObserverCaptor.getValue().onUrlUpdated(mTab1);
assertEquals(mNewDomain, mModel.get(POSITION1).model.get(TabProperties.URL_DOMAIN));
assertEquals(mTab2Domain, mModel.get(POSITION2).model.get(TabProperties.URL_DOMAIN));
+ assertNotEquals(
+ oldFetcher, mModel.get(POSITION1).model.get(TabProperties.THUMBNAIL_FETCHER));
}
@Test
@@ -2854,11 +2858,14 @@
// Update URL_DOMAIN for mTab1.
doReturn(new GURL(NEW_URL)).when(mTab1).getUrl();
+ var oldFetcher = mModel.get(POSITION1).model.get(TabProperties.THUMBNAIL_FETCHER);
mTabObserverCaptor.getValue().onUrlUpdated(mTab1);
assertEquals(
mNewDomain + ", " + mTab2Domain,
mModel.get(POSITION1).model.get(TabProperties.URL_DOMAIN));
+ var newFetcher = mModel.get(POSITION1).model.get(TabProperties.THUMBNAIL_FETCHER);
+ assertNotEquals(oldFetcher, newFetcher);
// Update URL_DOMAIN for mTab2.
doReturn(new GURL(NEW_URL)).when(mTab2).getUrl();
@@ -2867,6 +2874,8 @@
assertEquals(
mNewDomain + ", " + mNewDomain,
mModel.get(POSITION1).model.get(TabProperties.URL_DOMAIN));
+ var newestFetcher = mModel.get(POSITION1).model.get(TabProperties.THUMBNAIL_FETCHER);
+ assertNotEquals(newFetcher, newestFetcher);
}
@Test
@@ -2887,6 +2896,7 @@
doReturn(mNewDomain)
.when(mUrlUtilitiesJniMock)
.getDomainAndRegistry(eq(NEW_URL), anyBoolean());
+ var oldFetcher = mModel.get(POSITION1).model.get(TabProperties.THUMBNAIL_FETCHER);
// Update URL_DOMAIN for mTab1.
doReturn(new GURL(NEW_URL)).when(mTab1).getUrl();
@@ -2894,6 +2904,10 @@
assertEquals(mNewDomain, mModel.get(POSITION1).model.get(TabProperties.URL_DOMAIN));
assertEquals(mTab2Domain, mModel.get(POSITION2).model.get(TabProperties.URL_DOMAIN));
+ var newFetcher = mModel.get(POSITION1).model.get(TabProperties.THUMBNAIL_FETCHER);
+ assertNotEquals(oldFetcher, newFetcher);
+
+ oldFetcher = mModel.get(POSITION2).model.get(TabProperties.THUMBNAIL_FETCHER);
// Update URL_DOMAIN for mTab2.
doReturn(new GURL(NEW_URL)).when(mTab2).getUrl();
@@ -2901,6 +2915,9 @@
assertEquals(mNewDomain, mModel.get(POSITION1).model.get(TabProperties.URL_DOMAIN));
assertEquals(mNewDomain, mModel.get(POSITION2).model.get(TabProperties.URL_DOMAIN));
+
+ newFetcher = mModel.get(POSITION2).model.get(TabProperties.THUMBNAIL_FETCHER);
+ assertNotEquals(oldFetcher, newFetcher);
}
@Test
