New issue
Advanced search Search tips

Issue 761362 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Bug

Blocking:
issue 716053



Sign in to add a comment

Enable the account reconcilor for Dice

Project Member Reported by droger@chromium.org, Sep 1 2017

Issue description

The account reconcilor is needed for Dice, to enforce the account consistency.

There is some work to make it compatible with dice (it must not interfere with the Dice flow, removing accounts as Dice adds them), and enable it.
 
Cc: ew...@chromium.org msarda@chromium.org
Components: Services>SignIn

Comment 3 by ew...@chromium.org, Sep 6 2017

Labels: -Pri-3 OS-Linux OS-Mac OS-Windows Pri-2
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 11 2017

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

commit 59ed01b48792ce4c3a040526db5a0e1fca802b28
Author: David Roger <droger@chromium.org>
Date: Mon Sep 11 14:54:20 2017

[signin] Enable the AccountReconcilor with Dice.

This CL enables the AccountReconcilor when Dice is fully enabled:
see AccountReconcilor::IsAccountConsistencyEnabled().

To prevent conflicts between Dice and the account reconcilor, a
new mechanism to block (suspend) the reconcilor is introduced:
AccountReconcilor::Lock. The reconcilor is blocked as long as one
instance of this class is alive.

The blocking starts when the Dice request header is sent, and ends
when the new token has been added to the token service. This is
implemented by using three instances of AccountReconcilor::Lock.
- attached to the the net::URLRequest that carries signin cookies,
- attached to the net::Request that carries Dice headers,
- attached to the DiceTokenFetcher.

The lifetimes of these locks overlap, and so the account reconcilor
is blocked and unblocked only once.

Tests have been added to ensure that the AccountReconcilor is
correctly blocked and unblocked.

Bug:  761362 
Change-Id: I5564364516540554dfb4741fd9d3998d84507b9d
Reviewed-on: https://chromium-review.googlesource.com/647749
Commit-Queue: David Roger <droger@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500914}
[modify] https://crrev.com/59ed01b48792ce4c3a040526db5a0e1fca802b28/chrome/browser/signin/account_reconcilor_unittest.cc
[modify] https://crrev.com/59ed01b48792ce4c3a040526db5a0e1fca802b28/chrome/browser/signin/chrome_signin_helper.cc
[modify] https://crrev.com/59ed01b48792ce4c3a040526db5a0e1fca802b28/chrome/browser/signin/chrome_signin_helper.h
[modify] https://crrev.com/59ed01b48792ce4c3a040526db5a0e1fca802b28/chrome/browser/signin/dice_browsertest.cc
[modify] https://crrev.com/59ed01b48792ce4c3a040526db5a0e1fca802b28/chrome/browser/signin/dice_response_handler.cc
[modify] https://crrev.com/59ed01b48792ce4c3a040526db5a0e1fca802b28/chrome/browser/signin/dice_response_handler.h
[modify] https://crrev.com/59ed01b48792ce4c3a040526db5a0e1fca802b28/chrome/browser/signin/dice_response_handler_unittest.cc
[modify] https://crrev.com/59ed01b48792ce4c3a040526db5a0e1fca802b28/components/signin/core/browser/account_reconcilor.cc
[modify] https://crrev.com/59ed01b48792ce4c3a040526db5a0e1fca802b28/components/signin/core/browser/account_reconcilor.h
[modify] https://crrev.com/59ed01b48792ce4c3a040526db5a0e1fca802b28/components/signin/core/browser/signin_header_helper.cc
[modify] https://crrev.com/59ed01b48792ce4c3a040526db5a0e1fca802b28/components/signin/core/browser/signin_header_helper.h
[modify] https://crrev.com/59ed01b48792ce4c3a040526db5a0e1fca802b28/components/signin/core/browser/signin_header_helper_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 11 2017

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

commit 31d5543745f242f1872345a66130a26ffbb3d52a
Author: David Roger <droger@chromium.org>
Date: Mon Sep 11 15:55:44 2017

[signin] Simplify dice_browsertest and add reconcilor check

This CL replaces the call to domAutomationController from javascript by
a callback on the embedded HTTP server.
This simplifies the code, and allows for an additional check that the
reconcilor is blocked while the Dice request is made.

Bug:  761362 
Change-Id: I57ebe5ff395a41abed56ab3acd2809ed1d8c453a
Reviewed-on: https://chromium-review.googlesource.com/652474
Commit-Queue: David Roger <droger@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500926}
[modify] https://crrev.com/31d5543745f242f1872345a66130a26ffbb3d52a/chrome/browser/signin/dice_browsertest.cc

Comment 7 by droger@chromium.org, Sep 19 2017

Status: Fixed (was: Started)
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 22 2017

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

commit 2c9bd69f9017b561d74063d55e072a9c9a031c36
Author: David Roger <droger@chromium.org>
Date: Fri Sep 22 13:01:31 2017

[Signin] Add test table for Dice in AccountReconcilor

Bug:  761362 
Change-Id: I19644a8087c1ab775e3910afd8e774c12a2aa192
Reviewed-on: https://chromium-review.googlesource.com/675370
Commit-Queue: David Roger <droger@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503722}
[modify] https://crrev.com/2c9bd69f9017b561d74063d55e072a9c9a031c36/chrome/browser/signin/account_reconcilor_unittest.cc
[modify] https://crrev.com/2c9bd69f9017b561d74063d55e072a9c9a031c36/components/signin/core/browser/account_reconcilor.h

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 5 2017

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

commit d930ca3459daf30b76e9dc594136a582c7f007c4
Author: David Roger <droger@chromium.org>
Date: Thu Oct 05 17:11:09 2017

[Signin] Fix edge cases for the AccountReconcilor and add more tests

Bug:  761362 
Change-Id: Ib0cbc54e99bcde0be88317a87eeb903136128978
Reviewed-on: https://chromium-review.googlesource.com/691662
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506774}
[modify] https://crrev.com/d930ca3459daf30b76e9dc594136a582c7f007c4/chrome/browser/signin/account_reconcilor_unittest.cc
[modify] https://crrev.com/d930ca3459daf30b76e9dc594136a582c7f007c4/components/signin/core/browser/account_reconcilor.cc

Sign in to add a comment