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

Issue 658443 link

Starred by 2 users
Status: Fixed
Owner:
pav...@chromium.org
Closed: Nov 2016
Cc:
mastiz@chromium.org
█ tschumann@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

[USS] ModelType errors during initialization hit DCHECKs

Project Member Reported by s...@chromium.org, Oct 21 2016 
1. Force DeviceInfo to error before providing metadata. Such as editing DeviceInfoService::OnStoreCreated to always hit the else.

2. DCHECK is hit at https://cs.chromium.org/chromium/src/components/sync/driver/model_type_controller.cc?l=171 . Correctness is unclear to me. We failed to start, so stop was called.

3. If you comment out the above check, we hit another one at https://cs.chromium.org/chromium/src/components/sync/driver/model_type_controller.cc?q=model_type_controller.cc&sq=package:chromium&dr&l=136 . This is because of the disconnnect between the ModelAssociationManager and DataTypeManagerImpl. The MAM knows this type fails, and it removes it from |desired_types_|. But the DTMI doesn't get the memo, it's still in |last_enabled_types_|, so it tells the controller to RegisterWithBackend.
Project Member

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

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

commit 4e96ec4910903cb2a3b5e7ce27263723f2a725d9
Author: pavely <pavely@chromium.org>
Date: Tue Oct 25 21:58:48 2016

[Sync] Ensure RegisterWithBackend is only called for correct types

RegisterWithBackend prepares USS datatype for syncing by triggering creation
of ModelTypeWorker. It should only be called for type that just successfully
finished LoadModels.

BUG=655389, 658443 
R=skym@chromium.org

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

[modify] https://crrev.com/4e96ec4910903cb2a3b5e7ce27263723f2a725d9/components/sync/driver/data_type_manager_impl.cc
[modify] https://crrev.com/4e96ec4910903cb2a3b5e7ce27263723f2a725d9/components/sync/driver/data_type_manager_impl_unittest.cc

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

My understanding is that Pavel's change fixes what I described as 3., but not 2.
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 3 2016 

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

commit 9a4c395dd213a7ecbf0213924c213403aefc6a31
Author: pavely <pavely@chromium.org>
Date: Thu Nov 03 00:58:19 2016

[Sync] Make ModelTypeController::DeactivateDataType idempotent

The issue is that ModelAssociationManager::StopDatatype() always calls
DeactivateDataType before Stop. It causes DCHECK if type wasn't previously
activated.

DirectoryDataTypeController::Deactivate is already idempotent, this change is to
mirror this behavior for USS type.

Proper fix is to update controller's state during activation and not call
Deactivate when not needed, but this change is more involved.

BUG= 658443 
R=maxbogue@chromium.org

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

[modify] https://crrev.com/9a4c395dd213a7ecbf0213924c213403aefc6a31/components/sync/driver/model_type_controller.cc
[modify] https://crrev.com/9a4c395dd213a7ecbf0213924c213403aefc6a31/components/sync/driver/model_type_controller_unittest.cc

Comment 4 by pav...@chromium.org, Nov 3 2016

Owner: pav...@chromium.org
Status: Fixed (was: Available)
►

Sign in to add a comment