[Mac] Passwords: Don't always prompt to access passwords saved by other browsers

[Mac] Passwords: Don't always prompt to access passwords saved by other browsers

Disable Keychain password access prompts when Chrome's "Ask to save passwords"
option is disabled. This gives users an escape hatch other than clicking
"Always allow" or "Deny" for each password prompt when they have passwords
saved from other browsers. This previously regressed with http://crrev.com/166878.

R=stuartmorgan@chromium.org
TBR=atwilson@chromium.org
BUG=178358
TEST=
1. In *Safari*, navigate to a login page for which you can save a password.
2. Log in, and instruct Safari to save the password.
3. In *Chrome*, under Advanced Settings, disable password management.
4. Navigate to the same login page.
At step (4), there should be no prompt for authorization to access the
password saved in Safari.

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

[Patrick Dubroy, 2013-12-11 13:36:41]

Ilya, please take a look.

This seems like a simpler way to solve the problem. Oddly, I could have sworn I
tried this a few days ago and that it didn't work. But, I've tested this CL on
10.9 and 10.8 and the API seems to work as advertised.

[Ilya Sherman, 2013-12-12 08:41:14]

Very nice, LG2M -- thanks!  However, I'd really like to get Stuart's review as
well, since he is much more intimately familiar with Keychain APIs and
subtleties around them than I am.

[Patrick Dubroy, 2013-12-14 10:50:26]

Stuart: ping?

[stuartmorgan, 2013-12-16 17:42:01]

Mac parts LGTM (didn't look at the rest). Please put a big warning in the
PasswordStore API though, pointing out that calling it with different prompt
arguments could result in different sets of passwords being returned. (I want it
to be very clear to a caller that they could, e.g., confuse sync by calling it
different ways from the sync system.)


FWIW, I understand this as an incremental improvement, but I'm not wild about
this approach. It feels like we're punting the real question of what the
password autofill behavior should be, and instead just exposing fiddly details
of the Keychain implementation in the form of inconsistent end-user behavior.
After this patch, how would you clearly and succinctly explain to a user not
familiar with terms like "keychain ACL" whether or not a password will be filled
in, and why, with the 'save' pref turned off, and how would you explain why a
pref about *saving* changes how some passwords fill, but not others?

I don't think there are good ways, which points to this not actually being a
good solution; hopefully someone will drive a better fix to this in the longer
term.

[commit-bot: I haz the power, 2013-12-16 18:06:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dubroy@chromium.org/106053008/80001

[commit-bot: I haz the power, 2013-12-16 19:50:08]

Retried try job too often on linux_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-12-16 20:37:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dubroy@chromium.org/106053008/80001

[commit-bot: I haz the power, 2013-12-16 22:25:50]

Retried try job too often on linux_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-12-16 22:52:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dubroy@chromium.org/106053008/80001

[commit-bot: I haz the power, 2013-12-17 00:45:16]

Retried try job too often on linux_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[Patrick Dubroy, 2013-12-17 11:41:01]

Committed patchset #7 manually as r241255 (presubmit successful).

[Patrick Dubroy, 2013-12-17 13:26:14]

On 2013/12/16 17:42:01, stuartmorgan wrote:
> Mac parts LGTM (didn't look at the rest). Please put a big warning in the
> PasswordStore API though, pointing out that calling it with different prompt
> arguments could result in different sets of passwords being returned. (I want
it
> to be very clear to a caller that they could, e.g., confuse sync by calling it
> different ways from the sync system.)
> 
> 
> FWIW, I understand this as an incremental improvement, but I'm not wild about
> this approach. It feels like we're punting the real question of what the
> password autofill behavior should be, and instead just exposing fiddly details
> of the Keychain implementation in the form of inconsistent end-user behavior.
> After this patch, how would you clearly and succinctly explain to a user not
> familiar with terms like "keychain ACL" whether or not a password will be
filled
> in, and why, with the 'save' pref turned off, and how would you explain why a
> pref about *saving* changes how some passwords fill, but not others?

As I mentioned in my comments (http://goo.gl/y3q4vu), I agree. I wanted to land
something for M33, but I plan to work on a better fix in M34.

> I don't think there are good ways, which points to this not actually being a
> good solution; hopefully someone will drive a better fix to this in the longer
> term.