Basic implementation of showing password fill dialog on page load

Basic implementation of showing password fill dialog on page load

BUG=568713
Committed: https://crrev.com/fb2a143754f612573dddcf2496f6ba82c31e943d
Cr-Commit-Position: refs/heads/master@{#399583}

[jww, 2016-06-03 18:41:24]

jww@chromium.org changed reviewers:
+ vabr@chromium.org

[jww, 2016-06-03 18:41:25]

Hey vabr@. This is a first pass at the new fill-on-account-select UI. In
particular, it shows the password account menu on page load instead of
autofilling.

This is meant to be a first-pass sanity check, and there are many issues with it
and cleanups needed :-) For example, it doesn't actually implement the behavior
behind a flag; this CL makes it the default behaivor. Additionally, there's a
lot of basic refactoring and simplification that I need to do, add tests, et c.
In any case, if you can take a brief look and let me know what you think about
the approach, I can go ahead with the rest of that stuff.

[vabr (Chromium), 2016-06-06 16:04:28]

Hi jww@!

I skimmed this and it looks good, exactly as I hoped it would go. Did not manage
to compile and try it out today, but will attempt that later.

Happy to review any CLs you make this into. Thanks for making this happen! :)

Cheers,
Vaclav

[jww, 2016-06-06 17:27:24]

On 2016/06/06 16:04:28, vabr (Chromium) wrote:
> Hi jww@!
> 
> I skimmed this and it looks good, exactly as I hoped it would go. Did not
manage
> to compile and try it out today, but will attempt that later.
> 
> Happy to review any CLs you make this into. Thanks for making this happen! :)
> 
> Cheers,
> Vaclav

Phew! I'll try to finish this up over the next few days.

[vabr (Chromium), 2016-06-07 07:59:45]

On 2016/06/06 17:27:24, jww wrote:
> On 2016/06/06 16:04:28, vabr (Chromium) wrote:
> > Hi jww@!
> > 
> > I skimmed this and it looks good, exactly as I hoped it would go. Did not
> manage
> > to compile and try it out today, but will attempt that later.
> > 
> > Happy to review any CLs you make this into. Thanks for making this happen!
:)
> > 
> > Cheers,
> > Vaclav
> 
> Phew! I'll try to finish this up over the next few days.

I just tried your patch on https://rsolomakhin.github.io/autofill/. Once the
password form is visible during loading, this looks great! There was an issue
that when the password form was outside of the visible part of the page, the
pop-up was not shown, and any user action such as scrolling dismissed it. We
might need to make sure somehow that the user has a chance to understand that
there are passwords to fill when the user ultimately gets to the password form.

Cheers,
Vaclav

[jww, 2016-06-08 01:17:06]

On 2016/06/07 07:59:45, vabr (Chromium) wrote:
> On 2016/06/06 17:27:24, jww wrote:
> > On 2016/06/06 16:04:28, vabr (Chromium) wrote:
> > > Hi jww@!
> > > 
> > > I skimmed this and it looks good, exactly as I hoped it would go. Did not
> > manage
> > > to compile and try it out today, but will attempt that later.
> > > 
> > > Happy to review any CLs you make this into. Thanks for making this happen!
> :)
> > > 
> > > Cheers,
> > > Vaclav
> > 
> > Phew! I'll try to finish this up over the next few days.
> 
> I just tried your patch on https://rsolomakhin.github.io/autofill/. Once the
> password form is visible during loading, this looks great! There was an issue
> that when the password form was outside of the visible part of the page, the
> pop-up was not shown, and any user action such as scrolling dismissed it. We
> might need to make sure somehow that the user has a chance to understand that
> there are passwords to fill when the user ultimately gets to the password
form.
> 
> Cheers,
> Vaclav

Ah, great catch. For simplicity sake, how would you feel about landing a cleaned
up version of this CL and a later fix to address that? I'd personally like to
get the basic infrastructure up first.

[vabr (Chromium), 2016-06-08 07:55:55]

On 2016/06/08 01:17:06, jww wrote:
> On 2016/06/07 07:59:45, vabr (Chromium) wrote:
> > On 2016/06/06 17:27:24, jww wrote:
> > > On 2016/06/06 16:04:28, vabr (Chromium) wrote:
> > > > Hi jww@!
> > > > 
> > > > I skimmed this and it looks good, exactly as I hoped it would go. Did
not
> > > manage
> > > > to compile and try it out today, but will attempt that later.
> > > > 
> > > > Happy to review any CLs you make this into. Thanks for making this
happen!
> > :)
> > > > 
> > > > Cheers,
> > > > Vaclav
> > > 
> > > Phew! I'll try to finish this up over the next few days.
> > 
> > I just tried your patch on https://rsolomakhin.github.io/autofill/. Once the
> > password form is visible during loading, this looks great! There was an
issue
> > that when the password form was outside of the visible part of the page, the
> > pop-up was not shown, and any user action such as scrolling dismissed it. We
> > might need to make sure somehow that the user has a chance to understand
that
> > there are passwords to fill when the user ultimately gets to the password
> form.
> > 
> > Cheers,
> > Vaclav
> 
> Ah, great catch. For simplicity sake, how would you feel about landing a
cleaned
> up version of this CL and a later fix to address that? I'd personally like to
> get the basic infrastructure up first.

