New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 653219 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

[USS] Service should always have a processor

Project Member Reported by s...@chromium.org, Oct 5 2016

Issue description

In the initial implementation of DeviceInfoService, we check to see if there's any metadata present, and intelligently decide if we want to create a processor or not. Things however get messy when the processor is later set. We're currently using a boolean flag to track if the metadata was ever loaded, and if so, on proc set, we know that there must not have been any metadata so we pass the proc empty metadata. This OnChangeProcessorSet override and awkward tracking of state would need to be duplicated on every service. Alternatively the service could just always re-load the metadata, which is inefficient.

Instead, if we just always have a processor, then we can always load the metadata upon startup and this simplifies lifecycles and state tracking. The main ramification is that now instead of checking if our processor accessor returns nullptr or not, we'll need to call into the processor and have it tell us if we should skip Put/Delete calls.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 13 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/345782f64469290397bb2a80087dcfe3cf09ab43

commit 345782f64469290397bb2a80087dcfe3cf09ab43
Author: skym <skym@chromium.org>
Date: Thu Oct 13 21:22:40 2016

[Sync] Services can now always assume processor exists.

BUG= 653219 

Review-Url: https://codereview.chromium.org/2406163006
Cr-Commit-Position: refs/heads/master@{#425165}

[modify] https://crrev.com/345782f64469290397bb2a80087dcfe3cf09ab43/chrome/browser/sync/test/integration/two_client_uss_sync_test.cc
[modify] https://crrev.com/345782f64469290397bb2a80087dcfe3cf09ab43/components/sync/device_info/device_info_service.cc
[modify] https://crrev.com/345782f64469290397bb2a80087dcfe3cf09ab43/components/sync/device_info/device_info_service.h
[modify] https://crrev.com/345782f64469290397bb2a80087dcfe3cf09ab43/components/sync/device_info/device_info_service_unittest.cc
[modify] https://crrev.com/345782f64469290397bb2a80087dcfe3cf09ab43/components/sync/driver/model_type_controller_unittest.cc
[modify] https://crrev.com/345782f64469290397bb2a80087dcfe3cf09ab43/components/sync/model/fake_model_type_change_processor.cc
[modify] https://crrev.com/345782f64469290397bb2a80087dcfe3cf09ab43/components/sync/model/fake_model_type_change_processor.h
[modify] https://crrev.com/345782f64469290397bb2a80087dcfe3cf09ab43/components/sync/model/fake_model_type_service.cc
[modify] https://crrev.com/345782f64469290397bb2a80087dcfe3cf09ab43/components/sync/model/fake_model_type_service.h
[modify] https://crrev.com/345782f64469290397bb2a80087dcfe3cf09ab43/components/sync/model/model_type_change_processor.h
[modify] https://crrev.com/345782f64469290397bb2a80087dcfe3cf09ab43/components/sync/model/model_type_service.cc
[modify] https://crrev.com/345782f64469290397bb2a80087dcfe3cf09ab43/components/sync/model/model_type_service.h
[modify] https://crrev.com/345782f64469290397bb2a80087dcfe3cf09ab43/components/sync/model/model_type_service_unittest.cc
[modify] https://crrev.com/345782f64469290397bb2a80087dcfe3cf09ab43/components/sync/model/stub_model_type_service.cc
[modify] https://crrev.com/345782f64469290397bb2a80087dcfe3cf09ab43/components/sync/model/stub_model_type_service.h
[modify] https://crrev.com/345782f64469290397bb2a80087dcfe3cf09ab43/components/sync/model_impl/shared_model_type_processor.cc
[modify] https://crrev.com/345782f64469290397bb2a80087dcfe3cf09ab43/components/sync/model_impl/shared_model_type_processor.h
[modify] https://crrev.com/345782f64469290397bb2a80087dcfe3cf09ab43/components/sync/model_impl/shared_model_type_processor_unittest.cc

Comment 2 by s...@chromium.org, Oct 13 2016

Status: Fixed (was: Available)

Sign in to add a comment