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

Issue 768949 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

ChromiumOS sign-in browser_tests failing in debug builds

Project Member Reported by michae...@chromium.org, Sep 26 2017

Issue description

These tests are failing:

SAMLPolicyTest.SAMLNoLimit
UserCloudPolicyManagerTest.StartSession
SAMLPolicyTest.NoSAML
BootstrapTest.Basic

First failing build: https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29/builds/30568

Suspecting https://chromium.googlesource.com/chromium/src/+/b29b6884019c4c7b1bc0e074617d0550b75164fc from changelists.
 
Cc: achuith@chromium.org
Confirmed locally. Note that this only repros in debug builds.

Sending a revert now for https://chromium-review.googlesource.com/658378.
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 26 2017

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

commit 849942232cf7a1c3548d8fbb826d1bc0c317fad5
Author: Michael Giuffrida <michaelpg@chromium.org>
Date: Tue Sep 26 19:09:19 2017

Revert "cros: AuthSyncObserver observes SigninErrorController"

This reverts commit b29b6884019c4c7b1bc0e074617d0550b75164fc.

Reason for revert: CrOS browser_tests failing in debug builds
Bug:  768949 

Original change's description:
> cros: AuthSyncObserver observes SigninErrorController
> 
> - Make AuthSyncObserver observe SigninErrorController in addition
>   to sync service so that auth errors caused by other services
>   are caught as well.
> - Fix an edge case that OAuth2LoginManager marks valid oauth2
>   token when it thinks /MergeSession succeeds regardless of
>   whether there is any auth error;
> 
> Bug:  760610 
> Test: OAuth2Test.SetInvalidTokenStatus
> 
> Change-Id: Idbaee9f7faa1aa2aff939a1540bfcfdd9f49c96c
> Reviewed-on: https://chromium-review.googlesource.com/658378
> Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
> Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#504391}

TBR=xiyuan@chromium.org,achuith@chromium.org

Change-Id: If3421269787c16081e1fd00382e5d92f73d8f082
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  760610 
Reviewed-on: https://chromium-review.googlesource.com/685398
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504447}
[modify] https://crrev.com/849942232cf7a1c3548d8fbb826d1bc0c317fad5/chrome/browser/chromeos/login/session/user_session_manager.cc
[modify] https://crrev.com/849942232cf7a1c3548d8fbb826d1bc0c317fad5/chrome/browser/chromeos/login/signin/auth_sync_observer.cc
[modify] https://crrev.com/849942232cf7a1c3548d8fbb826d1bc0c317fad5/chrome/browser/chromeos/login/signin/auth_sync_observer.h
[modify] https://crrev.com/849942232cf7a1c3548d8fbb826d1bc0c317fad5/chrome/browser/chromeos/login/signin/auth_sync_observer_factory.cc
[modify] https://crrev.com/849942232cf7a1c3548d8fbb826d1bc0c317fad5/chrome/browser/chromeos/login/signin/oauth2_browsertest.cc
[modify] https://crrev.com/849942232cf7a1c3548d8fbb826d1bc0c317fad5/chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Sep 29 2017

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

commit 17f47a1f6763a2e1af29c97f10832276f40abcce
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Fri Sep 29 16:06:36 2017

Reland "cros: AuthSyncObserver observes SigninErrorController"

- Make AuthSyncObserver observe SigninErrorController in addition
  to sync service so that auth errors caused by other services
  are caught as well.
- Fix an edge case that OAuth2LoginManager marks valid oauth2
  token when it thinks /MergeSession succeeds regardless of
  whether there is any auth error;

Additional changes of reland:
- Update FakeGaia to mint tokens for any scopes when there is
  an AccessTokenInfo with no |scopes| defined;
- Update the failed tests to use that so that all auth requests
  succeed and AuthSyncObserver does not set invalid token status
  unexpectedly;

Bug:  760610 ,  768949 
Test: OAuth2Test.SetInvalidTokenStatus
Change-Id: I8173ceec02ba036820439b7a66f09cac40f27d3d
Reviewed-on: https://chromium-review.googlesource.com/685795
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505381}
[modify] https://crrev.com/17f47a1f6763a2e1af29c97f10832276f40abcce/chrome/browser/chromeos/login/saml/saml_browsertest.cc
[modify] https://crrev.com/17f47a1f6763a2e1af29c97f10832276f40abcce/chrome/browser/chromeos/login/session/user_session_manager.cc
[modify] https://crrev.com/17f47a1f6763a2e1af29c97f10832276f40abcce/chrome/browser/chromeos/login/signin/auth_sync_observer.cc
[modify] https://crrev.com/17f47a1f6763a2e1af29c97f10832276f40abcce/chrome/browser/chromeos/login/signin/auth_sync_observer.h
[modify] https://crrev.com/17f47a1f6763a2e1af29c97f10832276f40abcce/chrome/browser/chromeos/login/signin/auth_sync_observer_factory.cc
[modify] https://crrev.com/17f47a1f6763a2e1af29c97f10832276f40abcce/chrome/browser/chromeos/login/signin/oauth2_browsertest.cc
[modify] https://crrev.com/17f47a1f6763a2e1af29c97f10832276f40abcce/chrome/browser/chromeos/login/test/oobe_base_test.cc
[modify] https://crrev.com/17f47a1f6763a2e1af29c97f10832276f40abcce/chrome/browser/chromeos/login/test/oobe_base_test.h
[modify] https://crrev.com/17f47a1f6763a2e1af29c97f10832276f40abcce/chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc
[modify] https://crrev.com/17f47a1f6763a2e1af29c97f10832276f40abcce/chrome/browser/chromeos/policy/login_policy_test_base.cc
[modify] https://crrev.com/17f47a1f6763a2e1af29c97f10832276f40abcce/google_apis/gaia/fake_gaia.cc
[modify] https://crrev.com/17f47a1f6763a2e1af29c97f10832276f40abcce/google_apis/gaia/fake_gaia.h

Comment 4 by xiy...@chromium.org, Sep 29 2017

Status: Fixed (was: Assigned)

Comment 5 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 6 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)

Sign in to add a comment