That sounds reasonable to me! Just please make sure that "cleaned up" involves
hiding
this behind a flag.

Cheers,
Vaclav

[jww, 2016-06-08 13:34:34]

Haha, of course :-)

On Wed, Jun 8, 2016, 12:55 AM <vabr@chromium.org> wrote:

> On 2016/06/08 01:17:06, jww wrote:
> > On 2016/06/07 07:59:45, vabr (Chromium) wrote:
> > > On 2016/06/06 17:27:24, jww wrote:
> > > > On 2016/06/06 16:04:28, vabr (Chromium) wrote:
> > > > > Hi jww@!
> > > > >
> > > > > I skimmed this and it looks good, exactly as I hoped it would go.
> Did
> not
> > > > manage
> > > > > to compile and try it out today, but will attempt that later.
> > > > >
> > > > > Happy to review any CLs you make this into. Thanks for making this
> happen!
> > > :)
> > > > >
> > > > > Cheers,
> > > > > Vaclav
> > > >
> > > > Phew! I'll try to finish this up over the next few days.
> > >
> > > I just tried your patch on https://rsolomakhin.github.io/autofill/.
> Once the
> > > password form is visible during loading, this looks great! There was an
> issue
> > > that when the password form was outside of the visible part of the
> page, the
> > > pop-up was not shown, and any user action such as scrolling dismissed
> it. We
> > > might need to make sure somehow that the user has a chance to
> understand
> that
> > > there are passwords to fill when the user ultimately gets to the
> password
> > form.
> > >
> > > Cheers,
> > > Vaclav
> >
> > Ah, great catch. For simplicity sake, how would you feel about landing a
> cleaned
> > up version of this CL and a later fix to address that? I'd personally
> like to
> > get the basic infrastructure up first.
>
> That sounds reasonable to me! Just please make sure that "cleaned up"
> involves
> hiding
> this behind a flag.
>
> Cheers,
> Vaclav
>
> https://codereview.chromium.org/2035143002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jww, 2016-06-08 22:56:16]

vabr@, I think this is ready for review. Can you take a look?

[vabr (Chromium), 2016-06-09 17:40:48]

On 2016/06/08 22:56:16, jww wrote:
> vabr@, I think this is ready for review. Can you take a look?

Sorry for delay, was OOO most of this day. I'll try hard to find time tomorrow
(Friday), although I have a couple of meetings and a 500+ lines review looming
in my calendar for that day.

Thanks for making this happen, I'll update this ASAP.

Cheers,
Vaclav

[jww, 2016-06-09 17:44:00]

On 2016/06/09 17:40:48, vabr (Chromium) wrote:
> On 2016/06/08 22:56:16, jww wrote:
> > vabr@, I think this is ready for review. Can you take a look?
> 
> Sorry for delay, was OOO most of this day. I'll try hard to find time tomorrow
> (Friday), although I have a couple of meetings and a 500+ lines review looming
> in my calendar for that day.
> 
> Thanks for making this happen, I'll update this ASAP.
> 
> Cheers,
> Vaclav

No rush; thanks :-)

[vabr (Chromium), 2016-06-10 14:19:52]

LGTM with nits.

(Also, I was not aware before that you are not an OWNER of autofill_messages.h.
But at least you have a plenty of reviewers for that in your timezone :).)

Thanks!
Vaclav

https://codereview.chromium.org/2035143002/diff/20001/chrome/renderer/autofil...
File chrome/renderer/autofill/password_autofill_agent_browsertest.cc (right):

https://codereview.chromium.org/2035143002/diff/20001/chrome/renderer/autofil...
chrome/renderer/autofill/password_autofill_agent_browsertest.cc:260: // As
above, but fills for an iframe.
nit: This comment refers to line 239, but looks like referring to 249 now.

https://codereview.chromium.org/2035143002/diff/20001/components/autofill/con...
File components/autofill/content/renderer/password_autofill_agent.h (right):

