Regression:'Loading...' message is seen on chrome://history/syncedTabs for a long time after page refresh |
||||||||
Issue descriptionChrome Version:73.0.3643.0 (Official Build) Revision 310dbdcb8884c1a70ee5cc78c7527778870ea067-refs/branch-heads/3643@{#1}(32/64-bit) OS: Windows(7,8,8.1,10), Mac(10.13.1, 10.13.6, 10.14.3) and Linux(14.04 LTS) OS What steps will reproduce the problem? 1. Fresh launch chrome, navigate to chrome://history/syncedTabs. 2. Sign in to chrome by clicking on 'Turn On Sync' button. 3. If any entries are present after syncing then hide all the entries by clicking on the 'Hide now' option from more actions icon corresponding to each entry. 4. Once all the entries are hidden, refresh the page and observe. Actual Result : 'Loading...' message is seen on chrome://history/syncedTabs for a long time. Expected Result: Instead 'No tabs from other devices' message should be seen almost immediately after refreshing chrome://history/syncedTabs. This is a regression issue broken in ‘M-72’ and and below is the bisect info: Good Build : 72.0.3583.0(Revision:600164) Bad Build : 72.0.3584.0(Revision:600616) Per-revision bisect info: You are probably looking for a change made after 600224 (known good), but no later than 600225 (first known bad). CHANGE-LOG URL: https://chromium.googlesource.com/chromium/src/+log/a1f14a1097fa402e72b99c5cab944b2d588bbc79..177774d57a744b75630cba5507c3bf1ed419aea3 Suspecting: https://chromium.googlesource.com/chromium/src/+/177774d57a744b75630cba5507c3bf1ed419aea3 @dpapad: Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner. Thank You
,
Dec 18
This is due to an observer now firing when the signed in state changes from undefined to true at page load. The page thinks that the user has just signed in and it needs to wait for the list of tabs even though the list has already been initialized, which results in the loading message displaying forever. Candidate fix at https://chromium-review.googlesource.com/c/chromium/src/+/1381246.
,
Dec 18
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7ef0ead6ee467da48b76be0b2a94fbda3f0d6e33 commit 7ef0ead6ee467da48b76be0b2a94fbda3f0d6e33 Author: rbpotter <rbpotter@chromium.org> Date: Wed Dec 19 06:00:03 2018 History WebUI: Fix an issue with reload in Polymer 2 When reloading, the change in the user's signed in state from |undefined| to true triggers the signInStateChanged_ observer in SyncedDeviceManager. This happens after the session list and search term are initialized, so |fetchingSyncedTabs_| is set to true after all synced tabs have already been retrieved. This results in the "Loading" message being displayed to the user indefinitely. Fix by ignoring changes to the signed in state from an undefined value. This matches the Polymer 1 behavior and the observer only needs to run if the user has signed in or signed out when the page is already open. Bug: 915641 Change-Id: I4b1f3e5fee2e36d214f78cdd108af019d6120a86 Reviewed-on: https://chromium-review.googlesource.com/c/1381246 Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Commit-Queue: Rebekah Potter <rbpotter@chromium.org> Cr-Commit-Position: refs/heads/master@{#617738} [modify] https://crrev.com/7ef0ead6ee467da48b76be0b2a94fbda3f0d6e33/chrome/browser/resources/md_history/synced_device_manager.js [modify] https://crrev.com/7ef0ead6ee467da48b76be0b2a94fbda3f0d6e33/chrome/test/data/webui/md_history/history_synced_tabs_test.js [modify] https://crrev.com/7ef0ead6ee467da48b76be0b2a94fbda3f0d6e33/chrome/test/data/webui/md_history/md_history_browsertest.js
,
Dec 21
Tested this with 73.0.3647.0 Canary on Windows 10 and it appears to be fixed. Since this bug impacts any users who are signed in but have no synced tabs (chrome://history/syncedTabs will display a "Loading..." message forever), and the fix is small, has been in Canary for a few days, and is also validated by a new automated test added in the commit in comment 4, requesting a merge to M-72.
,
Dec 21
This bug requires manual review: M72 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 21
,
Dec 21
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fcbb615effc50fc44276a58123ae92a715f72cd7 Commit: fcbb615effc50fc44276a58123ae92a715f72cd7 Author: rbpotter@chromium.org Commiter: rbpotter@chromium.org Date: 2018-12-21 23:00:09 +0000 UTC History WebUI: Fix an issue with reload in Polymer 2 (M72) When reloading, the change in the user's signed in state from |undefined| to true triggers the signInStateChanged_ observer in SyncedDeviceManager. This happens after the session list and search term are initialized, so |fetchingSyncedTabs_| is set to true after all synced tabs have already been retrieved. This results in the "Loading" message being displayed to the user indefinitely. Fix by ignoring changes to the signed in state from an undefined value. This matches the Polymer 1 behavior and the observer only needs to run if the user has signed in or signed out when the page is already open. TBR=dpapad@chromium.org Bug: 915641 Change-Id: I4b1f3e5fee2e36d214f78cdd108af019d6120a86 Reviewed-on: https://chromium-review.googlesource.com/c/1381246 Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Commit-Queue: Rebekah Potter <rbpotter@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#617738}(cherry picked from commit 7ef0ead6ee467da48b76be0b2a94fbda3f0d6e33) Reviewed-on: https://chromium-review.googlesource.com/c/1389028 Reviewed-by: Rebekah Potter <rbpotter@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#509} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
,
Dec 21
,
Dec 21
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fcbb615effc50fc44276a58123ae92a715f72cd7 commit fcbb615effc50fc44276a58123ae92a715f72cd7 Author: rbpotter <rbpotter@chromium.org> Date: Fri Dec 21 23:00:09 2018 History WebUI: Fix an issue with reload in Polymer 2 (M72) When reloading, the change in the user's signed in state from |undefined| to true triggers the signInStateChanged_ observer in SyncedDeviceManager. This happens after the session list and search term are initialized, so |fetchingSyncedTabs_| is set to true after all synced tabs have already been retrieved. This results in the "Loading" message being displayed to the user indefinitely. Fix by ignoring changes to the signed in state from an undefined value. This matches the Polymer 1 behavior and the observer only needs to run if the user has signed in or signed out when the page is already open. TBR=dpapad@chromium.org Bug: 915641 Change-Id: I4b1f3e5fee2e36d214f78cdd108af019d6120a86 Reviewed-on: https://chromium-review.googlesource.com/c/1381246 Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Commit-Queue: Rebekah Potter <rbpotter@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#617738}(cherry picked from commit 7ef0ead6ee467da48b76be0b2a94fbda3f0d6e33) Reviewed-on: https://chromium-review.googlesource.com/c/1389028 Reviewed-by: Rebekah Potter <rbpotter@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#509} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/fcbb615effc50fc44276a58123ae92a715f72cd7/chrome/browser/resources/md_history/synced_device_manager.js [modify] https://crrev.com/fcbb615effc50fc44276a58123ae92a715f72cd7/chrome/test/data/webui/md_history/history_synced_tabs_test.js [modify] https://crrev.com/fcbb615effc50fc44276a58123ae92a715f72cd7/chrome/test/data/webui/md_history/md_history_browsertest.js |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by dpa...@chromium.org
, Dec 17Owner: rbpotter@chromium.org