New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 741431 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

MostVisitedSites::Refresh doesn't work with TopSites

Project Member Reported by treib@chromium.org, Jul 12 2017

Issue description

MostVisitedSites::Refresh is supposed to trigger a refresh of the tiles in the background. In practice, it just calls suggestions_service_->FetchSuggestionsData() (i.e. tries to update MostLikely). That makes sense; however if SuggestionsService returns no results, e.g. because the user isn't signed in, the refresh request just gets dropped, instead of being redirected to TopSites. That means currently it's not possible to trigger a refresh of TopSites through MostVisitedSites.
 

Comment 1 by fi...@chromium.org, Jul 20 2017

Labels: M-62
Pls find an owner for it or demote to P3. Thx!

Comment 2 by treib@chromium.org, Jul 20 2017

Cc: -mastiz@chromium.org
Owner: mastiz@chromium.org
Status: Assigned (was: Available)
Over to mastiz then, who can delegate if necessary :)

Comment 3 by fi...@chromium.org, Jul 20 2017

Labels: zine-triaged
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1d90c8175b1f4057e0e38afca54eeb65f0e2b25c

commit 1d90c8175b1f4057e0e38afca54eeb65f0e2b25c
Author: Mikel Astiz <mastiz@chromium.org>
Date: Fri Jul 21 13:52:51 2017

Fix InstantService not refreshing TopSites when kNtpTilesFeature enabled

InstantService is the only caller of MostVisitedSites::Refresh() and it
is obvious from the caller's side that TopSites should be refreshed too,
when an NTP is opened. Prior to this patch, TopSites would be updated
time-based, leading to more stale NTP tiles when the feature
kNtpTilesFeature is enabled (non-default case).

The bug is believed to make little impact.

Bug:  741431 
Change-Id: Iccdc4f89d3ab430eafd9a2faf1b1564f363ff29f
Reviewed-on: https://chromium-review.googlesource.com/581147
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488647}
[modify] https://crrev.com/1d90c8175b1f4057e0e38afca54eeb65f0e2b25c/chrome/browser/ntp_tiles/ntp_tiles_browsertest.cc
[modify] https://crrev.com/1d90c8175b1f4057e0e38afca54eeb65f0e2b25c/components/ntp_tiles/most_visited_sites.cc
[modify] https://crrev.com/1d90c8175b1f4057e0e38afca54eeb65f0e2b25c/components/ntp_tiles/most_visited_sites_unittest.cc

Comment 5 by mastiz@chromium.org, Jul 24 2017

Status: Fixed (was: Assigned)

Sign in to add a comment