Issue metadata
Sign in to add a comment
|
"Enable Sync" is displayed in Other Devices on every cold start. |
||||||||||||||||||||||
Issue descriptionApp Version: 60.0.3089.0 canary iOS Version: 10.3.2, 10.1.1 Device: iPhone, iPad URL: na Steps to reproduce: 1. Launch Google Chrome 2. Sing in to Chrome 3. Tap Menu → Recent Tabs 4. Background the app 5. Force quit the by swiping up from App Switcher 6. Launch the app Observed results: "Enable Sync" button is displayed in Other Devices. Expected results: Loading Status should be displayed in 'Other Devices' while sync is in progress. Good Version 58.0.3029.66 #f0ce1a9 Bad Version: 58.0.3029.67 #2fe3f21 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: NA Bug reproducible on Dolphin/Safari/Atomic: Dolphin: NA, Safari: NA Bug reproducible on current stable build (App Version, iOS Version): M58 Yes, Working fine in M57. Bug reproducible on the current beta channel build (App Version, iOS Version): M59 Yes Link to video/image: https://drive.google.com/file/d/0B-xmXLQhjeKuZW43QzdHVnlVSFk/view
,
May 8 2017
,
Aug 31 2017
Any updates on this older regression?
,
Sep 27 2017
,
Oct 6 2017
Ping ? Jif@ who should be assigned to this?
,
Oct 6 2017
Punctuation matters. Jif@, who should be assigned to this? :)
,
Oct 10 2017
Hi Jif@: Could you please assign this to someone who would be best suited to work on it ? Thanks.
,
Oct 10 2017
,
Oct 10 2017
Hmm. Gauthier seems out until next Monday. Is there anyone else who can work on this?
,
Oct 10 2017
,
Oct 10 2017
This was introduced in M58. The user experience is very poor. Add M62 milestone for evaluation in the event of a refresh release. But for sure we should work on this for M63.
,
Oct 10 2017
Can you take a look since gambard is out till next week?
,
Oct 10 2017
,
Oct 11 2017
May be related to this CL, which touched iOS Sync in the revision range: https://codereview.chromium.org/2790483002/
,
Oct 11 2017
The CL above changed the determination of whether a Sync data type is enabled. Before, preferred types were considered enabled, but now, only active types are. The data type for recent tabs (syncer::PROXY_TABS) is considered preferred, but not active (until the sync completes). So, until the sync is completed, the UI shows an activate sync button, since it thinks the data type is not enabled. Is this type supposed to be active if the user enabled it?
,
Oct 11 2017
,
Oct 11 2017
Issue is not limited to recent tabs https://drive.google.com/a/google.com/file/d/0ByhbEVeNGM-JS0JFdHlxNEJBcW8/view?usp=sharing
,
Oct 11 2017
Wow. Good catch Olivier. I am surprised this flew under the radar for 4 releases and we're shipping the 5th one now with this bug in it. There were not unit tests to catch this ?
,
Oct 11 2017
,
Oct 11 2017
Unittesting is not easy as it requires a slow syncing (slow server or a lot of sync data). I will try to do something.
,
Oct 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e4159d97a69759595319b3a48e9b93e0fec9b9eb commit e4159d97a69759595319b3a48e9b93e0fec9b9eb Author: Olivier Robin <olivierrobin@chromium.org> Date: Wed Oct 11 17:53:28 2017 Introduce SyncSetupService::IsDataTypeActive https://codereview.chromium.org/2790483002/ changed the logic of IsDataTypeEnabled, which broke both settings and recent tabs. Fork the IsDataTypeEnabled and introduce IsDataTypeActive - IsDataTypePreferred returns true if the user enabled the settings. - IsDataTypeActive returns true if IsDataTypeEnabled and the syncing setup is complete. Bug: 718462 Change-Id: I09d744725361989a9e165d29158f9f699bc579a2 Reviewed-on: https://chromium-review.googlesource.com/712038 Commit-Queue: Olivier Robin <olivierrobin@chromium.org> Reviewed-by: Sky Malice <skym@chromium.org> Reviewed-by: Eric Noyau <noyau@chromium.org> Cr-Commit-Position: refs/heads/master@{#508027} [modify] https://crrev.com/e4159d97a69759595319b3a48e9b93e0fec9b9eb/ios/chrome/browser/sync/sync_setup_service.cc [modify] https://crrev.com/e4159d97a69759595319b3a48e9b93e0fec9b9eb/ios/chrome/browser/sync/sync_setup_service.h [modify] https://crrev.com/e4159d97a69759595319b3a48e9b93e0fec9b9eb/ios/chrome/browser/sync/sync_setup_service_mock.h [modify] https://crrev.com/e4159d97a69759595319b3a48e9b93e0fec9b9eb/ios/chrome/browser/ui/history/history_collection_view_controller.mm [modify] https://crrev.com/e4159d97a69759595319b3a48e9b93e0fec9b9eb/ios/chrome/browser/ui/history/history_collection_view_controller_unittest.mm [modify] https://crrev.com/e4159d97a69759595319b3a48e9b93e0fec9b9eb/ios/chrome/browser/ui/ntp/recent_tabs/recent_tabs_table_coordinator_unittest.mm [modify] https://crrev.com/e4159d97a69759595319b3a48e9b93e0fec9b9eb/ios/chrome/browser/ui/settings/sync_settings_collection_view_controller.mm
,
Oct 12 2017
Merge status TBD
,
Oct 12 2017
Verified on chrome canary version 63.0.3238.0 on iPhone 6s plus and iPad Air with iOS 10.3.3, iPad Air with iOS 11.0.3, iPhone 8 with iOS 11.1 beta 2, following the steps mentioned in comment #0. After force quitting and relaunching chrome app, other devices under recent tab section is displayed in loading state. Looks good.
,
Oct 12 2017
,
Oct 12 2017
This bug requires manual review: Less than 1 days to go before AppStore submit on M62 Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 12 2017
,
Oct 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4a58092d07f7730c92c166f176b312f441073155 commit 4a58092d07f7730c92c166f176b312f441073155 Author: Olivier Robin <olivierrobin@chromium.org> Date: Thu Oct 12 19:33:23 2017 Introduce SyncSetupService::IsDataTypeActive https://codereview.chromium.org/2790483002/ changed the logic of IsDataTypeEnabled, which broke both settings and recent tabs. Fork the IsDataTypeEnabled and introduce IsDataTypeActive - IsDataTypePreferred returns true if the user enabled the settings. - IsDataTypeActive returns true if IsDataTypeEnabled and the syncing setup is complete. Bug: 718462 Change-Id: I09d744725361989a9e165d29158f9f699bc579a2 Reviewed-on: https://chromium-review.googlesource.com/712038 Commit-Queue: Olivier Robin <olivierrobin@chromium.org> Reviewed-by: Sky Malice <skym@chromium.org> Reviewed-by: Eric Noyau <noyau@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#508027}(cherry picked from commit e4159d97a69759595319b3a48e9b93e0fec9b9eb) Reviewed-on: https://chromium-review.googlesource.com/716382 Reviewed-by: Olivier Robin <olivierrobin@chromium.org> Cr-Commit-Position: refs/branch-heads/3202@{#667} Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} [modify] https://crrev.com/4a58092d07f7730c92c166f176b312f441073155/ios/chrome/browser/sync/sync_setup_service.cc [modify] https://crrev.com/4a58092d07f7730c92c166f176b312f441073155/ios/chrome/browser/sync/sync_setup_service.h [modify] https://crrev.com/4a58092d07f7730c92c166f176b312f441073155/ios/chrome/browser/sync/sync_setup_service_mock.h [modify] https://crrev.com/4a58092d07f7730c92c166f176b312f441073155/ios/chrome/browser/ui/history/history_collection_view_controller.mm [modify] https://crrev.com/4a58092d07f7730c92c166f176b312f441073155/ios/chrome/browser/ui/history/history_collection_view_controller_unittest.mm [modify] https://crrev.com/4a58092d07f7730c92c166f176b312f441073155/ios/chrome/browser/ui/ntp/recent_tabs/recent_tabs_table_coordinator_unittest.mm [modify] https://crrev.com/4a58092d07f7730c92c166f176b312f441073155/ios/chrome/browser/ui/settings/sync_settings_collection_view_controller.mm
,
Oct 13 2017
Verified on 62.0.3202.55 Beta in iPad Air(iOS 10.3.3), iPhone 6s(10.3.3), iPad Air (iOS 11.0) and iPad 8Plus(iOS 11.1 beta) Followed the steps mentioned in comment#0, after force quit and relaunch the recent tab is displaying a loading indicator until the Other devices records are fetched. Working as expected.
,
Oct 13 2017
Issue 770742 has been merged into this issue. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by srikanthg@chromium.org
, May 5 2017