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

Issue 898453 link

Starred by 1 user
Status: Fixed
Owner:
mastiz@chromium.org
Closed: Oct 24
Cc:
mastiz@chromium.org
█ tschumann@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

USS datatypes may prevent sync configuration from completing

Project Member Reported by mastiz@chromium.org, Oct 24 
If a USS  datatype (using ModelTypeController) enters an error state (FAILED), it prevents future sync reconfigurations from completing, leading to weird behavior.

This means DataTypeManagerImpl::OnModelAssociationDone() will never be exercised, where there's non-trivial logic, including notifications to the UI and logging of UMA metrics (which could introduce biases in histograms measuring errors).

As an example of this behavior, custom passphrase encryption may never start or be considered complete, which was surfaced in integration tests.
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 24 

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

commit 409ffd3edd8afc17d67b1eb6736793e0805aded7
Author: Mikel Astiz <mastiz@chromium.org>
Date: Wed Oct 24 11:42:25 2018

Fix missing USS controller stop completion

DataTypeManager is allowed to call Stop() while a datatype is
NOT_RUNNING or FAILED. In both cases, the function should be a no-op,
but the completion callback should be called.

Prior to this patch, the completion callback would NOT be run and hence
future sync reconfigurations may never complete, leading to weird
behavior including UI implications (missing updates), UMA metric
biases (error-related scenarios may be underrepresented) and issues
related custom passphrase encryption (which may never start or never
complete, observed in tests and the reason why this bug got surfaced).

Bug:  898453 , 870624 
Change-Id: I47a9c04ab34ac48e0fdf000df2acb70bbf3e4446
Reviewed-on: https://chromium-review.googlesource.com/c/1297419
Reviewed-by: Jan Krcal <jkrcal@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602300}
[modify] https://crrev.com/409ffd3edd8afc17d67b1eb6736793e0805aded7/components/sync/driver/model_type_controller.cc
[modify] https://crrev.com/409ffd3edd8afc17d67b1eb6736793e0805aded7/components/sync/driver/model_type_controller_unittest.cc

Comment 2 by mastiz@chromium.org, Oct 24

Status: Fixed (was: Started)
►

Sign in to add a comment