Eliminate usage of PO2TS::GetPrimaryAccountID() in UbertokenFetcher.

Eliminate usage of PO2TS::GetPrimaryAccountID() in UbertokenFetcher.

This change is a first step toward moving UbertokenFetcher and
GoogleAutoLoginHelper into //google_apis. Rather than UbertokenFetcher having a
variant of StartFetchingToken() wherein it uses the primary account ID,
callers must always supply the account ID. As GoogleAutoLoginHelper should
also not know about PO2TS, this CL similarly eliminates
GoogleAutoLoginHelper::Login().

BUG=330292
TBR=nyquist,mkwst,courage

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=244208

[blundell, 2014-01-09 16:44:07]

rogerta: for the whole CL
nyquist: for the Android-related part

https://codereview.chromium.org/131973004/diff/1/chrome/browser/signin/google...
File chrome/browser/signin/google_auto_login_helper.cc (left):

https://codereview.chromium.org/131973004/diff/1/chrome/browser/signin/google...
chrome/browser/signin/google_auto_login_helper.cc:27: if
(uber_token_fetcher_.get()) {
Is there anything I can do to preserve the DCHECK that was existing above for
the non-mirror case? Is there any danger that uber_token_fetcher_ will be
non-NULL in the non-mirror case?

[blundell, 2014-01-09 16:45:44]

Roger: I can wait to land this until https://codereview.chromium.org/110373007/
lands and then rebase onto it.

[Roger Tawa OOO till Jul 10th, 2014-01-10 00:08:14]

lgtm, see comment below.

My CL is going through the CQ now, so hopefully it will be committed by the time
you read this.  But don't block on it, I can merge too.

https://codereview.chromium.org/131973004/diff/1/chrome/browser/signin/google...
File chrome/browser/signin/google_auto_login_helper.cc (left):

https://codereview.chromium.org/131973004/diff/1/chrome/browser/signin/google...
chrome/browser/signin/google_auto_login_helper.cc:27: if
(uber_token_fetcher_.get()) {
On 2014/01/09 16:44:07, blundell wrote:
> Is there anything I can do to preserve the DCHECK that was existing above for
> the non-mirror case? Is there any danger that uber_token_fetcher_ will be
> non-NULL in the non-mirror case?

No need.  When I land https://codereview.chromium.org/110373007/, LogIn() will
be implemented by calling LogIn(string), so DCHECK is gone.  That DCHCK
essentially protects the reassignment to |uber_token_fetcher_|, which would
break if done concurrently with  a call to calling LogIn(string).

https://codereview.chromium.org/131973004/diff/1/chrome/browser/signin/uberto...
File chrome/browser/signin/ubertoken_fetcher_unittest.cc (right):

https://codereview.chromium.org/131973004/diff/1/chrome/browser/signin/uberto...
chrome/browser/signin/ubertoken_fetcher_unittest.cc:72:
scoped_ptr<TestingProfile> profile_;
It would be good to get rid of Profile here.  Its needed for
FakeProfileOAuth2TokenService though.

We should have a FakeOAuth2TokenService that lives in google_apis\gaia, that has
a set of IssueXXX methods just like FakeProfileOAuth2TokenService.  Then these
tests don't need to live in //chrome.  Since Mihai is refactoring those classes,
maybe coordinate with him.  I'd be OK though with committing as is, and having
that cleanup done as part of Mihai's refactor.

[blundell, 2014-01-10 09:02:02]

Thanks!

TBR'ing nyquist for //chrome/browser/android/signin
TBR'ing mkwset for //chrome/browser/ui/auto_login_infobar_delegate.cc
TBR'ing courage for
//chrome/browser/extensions/api/identity/gaia_web_auth_flow.cc

https://codereview.chromium.org/131973004/diff/1/chrome/browser/signin/uberto...
File chrome/browser/signin/ubertoken_fetcher_unittest.cc (right):

https://codereview.chromium.org/131973004/diff/1/chrome/browser/signin/uberto...
chrome/browser/signin/ubertoken_fetcher_unittest.cc:72:
scoped_ptr<TestingProfile> profile_;
Mihai and I discussed this earlier yesterday and came up with with the same plan
that you suggest. I am going to make those changes as part of moving this class
into google_apis.

On 2014/01/10 00:08:14, Roger Tawa wrote:
> It would be good to get rid of Profile here.  Its needed for
> FakeProfileOAuth2TokenService though.
> 
> We should have a FakeOAuth2TokenService that lives in google_apis\gaia, that
has
> a set of IssueXXX methods just like FakeProfileOAuth2TokenService.  Then these
> tests don't need to live in //chrome.  Since Mihai is refactoring those
classes,
> maybe coordinate with him.  I'd be OK though with committing as is, and having
> that cleanup done as part of Mihai's refactor.

[commit-bot: I haz the power, 2014-01-10 09:02:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/blundell@chromium.org/131973004/110001

[commit-bot: I haz the power, 2014-01-10 09:18:55]

Retried try job too often on linux_rel for step(s) base_unittests,
browser_tests, interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2014-01-10 16:31:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/blundell@chromium.org/131973004/400001

[commit-bot: I haz the power, 2014-01-10 18:48:54]

Change committed as 244208