Commit db8138f8 authored by Kristi Park's avatar Kristi Park Committed by Commit Bot

[NTP] Refresh the Most Visited tiles when the tile source changes

The "Restore default shortcuts" menu option is not updated unless an
onMostVisitedChanged event is sent. This only occurs when a URL/title
of the new tile set is different from the current tiles. As such (after
adding then removing a link), the restore default option will not be
disabled after clicking since the tiles are considered unchanged.

Now consider tiles with different sources to be unequal.
Screencast: https://screencast.googleplex.com/cast/NTQ0MDY0MTk2MjA4MjMwNHw0MTY5NmRmYi1mNg

Bug: 896566
Change-Id: I8c1474d2a567889fc5296c3a58228a92c3a67b71
Reviewed-on: https://chromium-review.googlesource.com/c/1292149
Commit-Queue: Kristi Park <kristipark@chromium.org>
Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601619}
parent 65c419d8
...@@ -211,7 +211,7 @@ void MostVisitedSitesBridge::Destroy(JNIEnv* env, ...@@ -211,7 +211,7 @@ void MostVisitedSitesBridge::Destroy(JNIEnv* env,
void MostVisitedSitesBridge::OnHomepageStateChanged( void MostVisitedSitesBridge::OnHomepageStateChanged(
JNIEnv* env, JNIEnv* env,
const JavaParamRef<jobject>& obj) { const JavaParamRef<jobject>& obj) {
most_visited_->RefreshHomepageTile(); most_visited_->RefreshTiles();
} }
void MostVisitedSitesBridge::SetHomepageClient( void MostVisitedSitesBridge::SetHomepageClient(
......
...@@ -43,7 +43,8 @@ bool AreMostVisitedItemsEqual( ...@@ -43,7 +43,8 @@ bool AreMostVisitedItemsEqual(
for (size_t i = 0; i < new_items.size(); ++i) { for (size_t i = 0; i < new_items.size(); ++i) {
if (new_items[i].url != old_item_id_pairs[i].second.url || if (new_items[i].url != old_item_id_pairs[i].second.url ||
new_items[i].title != old_item_id_pairs[i].second.title) { new_items[i].title != old_item_id_pairs[i].second.title ||
new_items[i].source != old_item_id_pairs[i].second.source) {
return false; return false;
} }
} }
......
...@@ -244,7 +244,7 @@ void MostVisitedSites::Refresh() { ...@@ -244,7 +244,7 @@ void MostVisitedSites::Refresh() {
suggestions_service_->FetchSuggestionsData(); suggestions_service_->FetchSuggestionsData();
} }
void MostVisitedSites::RefreshHomepageTile() { void MostVisitedSites::RefreshTiles() {
BuildCurrentTiles(); BuildCurrentTiles();
} }
...@@ -252,10 +252,8 @@ void MostVisitedSites::InitializeCustomLinks() { ...@@ -252,10 +252,8 @@ void MostVisitedSites::InitializeCustomLinks() {
if (!custom_links_ || !current_tiles_.has_value() || !custom_links_enabled_) if (!custom_links_ || !current_tiles_.has_value() || !custom_links_enabled_)
return; return;
if (custom_links_->Initialize(current_tiles_.value())) { if (custom_links_->Initialize(current_tiles_.value()))
custom_links_action_count_ = 0; custom_links_action_count_ = 0;
BuildCurrentTiles();
}
} }
void MostVisitedSites::UninitializeCustomLinks() { void MostVisitedSites::UninitializeCustomLinks() {
......
...@@ -156,8 +156,8 @@ class MostVisitedSites : public history::TopSitesObserver, ...@@ -156,8 +156,8 @@ class MostVisitedSites : public history::TopSitesObserver,
// if the request resulted in the set of tiles changing. // if the request resulted in the set of tiles changing.
void Refresh(); void Refresh();
// Forces a rebuild of the current tiles to update the pinned homepage. // Forces a rebuild of the current tiles.
void RefreshHomepageTile(); void RefreshTiles();
// Initializes custom links, which "freezes" the current MV tiles and converts // Initializes custom links, which "freezes" the current MV tiles and converts
// them to custom links. Once custom links is initialized, MostVisitedSites // them to custom links. Once custom links is initialized, MostVisitedSites
......
...@@ -650,7 +650,7 @@ TEST_P(MostVisitedSitesTest, ShouldUpdateHomepageTileWhenRefreshHomepageTile) { ...@@ -650,7 +650,7 @@ TEST_P(MostVisitedSitesTest, ShouldUpdateHomepageTileWhenRefreshHomepageTile) {
EXPECT_CALL(*mock_top_sites_, SyncWithHistory()).Times(0); EXPECT_CALL(*mock_top_sites_, SyncWithHistory()).Times(0);
EXPECT_CALL(mock_observer_, OnURLsAvailable(Not(FirstPersonalizedTileIs( EXPECT_CALL(mock_observer_, OnURLsAvailable(Not(FirstPersonalizedTileIs(
"", kHomepageUrl, TileSource::HOMEPAGE)))); "", kHomepageUrl, TileSource::HOMEPAGE))));
most_visited_sites_->RefreshHomepageTile(); most_visited_sites_->RefreshTiles();
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
} }
...@@ -1152,6 +1152,7 @@ TEST_P(MostVisitedSitesWithCustomLinksTest, ...@@ -1152,6 +1152,7 @@ TEST_P(MostVisitedSitesWithCustomLinksTest,
EXPECT_CALL(*mock_custom_links_, Initialize(_)).WillOnce(Return(true)); EXPECT_CALL(*mock_custom_links_, Initialize(_)).WillOnce(Return(true));
ExpectBuildWithCustomLinks(expected_links, &sections); ExpectBuildWithCustomLinks(expected_links, &sections);
most_visited_sites_->InitializeCustomLinks(); most_visited_sites_->InitializeCustomLinks();
most_visited_sites_->RefreshTiles();
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_THAT( EXPECT_THAT(
sections.at(SectionType::PERSONALIZED), sections.at(SectionType::PERSONALIZED),
...@@ -1194,6 +1195,7 @@ TEST_P(MostVisitedSitesWithCustomLinksTest, ...@@ -1194,6 +1195,7 @@ TEST_P(MostVisitedSitesWithCustomLinksTest,
EXPECT_CALL(*mock_custom_links_, Initialize(_)).WillOnce(Return(true)); EXPECT_CALL(*mock_custom_links_, Initialize(_)).WillOnce(Return(true));
ExpectBuildWithCustomLinks(expected_links, &sections); ExpectBuildWithCustomLinks(expected_links, &sections);
most_visited_sites_->InitializeCustomLinks(); most_visited_sites_->InitializeCustomLinks();
most_visited_sites_->RefreshTiles();
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
ASSERT_THAT( ASSERT_THAT(
sections.at(SectionType::PERSONALIZED), sections.at(SectionType::PERSONALIZED),
...@@ -1235,6 +1237,7 @@ TEST_P(MostVisitedSitesWithCustomLinksTest, ...@@ -1235,6 +1237,7 @@ TEST_P(MostVisitedSitesWithCustomLinksTest,
EXPECT_CALL(*mock_custom_links_, Initialize(_)).WillOnce(Return(true)); EXPECT_CALL(*mock_custom_links_, Initialize(_)).WillOnce(Return(true));
ExpectBuildWithCustomLinks(expected_links, &sections); ExpectBuildWithCustomLinks(expected_links, &sections);
most_visited_sites_->InitializeCustomLinks(); most_visited_sites_->InitializeCustomLinks();
most_visited_sites_->RefreshTiles();
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
ASSERT_THAT( ASSERT_THAT(
sections.at(SectionType::PERSONALIZED), sections.at(SectionType::PERSONALIZED),
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment