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

Issue 898430 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 24
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug-Regression



Sign in to add a comment

UMA histogram Sync.InitialState broken in M70+

Project Member Reported by treib@chromium.org, Oct 24

Issue description

https://crrev.com/c/1162174 changed SyncAuthManager::GetActiveAccountInfo() to only return a non-empty value after RegisterForAuthNotifications() has been called. Unfortunately, that happens just *after* we have recorded Sync.InitialState in ProfileSyncService::Initialize(). The consequence is that we'll always record "not signed in" simply because the active account hasn't been initialized yet.
 
Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 24

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

commit 58505ca736e7aff6c89e8f48904b028bdf088f62
Author: Marc Treib <treib@chromium.org>
Date: Wed Oct 24 10:43:07 2018

Fix UMA histogram Sync.InitialState

Before SyncAuthManager::RegisterForAuthNotifications(), the
active/authenticated account isn't initialized, so the histogram would
always record "not signed in". So make sure we record the histogram
only after registering for auth.

Bug:  898430 
Change-Id: I00064541cefe6cb65d5a246f12c7a8eaef469c28
Reviewed-on: https://chromium-review.googlesource.com/c/1297420
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602291}
[modify] https://crrev.com/58505ca736e7aff6c89e8f48904b028bdf088f62/components/browser_sync/profile_sync_service.cc

Labels: -Type-Bug Type-Bug-Regression
Status: Fixed (was: Started)
That should fix it. We could try to merge the fix to M71, but since it's already kinda late in the cycle for a non-critical merge, and nobody seems to urgently need the histogram data, I'm inclined to just let it roll out with M72. If anyone disagrees, let me know!

Sign in to add a comment