Componentize SigninManager.

Componentize SigninManager.

This CL does the following:

- Componentizes SigninIdAccountHelper, on which SigninManager depends
- Abstracts SigninManager's (embedder-level) calculation of whether to merge
  signin credentials into the cookie jar via SigninClient
- Eliminates the |profile| argument of SigninManagerBase::Initialize() now that
  SigninManager no longer needs it
- Componentizes SigninManager

BUG=334209, 333998
TBR=thakis

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

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

[blundell, 2014-03-31 07:50:26]

The only meaningful changes in this CL are in //chrome/browser/signin,
//components/signin*, and chrome_browser.gypi. The remaining changes are as
follows:

- Mechanically-generated updates of include paths.
- Removals of the |profile| argument in calls to
SigninManager(Base)::Initialize().

droger: for review.
rogerta: fyi.

[droger, 2014-03-31 09:13:20]

https://codereview.chromium.org/218713002/diff/20001/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

https://codereview.chromium.org/218713002/diff/20001/chrome/chrome_browser.gy...
chrome/chrome_browser.gypi:3093: 'signin/core/browser/signin_manager.cc',
This seems wrong.

https://codereview.chromium.org/218713002/diff/20001/components/signin/core/b...
File components/signin/core/browser/signin_client.h (right):

https://codereview.chromium.org/218713002/diff/20001/components/signin/core/b...
components/signin/core/browser/signin_client.h:38: virtual bool
MergeSigninCredentialsIntoCookieJar() = 0;
The function is named like it would actually merge the credentials.
But according to the documentation and implementation, it does not merge them,
but only tells wether they should be merged. Maybe we could find a better name
for this method?

[blundell, 2014-03-31 11:56:50]

Thanks!

https://codereview.chromium.org/218713002/diff/20001/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

https://codereview.chromium.org/218713002/diff/20001/chrome/chrome_browser.gy...
chrome/chrome_browser.gypi:3093: 'signin/core/browser/signin_manager.cc',

Yes, thanks. Moved the exclusion to //components/signin.gypi.

On 2014/03/31 09:13:21, droger wrote:
> This seems wrong.

https://codereview.chromium.org/218713002/diff/20001/components/signin/core/b...
File components/signin/core/browser/signin_client.h (right):

https://codereview.chromium.org/218713002/diff/20001/components/signin/core/b...
components/signin/core/browser/signin_client.h:38: virtual bool
MergeSigninCredentialsIntoCookieJar() = 0;
Good catch. Done. (ShouldMergeSigninCredentialsIntoCookieJar).

On 2014/03/31 09:13:21, droger wrote:
> The function is named like it would actually merge the credentials.
> But according to the documentation and implementation, it does not merge them,
> but only tells wether they should be merged. Maybe we could find a better name
> for this method?

[droger, 2014-03-31 15:08:27]

lgtm

[blundell, 2014-03-31 16:42:29]

TBR=thakis for //chrome changes outside of //chrome/browser/signin

[blundell, 2014-03-31 16:42:37]

The CQ bit was checked by [email protected]

