[Password Manager] Pass origin of the form which was autofilled in order to display it in managemen…

[Password Manager] Pass origin of the form which was autofilled in order to display it in management bubble.

Currently password management bubble uses the origin of the first
matching form from password store in order to determine if we should
present form origin to user or not. Instead we should show URL of the
form, where credentials were autofilled.

BUG=539889
Committed: https://crrev.com/28139e2acc6a1d23adb00f0d1ae3ececf2a8da91
Cr-Commit-Position: refs/heads/master@{#353980}

[melandory, 2015-10-12 15:22:23]

The CQ bit was checked by melandory@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-12 15:26:06]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1402803002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1402803002/1

[commit-bot: I haz the power, 2015-10-12 15:49:37]

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

[commit-bot: I haz the power, 2015-10-12 15:49:37]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[melandory, 2015-10-13 08:30:45]

Patchset #1 (id:1) has been deleted

[melandory, 2015-10-13 09:48:38]

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

[melandory, 2015-10-13 09:48:39]

Hi Vaclav,

PTAL, you own everything except
chrome/browser/ui/cocoa/passwords/manage_passwords_bubble_manage_view_controller_unittest.mm

[vabr (Chromium), 2015-10-13 10:45:20]

Thanks, Tanja, this LGTM.

Please make sure to land the corresponding iOS patch before or at the same time
when you land this.

Cheers,
Vaclav

https://codereview.chromium.org/1402803002/diff/40001/chrome/browser/ui/passw...
File chrome/browser/ui/passwords/manage_passwords_state.h (right):

https://codereview.chromium.org/1402803002/diff/40001/chrome/browser/ui/passw...
chrome/browser/ui/passwords/manage_passwords_state.h:63: // Move to MANAGE_STATE
or INACTIVE_STATE for PSL matched passwords.
nit: When you are already touching this, please expand the comment by explaining
the 2 arguments.

https://codereview.chromium.org/1402803002/diff/40001/chrome/browser/ui/passw...
File chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc
(right):

