New issue
Advanced search Search tips

Issue 753615 link

Starred by 0 users
Status: Started
Owner:
achuith@chromium.org
Cc:
xiy...@chromium.org
pkasting@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

oauth2_browsertest BrowserThreadBlocker needs to be rewritten

Project Member Reported by achuith@chromium.org, Aug 9 2017 
BrowserThreadBlocker blocks waiting for the DB thread here:
https://cs.chromium.org/chromium/src/chrome/browser/chromeos/login/signin/oauth2_browsertest.cc?l=561

The class is defined here:
https://cs.chromium.org/chromium/src/chrome/browser/chromeos/login/signin/oauth2_browsertest.cc?l=145-166

This class was introduced here:
https://codereview.chromium.org/2668043002/diff/80001/chrome/browser/chromeos/login/signin/oauth2_browsertest.cc

DB thread was replaced with an anonymous blocking thread in this CL:
https://chromium-review.googlesource.com/c/566181 

Rather curiously, the test continues to run successfully even though the synchronization is not longer correct. This may just be lucky timing.

Comment 1 by xiy...@chromium.org, Aug 9 2017 

Not sure how the test still passes with the webdata service changes to use post task API.

What BrowserThreadBlocker does is to run WaitableEvent::Wait on a specific thread to block it and allow let-go when the event is signaled. To replace that, we need a way to run WaitableEvent::Wait on the db task runner.

How about adding a WebDataServiceWrapper::PostDbTaskForTest that schedules a task on the db task runner?
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 22 2017 

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

commit ab3fcd8a1d471d349d91492786dfdc6a62b1e8ee
Author: Achuith Bhandarkar <achuith@chromium.org>
Date: Tue Aug 22 21:07:22 2017

Remove BrowserThread::ID from oauth2_browsertest.

BUG=chromium:753615
TEST=compiles

Change-Id: I4a830be764cebe7a7e25b1dc5c6b7d02ddf96969
Reviewed-on: https://chromium-review.googlesource.com/624570
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Achuith Bhandarkar <achuith@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496441}
[modify] https://crrev.com/ab3fcd8a1d471d349d91492786dfdc6a62b1e8ee/chrome/browser/chromeos/login/signin/oauth2_browsertest.cc

Comment 4 by achuith@chromium.org, Aug 23 2017

Status: Started (was: Untriaged)
https://chromium-review.googlesource.com/c/chromium/src/+/629418

Comment 5 by zalcorn@chromium.org, Sep 28 

Triage nag: This Chrome OS bug has an owner but no component. Please add a component so that this can be tracked by the relevant team.

Comment 6 by pkasting@chromium.org, Sep 28

Components: Services>SignIn
►

Sign in to add a comment