[commit-bot: I haz the power, 2014-03-31 16:44:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/[email protected]/218713002/80001

[commit-bot: I haz the power, 2014-03-31 20:35:08]

Change committed as 260636

[dewittj, 2014-03-31 20:49:02]

A revert of this CL has been created in
https://codereview.chromium.org/218613008/ by [email protected].

The reason for reverting is: Believed to have caused
http://build.chromium.org/p/chromium.memory/builders/Linux%20ASan%2BLSan%20Bu...
to fail.

../../chrome/browser/profiles/gaia_info_update_service.h:14:10: fatal error:
'chrome/browser/signin/signin_manager.h' file not found
#include "chrome/browser/signin/signin_manager.h".

[blundell, 2014-03-31 21:04:43]

Relanding with a fix for an include of SigninManager in its old location that
landed in a race with this CL going through the CQ and caused this CL to be
reverted after landing. The include path change is in
chrome/browser/profiles/gaia_info_update_service.h.

[blundell, 2014-03-31 21:04:52]

The CQ bit was checked by [email protected]

[commit-bot: I haz the power, 2014-03-31 21:06:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/[email protected]/218713002/100001

[commit-bot: I haz the power, 2014-03-31 21:07:22]

The CQ bit was unchecked by [email protected]

[commit-bot: I haz the power, 2014-03-31 21:07:23]

Failed to apply patch for chrome/browser/profiles/gaia_info_update_service.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/profiles/gaia_info_update_service.h
  Hunk #1 FAILED at 11.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/browser/profiles/gaia_info_update_service.h.rej

Patch:       chrome/browser/profiles/gaia_info_update_service.h
Index: chrome/browser/profiles/gaia_info_update_service.h
diff --git a/chrome/browser/profiles/gaia_info_update_service.h
b/chrome/browser/profiles/gaia_info_update_service.h
index
ab65137601da060fd3e727cf5c28f367e638a84d..531241ffc8a1e6ba71e20219e60441ed4af0f84a
100644
--- a/chrome/browser/profiles/gaia_info_update_service.h
+++ b/chrome/browser/profiles/gaia_info_update_service.h
@@ -11,8 +11,8 @@
 #include "base/timer/timer.h"
 #include "chrome/browser/profiles/profile_downloader.h"
 #include "chrome/browser/profiles/profile_downloader_delegate.h"
-#include "chrome/browser/signin/signin_manager.h"
 #include "components/keyed_service/core/keyed_service.h"
+#include "components/signin/core/browser/signin_manager.h"
 
 class Profile;
 class ProfileDownloader;

[blundell, 2014-03-31 21:22:23]

The CQ bit was checked by [email protected]

[commit-bot: I haz the power, 2014-03-31 21:24:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/[email protected]/218713002/110001

[dewittj, 2014-03-31 21:30:07]

This conflicted with r260630 and ultimately both got reverted in parallel.
 It should be safe to land so long as one or the other hits first.

Sorry for the inconvenience!

Justin


On Mon, Mar 31, 2014 at 1:49 PM, <[email protected]> wrote:

> A revert of this CL has been created in
> https://codereview.chromium.org/218613008/ by [email protected].
>
> The reason for reverting is: Believed to have caused
> http://build.chromium.org/p/chromium.memory/builders/
> Linux%20ASan%2BLSan%20Builder/builds/2113
> to fail.
>
> ../../chrome/browser/profiles/gaia_info_update_service.h:14:10: fatal
> error:
> 'chrome/browser/signin/signin_manager.h' file not found
> #include "chrome/browser/signin/signin_manager.h".
>
> https://codereview.chromium.org/218713002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].

[blundell1, 2014-03-31 21:32:35]

Mine is going through the CQ again now. This is the second time in a week
I've had a CL land through the CQ and have a race condition with another CL
:\.


On Mon, Mar 31, 2014 at 11:30 PM, Justin Dewitt <[email protected]>wrote:

> This conflicted with r260630 and ultimately both got reverted in parallel.
>  It should be safe to land so long as one or the other hits first.
>
> Sorry for the inconvenience!
>
> Justin
>
>
> On Mon, Mar 31, 2014 at 1:49 PM, <[email protected]> wrote:
>
>> A revert of this CL has been created in
>> https://codereview.chromium.org/218613008/ by [email protected].
>>
>> The reason for reverting is: Believed to have caused
>> http://build.chromium.org/p/chromium.memory/builders/
>> Linux%20ASan%2BLSan%20Builder/builds/2113
>> to fail.
>>
>> ../../chrome/browser/profiles/gaia_info_update_service.h:14:10: fatal
>> error:
>> 'chrome/browser/signin/signin_manager.h' file not found
>> #include "chrome/browser/signin/signin_manager.h".
>>
>> https://codereview.chromium.org/218713002/
>>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].

[commit-bot: I haz the power, 2014-03-31 22:15:57]

The CQ bit was unchecked by [email protected]

[commit-bot: I haz the power, 2014-03-31 22:15:57]

Try jobs failed on following builders:
  tryserver.chromium on mac_chromium_rel

[blundell, 2014-04-01 05:51:28]

The CQ bit was checked by [email protected]

[commit-bot: I haz the power, 2014-04-01 05:51:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/[email protected]/218713002/110001

[commit-bot: I haz the power, 2014-04-01 06:17:14]

The CQ bit was unchecked by [email protected]

[commit-bot: I haz the power, 2014-04-01 06:17:15]

Try jobs failed on following builders:
  tryserver.chromium on mac_chromium_rel

[blundell, 2014-04-01 06:29:47]

The CQ bit was checked by [email protected]

[commit-bot: I haz the power, 2014-04-01 06:30:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/[email protected]/218713002/110001

[commit-bot: I haz the power, 2014-04-01 11:19:21]

Change committed as 260831