https://codereview.chromium.org/2035143002/diff/20001/components/autofill/con...
components/autofill/content/renderer/password_autofill_agent.h:88: void
GetFillableElementFromFormData(
nit: Please add a comment explaining the effect of this method and meaning of
its arguments.

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
File components/password_manager/core/browser/password_form_manager.cc (right):

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
components/password_manager/core/browser/password_form_manager.cc:122: bool
ShouldShowInitialPasswordAccountSuggestions() {
Not for this CL, but if you get a chance, it might be a good idea to convert the
fill-on-account-select switch to a feature API. Will send some more internal
links and reasoning in a non-public e-mail.

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
components/password_manager/core/browser/password_form_manager.cc:569: // If
fill-on-account-select is not enabled, continue will autofilling any
typo: will -> with

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
File components/password_manager/core/browser/password_manager.cc (right):

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
components/password_manager/core/browser/password_manager.cc:777:
std::unique_ptr<BrowserSavePasswordProgressLogger> logger;
Yay, thanks for not forgetting the logging! :)

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
File components/password_manager/core/browser/password_manager.h (right):

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
components/password_manager/core/browser/password_manager.h:71: void
ShowInitialPasswordAccountSuggestions(
nit: Please add an explanatory comment: what does this do and how it's different
from the Autofill method.

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
File components/password_manager/core/browser/password_manager_driver.h (right):

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
components/password_manager/core/browser/password_manager_driver.h:72: virtual
void ShowInitialPasswordAccountSuggestions(
You will need to implement this in //ios/chrome/browser/passwords as well. I
guess for the experiment you can go with a trivial implementation, but for the
potential launch we should add a real one.

https://codereview.chromium.org/2035143002/diff/20001/ppapi/tests/ppapi_nacl_...
File ppapi/tests/ppapi_nacl_tests_newlib.nmf (right):

https://codereview.chromium.org/2035143002/diff/20001/ppapi/tests/ppapi_nacl_...
ppapi/tests/ppapi_nacl_tests_newlib.nmf:2: "files": {},
This looks funny, did you mean to include the changes for this file?

[vabr (Chromium), 2016-06-10 14:25:23]

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
File components/password_manager/core/browser/password_form_manager.cc (right):

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
components/password_manager/core/browser/password_form_manager.cc:136: return
group_name == kFillOnAccountSelectFieldTrialEnabledGroup;
Could you make this match that kFillOnAccountSelectFieldTrialEnabledGroup is a
prefix of the actual group, rather than it's precise name?
That allows different naming of dogfood groups while keeping the same behaviour
on the client.

[jww, 2016-06-10 23:06:54]

vabr@, I addressed your nits, but as I mention in the responses, can you take a
quick look again at password_autofill_agent.cc and autofill_agent.cc? I did some
small refactoring there and it would be great to get your sanity check on it.
Thanks!

https://codereview.chromium.org/2035143002/diff/20001/chrome/renderer/autofil...
File chrome/renderer/autofill/password_autofill_agent_browsertest.cc (right):

https://codereview.chromium.org/2035143002/diff/20001/chrome/renderer/autofil...
chrome/renderer/autofill/password_autofill_agent_browsertest.cc:260: // As
above, but fills for an iframe.
On 2016/06/10 14:19:51, vabr (Chromium) wrote:
> nit: This comment refers to line 239, but looks like referring to 249 now.

Nice catch. I swapped the location of the actual functions since it kind of
makes more sense for SimulateOnFillPasswordFormForFrame() directly below
SimulateOnFillPasswordForm().

https://codereview.chromium.org/2035143002/diff/20001/components/autofill/con...
File components/autofill/content/renderer/password_autofill_agent.h (right):

https://codereview.chromium.org/2035143002/diff/20001/components/autofill/con...
components/autofill/content/renderer/password_autofill_agent.h:88: void
GetFillableElementFromFormData(
On 2016/06/10 14:19:51, vabr (Chromium) wrote:
> nit: Please add a comment explaining the effect of this method and meaning of
> its arguments.

Done. I've also refactored the code for this a bit. namely, I removed the
"should_fill_forms" bit, and I've made OnFillPasswordForm do that work instead,
since it's really only specific to that function. I'm pretty sure that it's a
good change (and, of course, the tests pass), but it would still be great if you
could take a look to the most recent change in password_autofill_agent.cc to
make sure that it passes muster. Thanks!

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
File components/password_manager/core/browser/password_form_manager.cc (right):

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
components/password_manager/core/browser/password_form_manager.cc:122: bool
ShouldShowInitialPasswordAccountSuggestions() {
On 2016/06/10 14:19:52, vabr (Chromium) wrote:
> Not for this CL, but if you get a chance, it might be a good idea to convert
the
> fill-on-account-select switch to a feature API. Will send some more internal
> links and reasoning in a non-public e-mail.

Acknowledged.

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
components/password_manager/core/browser/password_form_manager.cc:136: return
group_name == kFillOnAccountSelectFieldTrialEnabledGroup;
On 2016/06/10 14:25:23, vabr (Chromium) wrote:
> Could you make this match that kFillOnAccountSelectFieldTrialEnabledGroup is a
> prefix of the actual group, rather than it's precise name?
> That allows different naming of dogfood groups while keeping the same
behaviour
> on the client.

Done.

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
components/password_manager/core/browser/password_form_manager.cc:569: // If
fill-on-account-select is not enabled, continue will autofilling any
On 2016/06/10 14:19:51, vabr (Chromium) wrote:
> typo: will -> with

Done.

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
File components/password_manager/core/browser/password_manager.cc (right):

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
components/password_manager/core/browser/password_manager.cc:777:
std::unique_ptr<BrowserSavePasswordProgressLogger> logger;
On 2016/06/10 14:19:52, vabr (Chromium) wrote:
> Yay, thanks for not forgetting the logging! :)

Acknowledged.

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
File components/password_manager/core/browser/password_manager.h (right):

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
components/password_manager/core/browser/password_manager.h:71: void
ShowInitialPasswordAccountSuggestions(
On 2016/06/10 14:19:52, vabr (Chromium) wrote:
> nit: Please add an explanatory comment: what does this do and how it's
different
> from the Autofill method.

Done.

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
File components/password_manager/core/browser/password_manager_driver.h (right):

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
components/password_manager/core/browser/password_manager_driver.h:72: virtual
void ShowInitialPasswordAccountSuggestions(
On 2016/06/10 14:19:52, vabr (Chromium) wrote:
> You will need to implement this in //ios/chrome/browser/passwords as well. I
> guess for the experiment you can go with a trivial implementation, but for the
> potential launch we should add a real one.

Maybe for now we'll just autofill on iOS and just not run the experiment?

https://codereview.chromium.org/2035143002/diff/20001/ppapi/tests/ppapi_nacl_...
File ppapi/tests/ppapi_nacl_tests_newlib.nmf (right):

https://codereview.chromium.org/2035143002/diff/20001/ppapi/tests/ppapi_nacl_...
ppapi/tests/ppapi_nacl_tests_newlib.nmf:2: "files": {},
On 2016/06/10 14:19:52, vabr (Chromium) wrote:
> This looks funny, did you mean to include the changes for this file?

Nope, this is a mistake. Will remove change.

[jww, 2016-06-11 18:50:36]

jww@chromium.org changed reviewers:
+ kenrb@chromium.org

[jww, 2016-06-11 18:50:37]

kenrb@, woud you mind taking a look at autofill_messages.h? Thanks!

[vabr (Chromium), 2016-06-13 08:46:41]

Still LGTM, with comments.

Thanks!
Vaclav

https://codereview.chromium.org/2035143002/diff/20001/components/autofill/con...
File components/autofill/content/renderer/password_autofill_agent.h (right):

https://codereview.chromium.org/2035143002/diff/20001/components/autofill/con...
components/autofill/content/renderer/password_autofill_agent.h:88: void
GetFillableElementFromFormData(
On 2016/06/10 23:06:54, jww wrote:
> On 2016/06/10 14:19:51, vabr (Chromium) wrote:
> > nit: Please add a comment explaining the effect of this method and meaning
of
> > its arguments.
> 
> Done. I've also refactored the code for this a bit. namely, I removed the
> "should_fill_forms" bit, and I've made OnFillPasswordForm do that work
instead,
> since it's really only specific to that function. I'm pretty sure that it's a
> good change (and, of course, the tests pass), but it would still be great if
you
> could take a look to the most recent change in password_autofill_agent.cc to
> make sure that it passes muster. Thanks!

Acknowledged, that sounds like an improvement to me!

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
File components/password_manager/core/browser/password_manager_driver.h (right):

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
components/password_manager/core/browser/password_manager_driver.h:72: virtual
void ShowInitialPasswordAccountSuggestions(
On 2016/06/10 23:06:54, jww wrote:
> On 2016/06/10 14:19:52, vabr (Chromium) wrote:
> > You will need to implement this in //ios/chrome/browser/passwords as well. I
> > guess for the experiment you can go with a trivial implementation, but for
the
> > potential launch we should add a real one.
> 
> Maybe for now we'll just autofill on iOS and just not run the experiment?

Yes, that's what I meant by the "trivial implementation". But to get this to
compile at all, you'll need to implement ShowInitialPasswordAccountSuggestions
at least as {} in
ios/chrome/browser/passwords/ios_chrome_password_manager_driver.*.

https://codereview.chromium.org/2035143002/diff/40001/components/password_man...
File components/password_manager/core/browser/password_form_manager.cc (right):

https://codereview.chromium.org/2035143002/diff/40001/components/password_man...
components/password_manager/core/browser/password_form_manager.cc:136: return
group_name.find(kFillOnAccountSelectFieldTrialEnabledGroup) == 0;
While this certainly works, it is not the most efficient way (keeps trying to
find this as a non-prefix substring although that's not needed.

Why not use the base/strings/string_util.h's base::StartsWith? As a bonus, that
will make the code a bit more readable as well.

[kenrb, 2016-06-13 14:49:34]

ipc lgtm

[jww, 2016-06-13 17:21:02]

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
File components/password_manager/core/browser/password_manager_driver.h (right):

https://codereview.chromium.org/2035143002/diff/20001/components/password_man...
components/password_manager/core/browser/password_manager_driver.h:72: virtual
void ShowInitialPasswordAccountSuggestions(
On 2016/06/13 08:46:41, vabr (Chromium) wrote:
> On 2016/06/10 23:06:54, jww wrote:
> > On 2016/06/10 14:19:52, vabr (Chromium) wrote:
> > > You will need to implement this in //ios/chrome/browser/passwords as well.
I
> > > guess for the experiment you can go with a trivial implementation, but for
> the
> > > potential launch we should add a real one.
> > 
> > Maybe for now we'll just autofill on iOS and just not run the experiment?
> 
> Yes, that's what I meant by the "trivial implementation". But to get this to
> compile at all, you'll need to implement ShowInitialPasswordAccountSuggestions
> at least as {} in
> ios/chrome/browser/passwords/ios_chrome_password_manager_driver.*.

Done.

https://codereview.chromium.org/2035143002/diff/40001/components/password_man...
File components/password_manager/core/browser/password_form_manager.cc (right):

https://codereview.chromium.org/2035143002/diff/40001/components/password_man...
components/password_manager/core/browser/password_form_manager.cc:136: return
group_name.find(kFillOnAccountSelectFieldTrialEnabledGroup) == 0;
On 2016/06/13 08:46:41, vabr (Chromium) wrote:
> While this certainly works, it is not the most efficient way (keeps trying to
> find this as a non-prefix substring although that's not needed.
> 
> Why not use the base/strings/string_util.h's base::StartsWith? As a bonus,
that
> will make the code a bit more readable as well.

Done.

[jww, 2016-06-13 17:21:08]

The CQ bit was checked by jww@chromium.org

[jww, 2016-06-13 17:21:12]

The patchset sent to the CQ was uploaded after l-g-t-m from vabr@chromium.org,
kenrb@chromium.org
Link to the patchset: https://codereview.chromium.org/2035143002/#ps60001
(title: "Fix nits")

[commit-bot: I haz the power, 2016-06-13 17:22:01]

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

[commit-bot: I haz the power, 2016-06-13 17:37:45]

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

[commit-bot: I haz the power, 2016-06-13 17:37:47]

Try jobs failed on following builders:
  ios-simulator-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)

[jww, 2016-06-13 18:00:49]

The CQ bit was checked by jww@chromium.org

[jww, 2016-06-13 18:00:50]

The patchset sent to the CQ was uploaded after l-g-t-m from vabr@chromium.org,
kenrb@chromium.org
Link to the patchset: https://codereview.chromium.org/2035143002/#ps80001
(title: "Fix iOS compile")

[commit-bot: I haz the power, 2016-06-13 18:01:43]

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

[commit-bot: I haz the power, 2016-06-13 21:25:46]

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

[commit-bot: I haz the power, 2016-06-13 21:25:47]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[jww, 2016-06-13 21:30:15]

The CQ bit was checked by jww@chromium.org

[commit-bot: I haz the power, 2016-06-13 21:31:01]

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

[commit-bot: I haz the power, 2016-06-13 22:44:28]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-06-13 22:44:48]

CQ bit was unchecked

[commit-bot: I haz the power, 2016-06-13 22:46:11]

Description was changed from

==========
Basic implementation of showing password fill dialog on page load

BUG=568713
==========

to

==========
Basic implementation of showing password fill dialog on page load

BUG=568713

Committed: https://crrev.com/fb2a143754f612573dddcf2496f6ba82c31e943d
Cr-Commit-Position: refs/heads/master@{#399583}
==========

[commit-bot: I haz the power, 2016-06-13 22:46:13]

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