mac: Don't show keychain prompt for password Autofill.

mac: Don't show keychain prompt for password Autofill.

Password Autofill is supposed to be a convenience. If it creates a
blocking dialog, it is no longer convenient. The original intention of
password Autofill was that it would only prompt the user after a full
username was typed in. Until that behavior is implemented, never prompt the
user for keychain access.

Effectively, this means that passwords stored by Chrome still work, since
Chrome can access those without a prompt, but passwords stored by Safari,
Firefox, or Chrome Canary will not work. Note that the latest build of Safari
and Firefox don't create keychain items with the relevant tags anyways.

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

[erikchen, 2014-07-11 23:26:18]

isherman: Please review.

[Ilya Sherman, 2014-07-11 23:44:06]

I'd prefer that we not keep this feature disabled indefinitely, but I guess this
is an acceptable stopgap in the meantime.  Thanks, Erik.  LGTM.

However, please also wait for Garrett or Stuart (who I realize is currently OOO)
to approve this CL as well -- I don't feel comfortable making the call for this
tradeoff on my own.

Even better would be if you went ahead an implemented the behavior to prompt
once the username is filled in -- then I would have no reservations about
blessing the CL ;)

[Ryan Sleevi, 2014-07-15 00:35:32]

I should mention that this LGTM (caveat is that we're affecting global state,
and thus can be affecting the IO thread while the keychain queries are
executing. But that's already existent in the current implementation, so I don't
*think* we'll break too much, and the keychain queries are behind a Big Global
Lock shared between the IO thread and this thread, because Apple broke
thread-safety of Security.framework in 10.6, really broke it in 10.8, and then
kinda broke it in 10.9)

[erikchen, 2014-07-15 21:10:56]

gcasto: Please review.

[Garrett Casto, 2014-07-15 21:37:22]

LGTM

It would be nice to see this fixed the right way, but this seems better than the
current behavior.

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

https://codereview.chromium.org/386043005/diff/20001/components/password_mana...
components/password_manager/core/browser/password_manager.cc:358: // password
Autofill was that it would only prompt the user after a full
Minor quibble, but I don't know if original intention is the right phrasing here
since this has never worked like this, as much as we would like it to. Something
like "We should only prompt ..." might be better.

[erikchen, 2014-07-15 21:47:25]

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

https://codereview.chromium.org/386043005/diff/20001/components/password_mana...
components/password_manager/core/browser/password_manager.cc:358: // password
Autofill was that it would only prompt the user after a full
On 2014/07/15 21:37:22, Garrett Casto wrote:
> Minor quibble, but I don't know if original intention is the right phrasing
here
> since this has never worked like this, as much as we would like it to.
Something
> like "We should only prompt ..." might be better.

Used your suggested wording.

[erikchen, 2014-07-15 21:47:28]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2014-07-15 21:51:10]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/erikchen@chromium.org/386043005/40001

[commit-bot: I haz the power, 2014-07-16 03:42:18]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/9...)
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/20...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win8_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win8_chromium_rel/bui...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/29021)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/33433)

[commit-bot: I haz the power, 2014-07-16 03:48:13]

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

[commit-bot: I haz the power, 2014-07-16 03:48:14]

Try jobs failed on following builders:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/20...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  win8_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win8_chromium_rel/bui...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/29118)

[erikchen, 2014-07-16 15:31:47]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2014-07-16 15:34:17]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/erikchen@chromium.org/386043005/60001

[commit-bot: I haz the power, 2014-07-16 20:50:35]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[commit-bot: I haz the power, 2014-07-16 21:21:28]

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

[commit-bot: I haz the power, 2014-07-16 21:21:29]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[erikchen, 2014-07-17 16:34:13]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2014-07-17 16:36:04]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/erikchen@chromium.org/386043005/60001

[commit-bot: I haz the power, 2014-07-17 17:44:23]

Change committed as 283805