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

Issue 913984 link

Starred by 1 user
Status: Started
Owner:
treib@chromium.org
Cc:
mastiz@chromium.org
jkrcal@chromium.org
ios-bugs@chromium.org
█ tschumann@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac
Pri: 3
Type: Task



Sign in to add a comment

SyncService::GetActiveDataTypes should maybe handle persistent auth errors

Project Member Reported by treib@chromium.org, Dec 11 
If there is a persistent auth error (in particular during the "Sync paused" state), then Sync isn't quite active, but GetActiveDataTypes() will still report all types as active. We should evaluate if it'd be better/safer to report "no active types" in this state.
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 16 (6 days ago) 

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

commit afab2e5e80bd6db6d6723f08b6e3ab531948537e
Author: Marc Treib <treib@chromium.org>
Date: Wed Jan 16 10:54:33 2019

SyncService::GetActiveDataTypes: return "none" if there is a persistent auth error

This seems like the safer (and more accurate) interpretation of "active".
In particular, this will affect the "Sync paused" state, for which we previously
claimed that all data types were active.

Bug: 913984
Change-Id: Iea5b0fd113cfc2d12b7173205302abb8fa862cc7
Reviewed-on: https://chromium-review.googlesource.com/c/1402877
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623190}
[modify] https://crrev.com/afab2e5e80bd6db6d6723f08b6e3ab531948537e/chrome/browser/sync/test/integration/profile_sync_service_harness.cc
[modify] https://crrev.com/afab2e5e80bd6db6d6723f08b6e3ab531948537e/chrome/browser/sync/test/integration/profile_sync_service_harness.h
[modify] https://crrev.com/afab2e5e80bd6db6d6723f08b6e3ab531948537e/chrome/browser/sync/test/integration/sync_auth_test.cc
[modify] https://crrev.com/afab2e5e80bd6db6d6723f08b6e3ab531948537e/components/browser_sync/profile_sync_service.cc

Comment 2 by treib@chromium.org, Jan 16 (6 days ago)

Status: Started (was: Assigned)
This is now done as specified. Note that IsSyncFeatureActive will still return true in the presence of auth errors, so this is a different interpretation of "active". (Note that it was already inconsistent with TransportState::ACTIVE...)

Maybe GetActiveDataTypes could be rename to GetSyncingDataTypes or something like that?
►

Sign in to add a comment