Extract LoginPerformer to chromeos/auth

Extract LoginPerformer to chromeos/auth

BUG=387613
R=nkostylev@chromium.org

Committed: https://crrev.com/2ec6339f32455ab40a9ada24ee8a38e396dca9fb
Cr-Commit-Position: refs/heads/master@{#300489}

[Nikita (slow), 2014-10-02 12:38:43]

I suggest experimenting with git cl upload
--similarity=xx parameter.

Decrease xx value (default 50) so that login_performer.cc and (maybe)
chrome_login_performer.cc will have their status shown as A+ and not just pure
adding A.

[Nikita (slow), 2014-10-06 14:55:33]

https://codereview.chromium.org/614973002/diff/80001/chromeos/login/auth/logi...
File chromeos/login/auth/login_performer.cc (right):

https://codereview.chromium.org/614973002/diff/80001/chromeos/login/auth/logi...
chromeos/login/auth/login_performer.cc:143: return;
nit: Add {}

https://codereview.chromium.org/614973002/diff/80001/chromeos/login/auth/logi...
chromeos/login/auth/login_performer.cc:179: return;
nit: Add {}

https://codereview.chromium.org/614973002/diff/80001/chromeos/login/auth/logi...
chromeos/login/auth/login_performer.cc:266: if (extended_authenticator_.get()) {
nit: drop {}

https://codereview.chromium.org/614973002/diff/80001/chromeos/login/auth/logi...
chromeos/login/auth/login_performer.cc:275: Profile* profile =
GetSigninProfile();
You should not use Profile in this base class, use BrowserContext instead.

https://codereview.chromium.org/614973002/diff/80001/chromeos/login/auth/logi...
File chromeos/login/auth/login_performer.h (right):

https://codereview.chromium.org/614973002/diff/80001/chromeos/login/auth/logi...
chromeos/login/auth/login_performer.h:30: class Profile;
use BrowserContext instead.

[Nikita (slow), 2014-10-06 15:02:31]

https://codereview.chromium.org/614973002/diff/80001/chromeos/login/auth/logi...
File chromeos/login/auth/login_performer.h (right):

https://codereview.chromium.org/614973002/diff/80001/chromeos/login/auth/logi...
chromeos/login/auth/login_performer.h:124: virtual void OnChecked(const
std::string& username, bool success) OVERRIDE;
nit: username > user_id here and through this class.

https://codereview.chromium.org/614973002/diff/80001/chromeos/login/auth/logi...
chromeos/login/auth/login_performer.h:126: // Platform-dependant methods do be
implemented by concrete class.
nit: to be

https://codereview.chromium.org/614973002/diff/80001/chromeos/login/auth/logi...
chromeos/login/auth/login_performer.h:128: // Run trusted check for platform. If
trusted check have to be performed
nit: for a platform

https://codereview.chromium.org/614973002/diff/80001/chromeos/login/auth/logi...
chromeos/login/auth/login_performer.h:129: // asynchronously,
nit: please combine with the next line
looks strange when there's only single word on a line.

https://codereview.chromium.org/614973002/diff/80001/chromeos/login/auth/logi...
chromeos/login/auth/login_performer.h:132: // called upon actual check.
nit: last two lines may be combined

https://codereview.chromium.org/614973002/diff/80001/chromeos/login/auth/logi...
chromeos/login/auth/login_performer.h:134: // Check if user is allowed to sign
in on device. |wildcard_match| will
nit: insert extra empty line before comment.

https://codereview.chromium.org/614973002/diff/80001/chromeos/login/auth/logi...
chromeos/login/auth/login_performer.h:135: // contain
nit: Please combine with the next line.

https://codereview.chromium.org/614973002/diff/80001/chromeos/login/auth/logi...
chromeos/login/auth/login_performer.h:137: // be relevant for
nit: combine two last lines.

https://codereview.chromium.org/614973002/diff/80001/chromeos/login/auth/logi...
chromeos/login/auth/login_performer.h:141: // This method should run addional
online check if user can sign in on device.
nit: Insert empty line before comment.

https://codereview.chromium.org/614973002/diff/80001/chromeos/login/auth/logi...
chromeos/login/auth/login_performer.h:165: // Run policy check for user. If
something is wrong, delegate's
nit: for |user_id|

https://codereview.chromium.org/614973002/diff/80001/chromeos/login/auth/logi...
chromeos/login/auth/login_performer.h:167: virtual bool CheckPolicyForUser(const
std::string& username) = 0;
nit: username > user_id

https://codereview.chromium.org/614973002/diff/80001/chromeos/login/auth/logi...
chromeos/login/auth/login_performer.h:170: virtual Profile* GetSigninProfile() =
0;
nit: Should not use Profile here thus rename method.

https://codereview.chromium.org/614973002/diff/80001/chromeos/login/auth/logi...
chromeos/login/auth/login_performer.h:172: // Get implementation of
RequestContext.
nit: Get RequestContext used for sign in.

[Nikita (slow), 2014-10-06 15:12:35]

https://codereview.chromium.org/614973002/diff/80001/chrome/browser/chromeos/...
File chrome/browser/chromeos/login/auth/chrome_login_performer.cc (right):

https://codereview.chromium.org/614973002/diff/80001/chrome/browser/chromeos/...
chrome/browser/chromeos/login/auth/chrome_login_performer.cc:54: callback.Run();
Please place last two lines of this method into else method with a comment (and
DCHECK)
// CrosSettingsProvider::TRUSTED

https://codereview.chromium.org/614973002/diff/80001/chrome/browser/chromeos/...
chrome/browser/chromeos/login/auth/chrome_login_performer.cc:77: callback.Run();
nit: Same here, place in else block with comment CrosSettingsProvider::TRUSTED

https://codereview.chromium.org/614973002/diff/80001/chrome/browser/chromeos/...
File chrome/browser/chromeos/login/auth/chrome_login_performer.h (right):

https://codereview.chromium.org/614973002/diff/80001/chrome/browser/chromeos/...
chrome/browser/chromeos/login/auth/chrome_login_performer.h:30: // This class
encapsulates sign in operations.
nit: Comment needs to be updated too.

[Denis Kuznetsov (DE-MUC), 2014-10-17 19:46:57]

addressed all comments, ptal.

[Nikita (slow), 2014-10-20 12:30:28]

lgtm

https://codereview.chromium.org/614973002/diff/120001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/auth/chrome_login_performer.cc (right):

https://codereview.chromium.org/614973002/diff/120001/chrome/browser/chromeos...
chrome/browser/chromeos/login/auth/chrome_login_performer.cc:54: //
CrosSettingsProvider::TRUSTED
nit: add DCHECK instead // just in case this enum ever gets new values.

https://codereview.chromium.org/614973002/diff/120001/chrome/browser/chromeos...
chrome/browser/chromeos/login/auth/chrome_login_performer.cc:79: //
CrosSettingsProvider::TRUSTED
nit: Add DCHECK instead.

https://codereview.chromium.org/614973002/diff/120001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/session/user_session_manager.cc (right):

https://codereview.chromium.org/614973002/diff/120001/chrome/browser/chromeos...
chrome/browser/chromeos/login/session/user_session_manager.cc:783: // the
authentication states in the login profile and the user's profile
nit: nit: login profile > auth context

https://codereview.chromium.org/614973002/diff/120001/chrome/browser/chromeos...
chrome/browser/chromeos/login/session/user_session_manager.cc:905: bool
restore_from_auth_cookies) {
nit: Please fix indentation (I known it's not your code).

https://codereview.chromium.org/614973002/diff/120001/chromeos/login/auth/aut...
File chromeos/login/auth/authenticator.h (right):

https://codereview.chromium.org/614973002/diff/120001/chromeos/login/auth/aut...
chromeos/login/auth/authenticator.h:36: virtual void
CompleteLogin(content::BrowserContext* profile,
nit: profile > browser_context

https://codereview.chromium.org/614973002/diff/120001/chromeos/login/auth/aut...
chromeos/login/auth/authenticator.h:42: virtual void
AuthenticateToLogin(content::BrowserContext* profile,
nit: profile > browser_context

https://codereview.chromium.org/614973002/diff/120001/chromeos/login/auth/aut...
chromeos/login/auth/authenticator.h:91: // BrowserContext (usually off the
record ) that was used to perform the last
nit: drop empty space before )

https://codereview.chromium.org/614973002/diff/120001/chromeos/login/auth/log...
File chromeos/login/auth/login_performer.h (right):

https://codereview.chromium.org/614973002/diff/120001/chromeos/login/auth/log...
chromeos/login/auth/login_performer.h:169: // Look up profile to use during
signin.
nit: profile > browser context

[Denis Kuznetsov (DE-MUC), 2014-10-21 14:02:25]

The CQ bit was checked by antrim@chromium.org

[commit-bot: I haz the power, 2014-10-21 14:03:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/614973002/140001

[commit-bot: I haz the power, 2014-10-21 15:12:39]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2014-10-21 15:13:22]

Patchset 8 (id:??) landed as
https://crrev.com/2ec6339f32455ab40a9ada24ee8a38e396dca9fb
Cr-Commit-Position: refs/heads/master@{#300489}

[Nikita (slow), 2014-10-25 19:12:14]

A revert of this CL (patchset #8 id:140001) has been created in
https://codereview.chromium.org/681593003/ by nkostylev@chromium.org.

The reason for reverting is: Introduces new crashes: crbug.com/426522.