Popular sites are not displayed in new tab page |
||||||||
Issue descriptionApp Version: 64.0.3282.87 Beta iOS Version: 10.3.2, 11.2.2, 11.2.5 beta 5 Device: iPhone and iPad Steps to reproduce: 1. Fresh install chrome 2. Turn on Airplane mode and launch chrome 3. Wait until baked in sites are displayed in tab page 4. Turn off the device Airplane mode and open a new tab page 5. Force quit the app and relaunch chrome Observed results: Baked in popular sites are not replaced with location based popular sites Expected results: Once device is connected to internet the location based popular sites should be displayed in new tab page Number of times you were able to reproduce: 5/5 Bug reproducible after clean install: Yes Bug reproducible after clearing cache and cookies: Yes Bug reproducible on Chrome Mobile on Android: Not tested Bug reproducible on Safari/Firefox: Firefox: NA, Safari: NA Bug reproducible on current stable build (App Version, iOS Version): Yes on M63 Bug reproducible on the current beta channel build (App Version, iOS Version): Yes on M64 Link to video/image: https://drive.google.com/file/d/1s9FEaomfSAJDT_EvB8CoSqVE7TqpkqVH/view?usp=sharing
,
Jan 19 2018
Assigning to fhorschig@: I am not sure who is the current owner for that, please reassign if needed.
,
Jan 19 2018
Reproducible on Android Chrome Beta 64. Mikel, do you know who will maintain popular sites in the future? What an interesting edge case. The ntp-internals (thanks for opening them in the video) show that IN is already set as country and - as it's not automatically refetching - the update went far enough to cache the last fetch of the sites. Still: the displayed sites (and apparently cached sites) are the baked-in ones which should be completely replaced by the newer ones. I don't really have an idea how this can happen. Observations so far: - Logs don't show any failures (like they should when the download fails) - gstatic links look good (i.e. for India/Germany where it's reproducible) - version and country are always cached once the all new sites were written to the cache: https://cs.chromium.org/chromium/src/components/ntp_tiles/popular_sites_impl.cc?sq=package:chromium&dr=CSs&l=483 ... so how can the cache still contain the old sites but the new version indicator?
,
Jan 22 2018
Found the underlying issue: We refresh popular sites only, if we have less than 8 tiles which are not "POPULAR". We introduced a second type of popular tiles called "POPULAR_BAKED_IN" which were not considered "POPULAR". The fix is just checking for that second line. The one liner is out for review here: https://crrev.com/c/878322
,
Jan 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/764f29082e59ba67ca66961611475f7b647e1bd2 commit 764f29082e59ba67ca66961611475f7b647e1bd2 Author: Friedrich Horschig <fhorschig@chromium.org> Date: Mon Jan 22 14:28:48 2018 Fix popular sites missing refreshes We count baked-in popular sites to personal sites when deciding whether to refetch the current set of popular sites. This causes popular sites to not refresh if the first fetch failed. With this CL, we treat baked-in popular sites as popular sites which brings back timely refreshes. Bug: 803773 Change-Id: I96c34f02f440d453eb1ae3b422c422c4827b0127 Reviewed-on: https://chromium-review.googlesource.com/878322 Commit-Queue: Friedrich Horschig <fhorschig@chromium.org> Reviewed-by: Marc Treib <treib@chromium.org> Cr-Commit-Position: refs/heads/master@{#530857} [modify] https://crrev.com/764f29082e59ba67ca66961611475f7b647e1bd2/components/ntp_tiles/most_visited_sites.cc
,
Jan 22 2018
This issue should now be fixed on M65. I assume we want to merge back to M64. Would someone please verify with an iOS device?
,
Jan 23 2018
Issue is still reproducible in 65.0.3325.11 beta, might require to merge the changes in to M65. Issue is fixed in 66.0.3329.0 Canary, popular sites are now displayed on following steps mentioned in comment #0. Tested on: iPhone 7(iOS 10.3.3) iPhone 8 plus(iOS 11.2.5 beta 7) iPad Air(iOS 11.2.5 beta7) Link to video of M66: https://drive.google.com/file/d/1MyoWdMWtG5e38kldJUZNzpJEfSdq9nM9/view?usp=sharing
,
Jan 23 2018
Thanks for testing and the correction: yes, I meant 66. I am requesting a merge for 65.
,
Jan 24 2018
Your change meets the bar and is auto-approved for M65. Please go ahead and merge the CL to branch 3325 manually. Please contact milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 24 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9e19030082989ad8bded403ff7534d68c7030359 commit 9e19030082989ad8bded403ff7534d68c7030359 Author: Friedrich Horschig <fhorschig@chromium.org> Date: Wed Jan 24 11:47:45 2018 Fix popular sites missing refreshes We count baked-in popular sites to personal sites when deciding whether to refetch the current set of popular sites. This causes popular sites to not refresh if the first fetch failed. With this CL, we treat baked-in popular sites as popular sites which brings back timely refreshes. Bug: 803773 Change-Id: I96c34f02f440d453eb1ae3b422c422c4827b0127 Reviewed-on: https://chromium-review.googlesource.com/878322 Commit-Queue: Friedrich Horschig <fhorschig@chromium.org> Reviewed-by: Marc Treib <treib@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#530857}(cherry picked from commit 764f29082e59ba67ca66961611475f7b647e1bd2) Reviewed-on: https://chromium-review.googlesource.com/883441 Reviewed-by: Friedrich Horschig <fhorschig@chromium.org> Cr-Commit-Position: refs/branch-heads/3325@{#58} Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369} [modify] https://crrev.com/9e19030082989ad8bded403ff7534d68c7030359/components/ntp_tiles/most_visited_sites.cc
,
Jan 24 2018
The merge happened, with the next 65 build, it should work as intended again.
,
Jan 25 2018
Verified in 65.0.3325.20 Beta, popular sites are now displayed on following step mentioned in comment #0. Works as intended. Tested on following devices: iPhone 7plus(iOS 10.3.3) iPhone X (iOS 11.2.2) iPad Air(iOS 11.2.5 beta7)
,
Jan 25 2018
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by stkhapugin@chromium.org
, Jan 19 2018Owner: gambard@chromium.org
Status: Assigned (was: Untriaged)