Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(38)

Issue 17552014: [Sync] Have SBH tell SyncManager which types to purge (Closed)

Created:
7 years, 6 months ago by Nicolas Zea
Modified:
7 years, 6 months ago
Reviewers:
rlarocque
CC:
chromium-reviews, Raghu Simha, haitaol1, akalin, tim (not reviewing)
Visibility:
Public.

Description

[Sync] Have SBH tell SyncManager which types to purge The SBH now specifies as part of the ConfigureSyncer call which types need to be purged. This is determined by whether this is a control types config (in which case all disabled types are purged) or whether any types have been disabled since the last configuration (as determined by the SBR). BUG=247115 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=208074

Patch Set 1 #

Patch Set 2 : Self review #

Patch Set 3 : Self review #

Total comments: 2

Patch Set 4 : Initialize last configured types properly #

Patch Set 5 : Fix dchecks #

Unified diffs Side-by-side diffs Delta from patch set Stats (+90 lines, -78 lines) Patch
M chrome/browser/sync/glue/sync_backend_host.h View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/sync/glue/sync_backend_host.cc View 1 2 3 4 9 chunks +23 lines, -6 lines 0 comments Download
M chrome/browser/sync/glue/sync_backend_host_unittest.cc View 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/sync/glue/sync_backend_registrar.h View 1 2 3 4 chunks +14 lines, -5 lines 0 comments Download
M chrome/browser/sync/glue/sync_backend_registrar.cc View 1 2 3 3 chunks +11 lines, -4 lines 0 comments Download
M chrome/browser/sync/glue/sync_backend_registrar_unittest.cc View 1 2 chunks +3 lines, -0 lines 0 comments Download
M chrome/browser/sync/test_profile_sync_service.h View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/sync/test_profile_sync_service.cc View 1 chunk +1 line, -0 lines 0 comments Download
M sync/internal_api/public/sync_manager.h View 1 1 chunk +1 line, -1 line 0 comments Download
M sync/internal_api/public/test/fake_sync_manager.h View 1 1 chunk +1 line, -1 line 0 comments Download
M sync/internal_api/sync_manager_impl.h View 1 2 2 chunks +6 lines, -8 lines 0 comments Download
M sync/internal_api/sync_manager_impl.cc View 1 2 3 chunks +15 lines, -33 lines 0 comments Download
M sync/internal_api/sync_manager_impl_unittest.cc View 1 4 chunks +4 lines, -9 lines 0 comments Download
M sync/internal_api/test/fake_sync_manager.cc View 1 2 chunks +6 lines, -9 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
Nicolas Zea
PTAL
7 years, 6 months ago (2013-06-21 20:25:04 UTC) #1
rlarocque
I'm not convinced that last_configured_types_ is necessary. What's the difference between it and GetRoutingInfoTypes(registrar_->GetModelSafeRoutingInfo())? https://codereview.chromium.org/17552014/diff/5001/chrome/browser/sync/glue/sync_backend_registrar.cc ...
7 years, 6 months ago (2013-06-21 20:40:02 UTC) #2
Nicolas Zea
Last configured types is necessary because the DTM/PSS can and does call DeactivateDataTypes before calling ...
7 years, 6 months ago (2013-06-21 21:20:48 UTC) #3
rlarocque
lgtm
7 years, 6 months ago (2013-06-21 21:33:35 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/zea@chromium.org/17552014/10001
7 years, 6 months ago (2013-06-21 21:36:06 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/zea@chromium.org/17552014/24001
7 years, 6 months ago (2013-06-21 21:59:56 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/zea@chromium.org/17552014/24001
7 years, 6 months ago (2013-06-22 02:46:29 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/zea@chromium.org/17552014/24001
7 years, 6 months ago (2013-06-22 03:09:14 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/zea@chromium.org/17552014/24001
7 years, 6 months ago (2013-06-22 03:22:20 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/zea@chromium.org/17552014/24001
7 years, 6 months ago (2013-06-22 03:37:01 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/zea@chromium.org/17552014/24001
7 years, 6 months ago (2013-06-22 03:44:24 UTC) #11
commit-bot: I haz the power
7 years, 6 months ago (2013-06-22 17:10:39 UTC) #12
Message was sent while issue was closed.
Change committed as 208074

Powered by Google App Engine
This is Rietveld 408576698