Sync metadata can leak if sync stopped while starting |
||
Issue descriptionThe simplest scenario where this can occur is a busy I/O thread causing a slow startup process for a datatype. If sync is disabled meanwhile, ModelTypeController is not supposed to just ignore the event.
,
Jun 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/39396141ab32683696bfbda5887b26792034dd75 commit 39396141ab32683696bfbda5887b26792034dd75 Author: Mikel Astiz <mastiz@chromium.org> Date: Fri Jun 22 11:39:57 2018 Refactor ModelTypeController state transitions More explicit state handling is introduced with a switch, and some dead code is replaced with a DCHECK. Bug: 855375 Change-Id: I644d26e63bf51bf0cb56362afa0fa990f75e8be7 Reviewed-on: https://chromium-review.googlesource.com/1111708 Reviewed-by: Marc Treib <treib@chromium.org> Commit-Queue: Mikel Astiz <mastiz@chromium.org> Cr-Commit-Position: refs/heads/master@{#569579} [modify] https://crrev.com/39396141ab32683696bfbda5887b26792034dd75/components/sync/driver/model_association_manager.cc [modify] https://crrev.com/39396141ab32683696bfbda5887b26792034dd75/components/sync/driver/model_type_controller.cc [modify] https://crrev.com/39396141ab32683696bfbda5887b26792034dd75/components/sync/driver/model_type_controller_unittest.cc
,
Jun 25 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7059913e9dfc552ed5c85b0b626a6364517e8679 commit 7059913e9dfc552ed5c85b0b626a6364517e8679 Author: Mikel Astiz <mastiz@chromium.org> Date: Mon Jun 25 19:43:26 2018 Enable more EnableDisableSingleClientTest tests These tests were believed to be failing or be flaky, but I could not reproduce the failures locally, presumably due to recent improvements in https://chromium-review.googlesource.com/1111716 As it seems, DataTypeManagerImpl's mechanism to defer reconfigurations via |needs_reconfigure_| is sufficient to handle fast enable-disable-enable sequences. Bug: 855375 Change-Id: Iec677202f9db15787da1a62345e09852e84a9f79 Reviewed-on: https://chromium-review.googlesource.com/1111844 Commit-Queue: Mikel Astiz <mastiz@chromium.org> Reviewed-by: Marc Treib <treib@chromium.org> Cr-Commit-Position: refs/heads/master@{#570137} [modify] https://crrev.com/7059913e9dfc552ed5c85b0b626a6364517e8679/chrome/browser/sync/test/integration/enable_disable_test.cc
,
Jul 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/23919ce6813da7b70e2d5cf488345f5f7f42fa1d commit 23919ce6813da7b70e2d5cf488345f5f7f42fa1d Author: Mikel Astiz <mastiz@chromium.org> Date: Wed Jul 04 14:12:58 2018 Wait until STOPPING finishes during configuration The process of stopping a datatype is asynchronous by nature, most prominently in USS where we want to take special care about sync metadata being cleared. Prior to this patch, ModelTypeController could silently drop stop events, which could lead to the DataTypeManager (effectively the UI) think a datatype is stopped, while in reality it could remain running. This is problematic in multiple ways, most importantly due to the risk of leaking sync metadata or even data. Instead, what we want is that calls to OnSyncStarting() always have a corresponding StopSync(), and guarantee that OnSyncStarting() is not called twice without a StopSync() call in between. In order to do that, ModelTypeController now enters state STOPPING if the Stop() cannot be immediately executed, because it's currently STARTING. Once the start is finalized, stop is immediately issued and the corresponding callback informed, which allows ModelAssociationManager to continue its job. Bug: 855375 Change-Id: I6e22940fcb56816c43181c34e04c4a6e53c70a21 Reviewed-on: https://chromium-review.googlesource.com/1122863 Commit-Queue: Mikel Astiz <mastiz@chromium.org> Reviewed-by: Marc Treib <treib@chromium.org> Cr-Commit-Position: refs/heads/master@{#572555} [modify] https://crrev.com/23919ce6813da7b70e2d5cf488345f5f7f42fa1d/components/sync/driver/data_type_controller.h [modify] https://crrev.com/23919ce6813da7b70e2d5cf488345f5f7f42fa1d/components/sync/driver/data_type_manager_impl.cc [modify] https://crrev.com/23919ce6813da7b70e2d5cf488345f5f7f42fa1d/components/sync/driver/directory_data_type_controller.cc [modify] https://crrev.com/23919ce6813da7b70e2d5cf488345f5f7f42fa1d/components/sync/driver/directory_data_type_controller.h [modify] https://crrev.com/23919ce6813da7b70e2d5cf488345f5f7f42fa1d/components/sync/driver/model_association_manager.cc [modify] https://crrev.com/23919ce6813da7b70e2d5cf488345f5f7f42fa1d/components/sync/driver/model_association_manager.h [modify] https://crrev.com/23919ce6813da7b70e2d5cf488345f5f7f42fa1d/components/sync/driver/model_type_controller.cc [modify] https://crrev.com/23919ce6813da7b70e2d5cf488345f5f7f42fa1d/components/sync/driver/model_type_controller.h [modify] https://crrev.com/23919ce6813da7b70e2d5cf488345f5f7f42fa1d/components/sync/driver/model_type_controller_unittest.cc [modify] https://crrev.com/23919ce6813da7b70e2d5cf488345f5f7f42fa1d/components/sync/driver/proxy_data_type_controller.cc [modify] https://crrev.com/23919ce6813da7b70e2d5cf488345f5f7f42fa1d/components/sync/driver/proxy_data_type_controller.h
,
Jul 13
|
||
►
Sign in to add a comment |
||
Comment 1 by mastiz@chromium.org
, Jun 22 2018