https://codereview.chromium.org/1402803002/diff/40001/chrome/browser/ui/passw...
chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc:128: GURL
OriginFromPasswordFormMap(const autofill::PasswordFormMap& map) {
Typing
  OriginFromPasswordFormMap(map)
is actually longer than
  map.begin()->second->origin
and the latter makes it clearer what is being done, so I suggest getting rid of
the function.

[melandory, 2015-10-13 13:48:08]

On 2015/10/13 10:45:20, vabr (Chromium) wrote:
> Thanks, Tanja, this LGTM.
> 
> Please make sure to land the corresponding iOS patch before or at the same
time
> when you land this.
Stupid question, I don't see how iOS is affected here, so what do you think
should be done there?
> 
> Cheers,
> Vaclav
> 
>
https://codereview.chromium.org/1402803002/diff/40001/chrome/browser/ui/passw...
> File chrome/browser/ui/passwords/manage_passwords_state.h (right):
> 
>
https://codereview.chromium.org/1402803002/diff/40001/chrome/browser/ui/passw...
> chrome/browser/ui/passwords/manage_passwords_state.h:63: // Move to
MANAGE_STATE
> or INACTIVE_STATE for PSL matched passwords.
> nit: When you are already touching this, please expand the comment by
explaining
> the 2 arguments.
> 
>
https://codereview.chromium.org/1402803002/diff/40001/chrome/browser/ui/passw...
> File chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc
> (right):
> 
>
https://codereview.chromium.org/1402803002/diff/40001/chrome/browser/ui/passw...
> chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc:128:
GURL
> OriginFromPasswordFormMap(const autofill::PasswordFormMap& map) {
> Typing
>   OriginFromPasswordFormMap(map)
> is actually longer than
>   map.begin()->second->origin
> and the latter makes it clearer what is being done, so I suggest getting rid
of
> the function.

[melandory, 2015-10-13 13:48:16]

https://codereview.chromium.org/1402803002/diff/40001/chrome/browser/ui/passw...
File chrome/browser/ui/passwords/manage_passwords_state.h (right):

https://codereview.chromium.org/1402803002/diff/40001/chrome/browser/ui/passw...
chrome/browser/ui/passwords/manage_passwords_state.h:63: // Move to MANAGE_STATE
or INACTIVE_STATE for PSL matched passwords.
On 2015/10/13 10:45:20, vabr (Chromium) wrote:
> nit: When you are already touching this, please expand the comment by
explaining
> the 2 arguments.

Done.

https://codereview.chromium.org/1402803002/diff/40001/chrome/browser/ui/passw...
File chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc
(right):

https://codereview.chromium.org/1402803002/diff/40001/chrome/browser/ui/passw...
chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc:128: GURL
OriginFromPasswordFormMap(const autofill::PasswordFormMap& map) {
On 2015/10/13 10:45:20, vabr (Chromium) wrote:
> Typing
>   OriginFromPasswordFormMap(map)
> is actually longer than
>   map.begin()->second->origin
> and the latter makes it clearer what is being done, so I suggest getting rid
of
> the function.

Done.

[vabr (Chromium), 2015-10-13 13:51:53]

On 2015/10/13 13:48:08, melandory wrote:
> On 2015/10/13 10:45:20, vabr (Chromium) wrote:
> > Thanks, Tanja, this LGTM.
> > 
> > Please make sure to land the corresponding iOS patch before or at the same
> time
> > when you land this.
> Stupid question, I don't see how iOS is affected here, so what do you think
> should be done there?
My bad, I missed that iOS uses the default implementation of
PasswordWasAutofilled.
No worries then, this should work out of the box on iOS.
> > 
> > Cheers,
> > Vaclav
> > 
> >
>
https://codereview.chromium.org/1402803002/diff/40001/chrome/browser/ui/passw...
> > File chrome/browser/ui/passwords/manage_passwords_state.h (right):
> > 
> >
>
https://codereview.chromium.org/1402803002/diff/40001/chrome/browser/ui/passw...
> > chrome/browser/ui/passwords/manage_passwords_state.h:63: // Move to
> MANAGE_STATE
> > or INACTIVE_STATE for PSL matched passwords.
> > nit: When you are already touching this, please expand the comment by
> explaining
> > the 2 arguments.
> > 
> >
>
https://codereview.chromium.org/1402803002/diff/40001/chrome/browser/ui/passw...
> > File chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc
> > (right):
> > 
> >
>
https://codereview.chromium.org/1402803002/diff/40001/chrome/browser/ui/passw...
> > chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc:128:
> GURL
> > OriginFromPasswordFormMap(const autofill::PasswordFormMap& map) {
> > Typing
> >   OriginFromPasswordFormMap(map)
> > is actually longer than
> >   map.begin()->second->origin
> > and the latter makes it clearer what is being done, so I suggest getting rid
> of
> > the function.

[vabr (Chromium), 2015-10-13 13:52:19]

lgtm

[melandory, 2015-10-13 13:55:21]

melandory@chromium.org changed reviewers:
+ asvitkine@chromium.org

[melandory, 2015-10-13 13:55:21]

asvitkine@chromium.org: Please review changes in 

chrome/browser/ui/cocoa/passwords/manage_passwords_bubble_manage_view_controller_unittest.mm

[melandory, 2015-10-13 14:27:08]

melandory@chromium.org changed reviewers:
+ ccameron@chromium.org
- asvitkine@chromium.org

[melandory, 2015-10-13 14:27:09]

ccameron@chromium.org: Please review changes in 

chrome/browser/ui/cocoa/passwords/manage_passwords_bubble_manage_view_controller_unittest.mm

[Alexei Svitkine (slow), 2015-10-13 16:44:53]

asvitkine@chromium.org changed reviewers:
+ asvitkine@chromium.org

[Alexei Svitkine (slow), 2015-10-13 16:44:54]

lgtm

[melandory, 2015-10-14 08:06:09]

The CQ bit was checked by melandory@chromium.org

[commit-bot: I haz the power, 2015-10-14 08:06:31]

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

[commit-bot: I haz the power, 2015-10-14 08:20:14]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-10-14 08:21:00]

Patchset 3 (id:??) landed as
https://crrev.com/28139e2acc6a1d23adb00f0d1ae3ececf2a8da91
Cr-Commit-Position: refs/heads/master@{#353980}