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

Issue 688533 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocking:
issue 696052



Sign in to add a comment

[Sync] ModelTypeStore needs to work when sync is disabled by flag

Project Member Reported by s...@chromium.org, Feb 3 2017

Issue description

Right now ProfileSyncServiceFactory will return nullptr instead of a PSS if the --disable-sync flag is passed, see https://cs.chromium.org/chromium/src/chrome/browser/sync/profile_sync_service_factory.cc?l=92

This is a problem for anyone using ModelTypeStore (like printers), because they need to call https://cs.chromium.org/chromium/src/components/browser_sync/profile_sync_service.cc?type=cs&q=GetModelTypeStoreFactory&sq=package:chromium&l=1672 during their initialization.

Temporarily we essentially break this flag. Is that okay? Unclear. Would be nice if we could grab the task runner from somewhere else.
 

Comment 1 Deleted

Comment 2 Deleted

Comment 3 by s...@chromium.org, Feb 24 2017

This is the root cause of  issue 694992 , it looks like ChromeOS Guest Mode doesn't have a PSS.

Comment 4 by skau@chromium.org, Feb 24 2017

Blocking: 696052

Comment 5 by pav...@chromium.org, Feb 25 2017

Labels: -Pri-3 Pri-1
Owner: pav...@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 9 2017

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

commit 222de9fb1677cdabfbf8998c17dd0e394f71a81e
Author: pavely <pavely@chromium.org>
Date: Thu Mar 09 15:23:59 2017

[Sync] ModelTypeStore factory shouldn't require valid PSS to function correctly

The issue is that PSS::GetModelTypeStoreFactory requires valid PSS which doesn't
have to be available when sync is disabled (for example in guest mode on
ChromeOS). The reason it is currently wired through PSS is that PSS conveniently
has path and blocking task runner required for ModelTypeStore.

I added static GetModelTypeStoreFactory function that takes profile path and
SequencedWorkerPool. It ensures that, for a given path, all task runners use the
same sequence token and thus properly sequneced. Datatypes that wish to be
hosted in shared leveldb database should use this factory function.

I also fixed issue that backend_map_ is accessed from multiple threads and thus
might get corrupt. I wrapped it into a class with lock.

BUG= 688533 
R=skym@chromium.org

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

[modify] https://crrev.com/222de9fb1677cdabfbf8998c17dd0e394f71a81e/chrome/browser/chromeos/printing/printers_manager_factory.cc
[modify] https://crrev.com/222de9fb1677cdabfbf8998c17dd0e394f71a81e/chrome/browser/sync/chrome_sync_client.cc
[modify] https://crrev.com/222de9fb1677cdabfbf8998c17dd0e394f71a81e/chrome/browser/sync/chrome_sync_client.h
[modify] https://crrev.com/222de9fb1677cdabfbf8998c17dd0e394f71a81e/chrome/browser/sync/profile_sync_service_factory.cc
[modify] https://crrev.com/222de9fb1677cdabfbf8998c17dd0e394f71a81e/chrome/browser/sync/profile_sync_test_util.cc
[modify] https://crrev.com/222de9fb1677cdabfbf8998c17dd0e394f71a81e/components/browser_sync/profile_sync_service.cc
[modify] https://crrev.com/222de9fb1677cdabfbf8998c17dd0e394f71a81e/components/browser_sync/profile_sync_service.h
[modify] https://crrev.com/222de9fb1677cdabfbf8998c17dd0e394f71a81e/components/browser_sync/profile_sync_test_util.cc
[modify] https://crrev.com/222de9fb1677cdabfbf8998c17dd0e394f71a81e/components/sync/driver/fake_sync_client.cc
[modify] https://crrev.com/222de9fb1677cdabfbf8998c17dd0e394f71a81e/components/sync/driver/fake_sync_client.h
[modify] https://crrev.com/222de9fb1677cdabfbf8998c17dd0e394f71a81e/components/sync/driver/sync_client.h
[modify] https://crrev.com/222de9fb1677cdabfbf8998c17dd0e394f71a81e/components/sync/driver/sync_service_base.cc
[modify] https://crrev.com/222de9fb1677cdabfbf8998c17dd0e394f71a81e/components/sync/driver/sync_service_base.h
[modify] https://crrev.com/222de9fb1677cdabfbf8998c17dd0e394f71a81e/components/sync/model/model_type_store.h
[modify] https://crrev.com/222de9fb1677cdabfbf8998c17dd0e394f71a81e/components/sync/model_impl/model_type_store_backend.cc
[modify] https://crrev.com/222de9fb1677cdabfbf8998c17dd0e394f71a81e/components/sync/model_impl/model_type_store_backend.h
[modify] https://crrev.com/222de9fb1677cdabfbf8998c17dd0e394f71a81e/components/sync/model_impl/model_type_store_backend_unittest.cc
[modify] https://crrev.com/222de9fb1677cdabfbf8998c17dd0e394f71a81e/ios/chrome/browser/reading_list/reading_list_model_factory.cc
[modify] https://crrev.com/222de9fb1677cdabfbf8998c17dd0e394f71a81e/ios/chrome/browser/sync/ios_chrome_profile_sync_service_factory.cc
[modify] https://crrev.com/222de9fb1677cdabfbf8998c17dd0e394f71a81e/ios/chrome/browser/sync/ios_chrome_profile_sync_test_util.cc
[modify] https://crrev.com/222de9fb1677cdabfbf8998c17dd0e394f71a81e/ios/chrome/browser/sync/ios_chrome_sync_client.h
[modify] https://crrev.com/222de9fb1677cdabfbf8998c17dd0e394f71a81e/ios/chrome/browser/sync/ios_chrome_sync_client.mm

Comment 7 by pav...@chromium.org, Mar 10 2017

Status: Fixed (was: Assigned)

Comment 8 by dchan@google.com, May 30 2017

Labels: VerifyIn-60

Comment 9 by dchan@chromium.org, Aug 1 2017

Labels: VerifyIn-61

Comment 10 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment