ProfileSyncService::Initialize() handles disable reasons poorly |
||
Issue descriptionDuring ProfileSyncService::Initialize(), we currently early-out if DISABLE_REASON_PLATFORM_OVERRIDE or DISABLE_REASON_ENTERPRISE_POLICY is set [1]. In the policy case, we also attempt to clear sync data. There are several problems with this: - In practice, platform_sync_allowed_provider_ isn't set at this point, so DISABLE_REASON_PLATFORM_OVERRIDE will never occur. - At this point, neither engine_ nor sync_thread_ exist, which means that we won't actually clear data [2]. - Both disable reasons can disappear during Chrome's lifetime, in which case ProfileSyncService is left in a half-initialized state. [1] https://cs.chromium.org/chromium/src/components/browser_sync/profile_sync_service.cc?rcl=c0439d22bf577600fde02f6d4919da679224447e&l=307 [2] https://cs.chromium.org/chromium/src/components/browser_sync/profile_sync_service.cc?rcl=c0439d22bf577600fde02f6d4919da679224447e&l=675
,
Aug 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/66ac6eece5eade8958eeed6dcbdc210be2028e27 commit 66ac6eece5eade8958eeed6dcbdc210be2028e27 Author: Marc Treib <treib@chromium.org> Date: Mon Aug 06 08:03:03 2018 Fix DisableReason handling in ProfileSyncService::Initialize ProfileSyncService::Initialize used to early-out if DISABLE_REASON_PLATFORM_OVERRIDE or DISABLE_REASON_ENTERPRISE_POLICY were present. However: - In practice, platform_sync_allowed_provider_ isn't set at this point, so DISABLE_REASON_PLATFORM_OVERRIDE will never occur. Even if it did, calling StopImpl(KEEP_DATA) at this point (before |engine_| exists) does nothing. - Both disable reasons can disappear during Chrome's lifetime, in which case ProfileSyncService is left in a half-initialized state. In particular, it won't be listening for auth events. This CL removes handling for DISABLE_REASON_PLATFORM_OVERRIDE, and does not early-out anymore for DISABLE_REASON_ENTERPRISE_POLICY. Bug: 870683 Change-Id: I354629deaa42a782e10658190c84e967fee593a3 Reviewed-on: https://chromium-review.googlesource.com/1162236 Reviewed-by: Mikel Astiz <mastiz@chromium.org> Commit-Queue: Marc Treib <treib@chromium.org> Cr-Commit-Position: refs/heads/master@{#580818} [modify] https://crrev.com/66ac6eece5eade8958eeed6dcbdc210be2028e27/components/browser_sync/profile_sync_service.cc [modify] https://crrev.com/66ac6eece5eade8958eeed6dcbdc210be2028e27/components/browser_sync/profile_sync_service_unittest.cc
,
Aug 6
|
||
►
Sign in to add a comment |
||
Comment 1 by treib@chromium.org
, Aug 3