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

Issue 659263 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocked on:
issue 672600



Sign in to add a comment

[USS] DeviceInfoService's |has_provider_initialized_| is fundamentally broken.

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

Issue description

Right now, DeviceInfoService subscribes to LocalDeviceInfoProvider's RegisterOnInitializedCallback, and when it gets called, sets a boolean has_provider_initialized_. However, this doesn't really work because the provider will stop giving real data is the user logs out of sync. See https://cs.chromium.org/chromium/src/components/browser_sync/profile_sync_service.cc?q=profile_sync_service.cc&sq=package:chromium&dr&l=773

We need to figure out a way to deal with having local validity of the service fluctuate, and having many entry points from the processor.

At very least, has_provider_initialized_ and all the DCHECKs need to be replaced by something.
 

Comment 1 by s...@chromium.org, Oct 25 2016

Also, while we're in here cleaning this up, it might make sense to try to improve the RegisterOnInitializedCallback, https://cs.chromium.org/chromium/src/components/sync/device_info/local_device_info_provider.h?cl=GROK&rcl=1477408745&l=55 , it looks like everyone is currently using it once and then calling .reset() on the subscription.
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 28 2016

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

commit fc8171bfb29731cb8f3c302594eb2b9ba9b9239d
Author: skym <skym@chromium.org>
Date: Fri Oct 28 22:38:29 2016

[Sync] DeviceInfoService static method and error cleanup.

* Exampling upon ModelTypeService's comments to explain when OnMetadataLoaded should be called.
* Removed old TODOs from DeviceInfoService
* Updated error handling to be slightly more consistent.
* Moving static methods to be anonymous namespaced.
* Updated unittests to not need static DeviceInfoService methods by modifying the way test data is created.

BUG= 659263 

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

[modify] https://crrev.com/fc8171bfb29731cb8f3c302594eb2b9ba9b9239d/components/sync/device_info/device_info_service.cc
[modify] https://crrev.com/fc8171bfb29731cb8f3c302594eb2b9ba9b9239d/components/sync/device_info/device_info_service.h
[modify] https://crrev.com/fc8171bfb29731cb8f3c302594eb2b9ba9b9239d/components/sync/device_info/device_info_service_unittest.cc
[modify] https://crrev.com/fc8171bfb29731cb8f3c302594eb2b9ba9b9239d/components/sync/model/model_type_sync_bridge.h

Comment 3 by s...@chromium.org, Dec 8 2016

Blockedon: 672600

Comment 4 by s...@chromium.org, Jan 17 2018

Status: WontFix (was: Assigned)

Sign in to add a comment