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

Issue 889859 link

Starred by 1 user
Status: Fixed
Owner:
treib@chromium.org
Closed: Sep 28
Cc:
mastiz@chromium.org
█ tschumann@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

chrome://sync-internals sometimes shows "Next Token Request" time in the past

Project Member Reported by treib@chromium.org, Sep 27 
We've had a bunch of reports recently where the "Next Token Request" time shown on chrome://sync-internals is actually in the past. This probably means that there are some cases where SyncAuthManager doesn't clear this time if a scheduled request gets either executed or canceled.
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 27 

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

commit 140fb2db61af25971770256d61607c27fe1f2f8b
Author: Marc Treib <treib@chromium.org>
Date: Thu Sep 27 14:27:35 2018

SyncAuthManager: make sure next_token_request_time gets cleared

This fixes one (small) issue where the next_token_request_time didn't
get cleared if the retry timer called us while there was already a
request ongoing. It also adds a bunch of DCHECKs to hopefully catch
any other cases I might have missed.
Inspired by some recent bug reports that showed a "Next Token Request"
time in the past on about:sync-internals.

Bug:  889859 
Change-Id: I8cd189c0e4acccc4af0d38a3ce9da6ccbd3cac7a
Reviewed-on: https://chromium-review.googlesource.com/1249067
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594714}
[modify] https://crrev.com/140fb2db61af25971770256d61607c27fe1f2f8b/components/browser_sync/sync_auth_manager.cc
[modify] https://crrev.com/140fb2db61af25971770256d61607c27fe1f2f8b/components/browser_sync/sync_auth_manager.h
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 28 

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

commit 45c596b9e361b76a2712fd21bcd08c48654cd968
Author: Marc Treib <treib@chromium.org>
Date: Fri Sep 28 14:52:42 2018

SyncAuthManager: compute next_token_request_time on demand

next_token_request_time is always supposed to be equal to the scheduled
run time of request_access_token_retry_timer_. Instead of keeping this
redundant state (and risk it getting out of sync), let's just compute
next_token_request_time when we need it.

Bug:  889859 
Change-Id: I2b47d5dd78ef55bbfe1c0b43657cf9125e3f4586
Reviewed-on: https://chromium-review.googlesource.com/1250927
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595090}
[modify] https://crrev.com/45c596b9e361b76a2712fd21bcd08c48654cd968/components/browser_sync/sync_auth_manager.cc
[modify] https://crrev.com/45c596b9e361b76a2712fd21bcd08c48654cd968/components/browser_sync/sync_auth_manager.h

Comment 3 by treib@chromium.org, Sep 28

Status: Fixed (was: Started)
►

Sign in to add a comment