Implement new password separated sign in flow for chrome desktop.

Implement new password separated sign in flow for chrome.
This is currently behind a flag to allow chrome and GAIA UX folks to play
with the feature.

BUG=533004
Committed: https://crrev.com/fb8dc62b48a488f1dcd65948e6b34316c5c3c257
Cr-Commit-Position: refs/heads/master@{#352598}

[Roger Tawa OOO till Jul 10th, 2015-09-12 18:38:09]

Patchset #1 (id:1) has been deleted

[Roger Tawa OOO till Jul 10th, 2015-09-14 19:04:03]

rogerta@chromium.org changed reviewers:
+ achuith@chromium.org, anthonyvd@chromium.org

[Roger Tawa OOO till Jul 10th, 2015-09-14 19:04:03]

Hi Anthony, Achuith,

Achuith: please take a look at
*.js
chrome/browser/ui/webui/*

Anthony: please do general review of whole CL.

Thanks.

[anthonyvd, 2015-09-15 18:13:47]

Great, just a few comments :)

This will require a seperate Mac implementation, correct?

https://codereview.chromium.org/1344443002/diff/60001/chrome/browser/about_fl...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/1344443002/diff/60001/chrome/browser/about_fl...
chrome/browser/about_flags.cc:1374: kOsWin | kOsLinux,
No Mac because this only includes the views implementation, right?

https://codereview.chromium.org/1344443002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/gaia_auth_host/authenticator.js (right):

https://codereview.chromium.org/1344443002/diff/60001/chrome/browser/resource...
chrome/browser/resources/gaia_auth_host/authenticator.js:280: url =
appendParam(url, 'Email', data.email);
nit: other params are all lower case, does this one need to be capitalized?

https://codereview.chromium.org/1344443002/diff/60001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/profile_chooser_view.cc (right):

https://codereview.chromium.org/1344443002/diff/60001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:1658: gfx::Size
pref_size = switches::UseNewGaiaFlow()
Maybe move this before the switch and use pref_size.width() at line 796?

https://codereview.chromium.org/1344443002/diff/60001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc (right):

https://codereview.chromium.org/1344443002/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:234:
params.SetString("readOnlyEmail", "1");
I only see this used as a conditional, any reason it's a string instead of a
bool?

https://codereview.chromium.org/1344443002/diff/60001/components/signin/core/...
File components/signin/core/common/profile_management_switches.h (right):

https://codereview.chromium.org/1344443002/diff/60001/components/signin/core/...
components/signin/core/common/profile_management_switches.h:42: // Checks
whether account consistency is enabled. If enabled, the account
This comment doesn't seem to reflect what this function does.

[Roger Tawa OOO till Jul 10th, 2015-09-16 19:27:29]

https://codereview.chromium.org/1344443002/diff/60001/chrome/browser/about_fl...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/1344443002/diff/60001/chrome/browser/about_fl...
chrome/browser/about_flags.cc:1374: kOsWin | kOsLinux,
On 2015/09/15 18:13:47, anthonyvd wrote:
> No Mac because this only includes the views implementation, right?

Correct.

https://codereview.chromium.org/1344443002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/gaia_auth_host/authenticator.js (right):

https://codereview.chromium.org/1344443002/diff/60001/chrome/browser/resource...
chrome/browser/resources/gaia_auth_host/authenticator.js:280: url =
appendParam(url, 'Email', data.email);
On 2015/09/15 18:13:47, anthonyvd wrote:
> nit: other params are all lower case, does this one need to be capitalized?

Yes.  This is specified by gaia.  The parameter needs to be specified like this.

https://codereview.chromium.org/1344443002/diff/60001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/profile_chooser_view.cc (right):

https://codereview.chromium.org/1344443002/diff/60001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:1658: gfx::Size
pref_size = switches::UseNewGaiaFlow()
On 2015/09/15 18:13:47, anthonyvd wrote:
> Maybe move this before the switch and use pref_size.width() at line 796?

Line 796 is in another function.  Maybe I'm missing something?

https://codereview.chromium.org/1344443002/diff/60001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc (right):

https://codereview.chromium.org/1344443002/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:234:
params.SetString("readOnlyEmail", "1");
On 2015/09/15 18:13:47, anthonyvd wrote:
> I only see this used as a conditional, any reason it's a string instead of a
> bool?

Done.

https://codereview.chromium.org/1344443002/diff/60001/components/signin/core/...
File components/signin/core/common/profile_management_switches.h (right):

https://codereview.chromium.org/1344443002/diff/60001/components/signin/core/...
components/signin/core/common/profile_management_switches.h:42: // Checks
whether account consistency is enabled. If enabled, the account
On 2015/09/15 18:13:47, anthonyvd wrote:
> This comment doesn't seem to reflect what this function does.

Done.  Darn cut and paste...

[anthonyvd, 2015-09-16 19:55:32]

lgtm, let me know if you need someone for the Mac port :)

https://codereview.chromium.org/1344443002/diff/60001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/profile_chooser_view.cc (right):

https://codereview.chromium.org/1344443002/diff/60001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:1658: gfx::Size
pref_size = switches::UseNewGaiaFlow()
On 2015/09/16 19:27:29, Roger Tawa wrote:
> On 2015/09/15 18:13:47, anthonyvd wrote:
> > Maybe move this before the switch and use pref_size.width() at line 796?
> 
> Line 796 is in another function.  Maybe I'm missing something?

That's entirely my bad, the lines in between were collapsed and I didn't notice
-_-

[achuithb, 2015-09-16 22:53:27]

https://codereview.chromium.org/1344443002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/gaia_auth_host/authenticator.js (right):

https://codereview.chromium.org/1344443002/diff/80001/chrome/browser/resource...
chrome/browser/resources/gaia_auth_host/authenticator.js:282: url =
appendParam(url, 'email_hint', data.email);
What's this?

https://codereview.chromium.org/1344443002/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/profile_chooser_view.cc (right):

https://codereview.chromium.org/1344443002/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:1658: gfx::Size
pref_size = switches::UseNewGaiaFlow()
nit const

https://codereview.chromium.org/1344443002/diff/80001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/signin/inline_login_handler_impl.cc (right):

https://codereview.chromium.org/1344443002/diff/80001/chrome/browser/ui/webui...
chrome/browser/ui/webui/signin/inline_login_handler_impl.cc:623: GURL url =
is_new_gaia_flow
const

[achuithb, 2015-09-16 22:55:41]

Is there a bug? Please file one if not.

Overall this looks fine. I don't know if it would be possible to split it into
smaller CLs. Especially the views UI stuff.

I also don't know if old/new is the best way to name this. For cros, I thought
IFrame/Webview was more descriptive.

What are the plans for browsertests for this?

[Roger Tawa OOO till Jul 10th, 2015-09-24 01:01:09]

Patchset #8 (id:160001) has been deleted

[Roger Tawa OOO till Jul 10th, 2015-09-24 05:54:18]

Thanks Achuith.

Attached crbug to CL.  Not sure it's worth breaking into smaller CLs.  This is
not very large as is and all the bits go together.  Changed old/new to password
combined/separated.

I'm noticing there are no browser tests for this code.  Can you point me to the
cros tests that exercise the password separated sign in flow?

https://codereview.chromium.org/1344443002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/gaia_auth_host/authenticator.js (right):

https://codereview.chromium.org/1344443002/diff/80001/chrome/browser/resource...
chrome/browser/resources/gaia_auth_host/authenticator.js:282: url =
appendParam(url, 'email_hint', data.email);
On 2015/09/16 22:53:27, achuithb wrote:
> What's this?

The email fields allow for the following possibilities:

1/ If neither Email nor email_hint is supplied, then the email text field is
blank and the user must type an email to proceed.

2/ If Email is supplied, then the email is hardcoded and the user cannot change
it.  The user is asked for password.  This is useful for re-auth scenarios,
where chrome needs the user to authenticate for a specific account and only that
account.

3/ If email_hint is supplied, gaia will prefill the email text field using the
given email address, but the user can still change it and then proceed.  This is
used on desktop when the user disconnects their profile then reconnects, to
encourage them to use the same account.

https://codereview.chromium.org/1344443002/diff/80001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/profile_chooser_view.cc (right):

https://codereview.chromium.org/1344443002/diff/80001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:1658: gfx::Size
pref_size = switches::UseNewGaiaFlow()
On 2015/09/16 22:53:27, achuithb wrote:
> nit const

Done.

https://codereview.chromium.org/1344443002/diff/80001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/signin/inline_login_handler_impl.cc (right):

https://codereview.chromium.org/1344443002/diff/80001/chrome/browser/ui/webui...
chrome/browser/ui/webui/signin/inline_login_handler_impl.cc:623: GURL url =
is_new_gaia_flow
On 2015/09/16 22:53:27, achuithb wrote:
> const

Done.

[achuithb, 2015-09-28 21:35:55]

On 2015/09/24 05:54:18, Roger Tawa wrote:
> Thanks Achuith.
> 
> Attached crbug to CL.  Not sure it's worth breaking into smaller CLs.  This is
> not very large as is and all the bits go together.  Changed old/new to
password
> combined/separated.
> 
> I'm noticing there are no browser tests for this code.  Can you point me to
the
> cros tests that exercise the password separated sign in flow?
> 

We have some here:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...

Not sure if something similar to this would be appropriate for this?

[achuithb, 2015-09-28 21:51:38]

https://codereview.chromium.org/1344443002/diff/200001/chrome/browser/resourc...
File chrome/browser/resources/gaia_auth_host/authenticator.js (right):

https://codereview.chromium.org/1344443002/diff/200001/chrome/browser/resourc...
chrome/browser/resources/gaia_auth_host/authenticator.js:279: if
(data.readOnlyEmail) {
Is a comment explaining readOnlyEmails appropriate here? Or is this assumed to
be well known? I don't think the concept exists on chromeos.

https://codereview.chromium.org/1344443002/diff/200001/chrome/browser/ui/view...
File chrome/browser/ui/views/profiles/profile_chooser_view.cc (right):

https://codereview.chromium.org/1344443002/diff/200001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:195: HostView() :
title_view_(nullptr), title_widget_(nullptr) {}
Use data member initialization instead for title_view_. title_widget_ is
auto-initialized.

https://codereview.chromium.org/1344443002/diff/200001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:205: // The title
itself and the overlaped widget that contains it.
Also add comment: Not owned.

https://codereview.chromium.org/1344443002/diff/200001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:206: views::View*
title_view_;
= null_ptr

https://codereview.chromium.org/1344443002/diff/200001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:476: static
views::ImageButton* CreateBackButton(views::ButtonListener* listener) {
We prefer this here rather than the anonymous namespace, right?

https://codereview.chromium.org/1344443002/diff/200001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:792: int width =
switches::UsePasswordSeparatedSigninFlow()
const

[Roger Tawa OOO till Jul 10th, 2015-10-02 01:16:31]

Thanks Achutih.

I've written browser tests for InlineLoginHelper.  If it's OK with you, tests
for the js part can be written when we change this flow to use a modal dialog
instead of the user menu.  I have already opened a bug to track this:

https://crbug.com/536918

https://codereview.chromium.org/1344443002/diff/200001/chrome/browser/resourc...
File chrome/browser/resources/gaia_auth_host/authenticator.js (right):

https://codereview.chromium.org/1344443002/diff/200001/chrome/browser/resourc...
chrome/browser/resources/gaia_auth_host/authenticator.js:279: if
(data.readOnlyEmail) {
On 2015/09/28 21:51:37, achuithb wrote:
> Is a comment explaining readOnlyEmails appropriate here? Or is this assumed to
> be well known? I don't think the concept exists on chromeos.

Done.

https://codereview.chromium.org/1344443002/diff/200001/chrome/browser/ui/view...
File chrome/browser/ui/views/profiles/profile_chooser_view.cc (right):

https://codereview.chromium.org/1344443002/diff/200001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:195: HostView() :
title_view_(nullptr), title_widget_(nullptr) {}
On 2015/09/28 21:51:38, achuithb wrote:
> Use data member initialization instead for title_view_. title_widget_ is
> auto-initialized.

Done.

https://codereview.chromium.org/1344443002/diff/200001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:205: // The title
itself and the overlaped widget that contains it.
On 2015/09/28 21:51:37, achuithb wrote:
> Also add comment: Not owned.

Done.

https://codereview.chromium.org/1344443002/diff/200001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:206: views::View*
title_view_;
On 2015/09/28 21:51:37, achuithb wrote:
> = null_ptr

Done.

https://codereview.chromium.org/1344443002/diff/200001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:476: static
views::ImageButton* CreateBackButton(views::ButtonListener* listener) {
On 2015/09/28 21:51:38, achuithb wrote:
> We prefer this here rather than the anonymous namespace, right?

Nope.  Moved to anonymous namespace.

https://codereview.chromium.org/1344443002/diff/200001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/profile_chooser_view.cc:792: int width =
switches::UsePasswordSeparatedSigninFlow()
On 2015/09/28 21:51:37, achuithb wrote:
> const

Done.

[achuithb, 2015-10-02 21:51:07]

https://codereview.chromium.org/1344443002/diff/320001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/signin/inline_login_handler_impl.cc (right):

https://codereview.chromium.org/1344443002/diff/320001/chrome/browser/ui/webu...
chrome/browser/ui/webui/signin/inline_login_handler_impl.cc:344: // Call
OneClickSigninSyncStarter to exchange oauth code for tokens.
This comment should go in the header file I think

https://codereview.chromium.org/1344443002/diff/320001/chrome/browser/ui/webu...
chrome/browser/ui/webui/signin/inline_login_handler_impl.cc:345: //
OneClickSigninSyncStarter will delete itself once the job is done.
This comment should go right before you create a new instance of
OneClickSigninSyncStarter

https://codereview.chromium.org/1344443002/diff/320001/chrome/browser/ui/webu...
chrome/browser/ui/webui/signin/inline_login_handler_impl.cc:585: const GURL url
= is_new_gaia_flow
const GURL&, you don't need to copy

https://codereview.chromium.org/1344443002/diff/320001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc (right):

https://codereview.chromium.org/1344443002/diff/320001/chrome/browser/ui/webu...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:128: // tests
below can ensure they are called without causing the side effects.
s/the//

https://codereview.chromium.org/1344443002/diff/320001/chrome/browser/ui/webu...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:186: // tests
below can ensure they are called without causing the side effects.
s/the//

https://codereview.chromium.org/1344443002/diff/320001/chrome/browser/ui/webu...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:537:
IN_PROC_BROWSER_TEST_F(InlineLoginHelperBrowserTest, WithSessionIndex) {
Please add a comment for this and all new tests what is being tested.

https://codereview.chromium.org/1344443002/diff/320001/chrome/browser/ui/webu...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:584: // possible
values of source=.
Could we also add a comment saying that source 0 is the start page, and what
some of the other sources are so the reader of this browsertest doesn't have to
bounce back and forth between signin_metrics.h and this file?

https://codereview.chromium.org/1344443002/diff/320001/chrome/browser/ui/webu...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:617: GURL
url("chrome://chrome-signin/?source=0");
const

https://codereview.chromium.org/1344443002/diff/320001/chrome/browser/ui/webu...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:681: GURL
url("chrome://chrome-signin/?source=3");
comment that this is settings

https://codereview.chromium.org/1344443002/diff/320001/chrome/browser/ui/webu...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:718: GURL
url("chrome://chrome-signin/?source=11");
comment that this is Reauth

[Roger Tawa OOO till Jul 10th, 2015-10-05 12:18:59]

https://codereview.chromium.org/1344443002/diff/320001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/signin/inline_login_handler_impl.cc (right):

https://codereview.chromium.org/1344443002/diff/320001/chrome/browser/ui/webu...
chrome/browser/ui/webui/signin/inline_login_handler_impl.cc:344: // Call
OneClickSigninSyncStarter to exchange oauth code for tokens.
On 2015/10/02 21:51:06, achuithb wrote:
> This comment should go in the header file I think

Done.

https://codereview.chromium.org/1344443002/diff/320001/chrome/browser/ui/webu...
chrome/browser/ui/webui/signin/inline_login_handler_impl.cc:345: //
OneClickSigninSyncStarter will delete itself once the job is done.
On 2015/10/02 21:51:06, achuithb wrote:
> This comment should go right before you create a new instance of
> OneClickSigninSyncStarter

Done.

https://codereview.chromium.org/1344443002/diff/320001/chrome/browser/ui/webu...
chrome/browser/ui/webui/signin/inline_login_handler_impl.cc:585: const GURL url
= is_new_gaia_flow
On 2015/10/02 21:51:06, achuithb wrote:
> const GURL&, you don't need to copy

Done.

https://codereview.chromium.org/1344443002/diff/320001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc (right):

https://codereview.chromium.org/1344443002/diff/320001/chrome/browser/ui/webu...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:128: // tests
below can ensure they are called without causing the side effects.
On 2015/10/02 21:51:07, achuithb wrote:
> s/the//

Done.

https://codereview.chromium.org/1344443002/diff/320001/chrome/browser/ui/webu...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:186: // tests
below can ensure they are called without causing the side effects.
On 2015/10/02 21:51:06, achuithb wrote:
> s/the//

Done.

https://codereview.chromium.org/1344443002/diff/320001/chrome/browser/ui/webu...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:537:
IN_PROC_BROWSER_TEST_F(InlineLoginHelperBrowserTest, WithSessionIndex) {
On 2015/10/02 21:51:06, achuithb wrote:
> Please add a comment for this and all new tests what is being tested.

Done.

https://codereview.chromium.org/1344443002/diff/320001/chrome/browser/ui/webu...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:584: // possible
values of source=.
On 2015/10/02 21:51:06, achuithb wrote:
> Could we also add a comment saying that source 0 is the start page, and what
> some of the other sources are so the reader of this browsertest doesn't have
to
> bounce back and forth between signin_metrics.h and this file? 

I'd rather not duplicate that info here, which is why I put a really easy way to
find the information in the comment.

https://codereview.chromium.org/1344443002/diff/320001/chrome/browser/ui/webu...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:617: GURL
url("chrome://chrome-signin/?source=0");
On 2015/10/02 21:51:07, achuithb wrote:
> const

Done.

https://codereview.chromium.org/1344443002/diff/320001/chrome/browser/ui/webu...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:681: GURL
url("chrome://chrome-signin/?source=3");
On 2015/10/02 21:51:06, achuithb wrote:
> comment that this is settings

Done.

https://codereview.chromium.org/1344443002/diff/320001/chrome/browser/ui/webu...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:718: GURL
url("chrome://chrome-signin/?source=11");
On 2015/10/02 21:51:06, achuithb wrote:
> comment that this is Reauth

Done in function comment.

[achuithb, 2015-10-05 19:33:22]

lgtm

[Roger Tawa OOO till Jul 10th, 2015-10-05 21:06:32]

The CQ bit was checked by rogerta@chromium.org

[Roger Tawa OOO till Jul 10th, 2015-10-05 21:06:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
anthonyvd@chromium.org
Link to the patchset: https://codereview.chromium.org/1344443002/#ps360001
(title: "merged")

[commit-bot: I haz the power, 2015-10-05 21:07:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1344443002/360001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1344443002/360001

[commit-bot: I haz the power, 2015-10-05 21:21:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-05 21:21:23]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Roger Tawa OOO till Jul 10th, 2015-10-05 21:46:59]

rogerta@chromium.org changed reviewers:
+ sky@chromium.org

[Roger Tawa OOO till Jul 10th, 2015-10-05 21:47:00]

Hi Scott,

This the CL we discussed a few weeks ago that you helped me with overlapped
views. Can you please do an owner review? Thanks.

[sky, 2015-10-05 23:57:31]

LGTM

[Roger Tawa OOO till Jul 10th, 2015-10-06 00:33:26]

The CQ bit was checked by rogerta@chromium.org

[commit-bot: I haz the power, 2015-10-06 00:34:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1344443002/360001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1344443002/360001

[commit-bot: I haz the power, 2015-10-06 00:47:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-06 00:47:16]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Roger Tawa OOO till Jul 10th, 2015-10-06 01:23:27]

rogerta@chromium.org changed reviewers:
+ jwd@chromium.org

[Roger Tawa OOO till Jul 10th, 2015-10-06 01:23:28]

Hi Jesse,

Please do owner review for histograms.xml.  Thanks,

[jwd, 2015-10-06 13:53:38]

lgtm

[Roger Tawa OOO till Jul 10th, 2015-10-06 14:39:47]

The CQ bit was checked by rogerta@chromium.org

[commit-bot: I haz the power, 2015-10-06 14:39:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1344443002/360001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1344443002/360001

[commit-bot: I haz the power, 2015-10-06 14:45:42]

Committed patchset #17 (id:360001)

[commit-bot: I haz the power, 2015-10-06 14:46:51]

Patchset 17 (id:??) landed as
https://crrev.com/fb8dc62b48a488f1dcd65948e6b34316c5c3c257
Cr-Commit-Position: refs/heads/master@{#352598}