Disable ability to show passwords when running in Metro.

Disable ability to show passwords when running in Metro.

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

[Will Harris, 2013-12-12 16:34:34]

The Win8 team suggested disabling showing passwords on Metro as the dialog must
be popped from the right Metro process, and that requires quite a lot of work. 
This is what we've done with some other features in Metro

[Patrick Dubroy, 2013-12-12 17:03:32]

https://codereview.chromium.org/107023009/diff/1/chrome/browser/ui/webui/opti...
File chrome/browser/ui/webui/options/password_manager_handler.cc (right):

https://codereview.chromium.org/107023009/diff/1/chrome/browser/ui/webui/opti...
chrome/browser/ui/webui/options/password_manager_handler.cc:13: #include
"chrome/browser/ui/ash/ash_util.h"
Includes should be in alphabetical order.

https://codereview.chromium.org/107023009/diff/1/chrome/browser/ui/webui/opti...
chrome/browser/ui/webui/options/password_manager_handler.cc:101: #if
defined(OS_WIN) && defined(USE_ASH)
I think it's easier to just pass a boolean in with the localized strings. Take a
look at what the history page does, for example, passing in several boolean
args: http://goo.gl/nAcjes

Then, you can just initialize showPassword_ to whatever value gets passed in
there.

[Will Harris, 2013-12-12 18:31:09]

PTAL

https://codereview.chromium.org/107023009/diff/1/chrome/browser/ui/webui/opti...
File chrome/browser/ui/webui/options/password_manager_handler.cc (right):

https://codereview.chromium.org/107023009/diff/1/chrome/browser/ui/webui/opti...
chrome/browser/ui/webui/options/password_manager_handler.cc:13: #include
"chrome/browser/ui/ash/ash_util.h"
On 2013/12/12 17:03:33, Patrick Dubroy wrote:
> Includes should be in alphabetical order.

Done.

https://codereview.chromium.org/107023009/diff/1/chrome/browser/ui/webui/opti...
chrome/browser/ui/webui/options/password_manager_handler.cc:101: #if
defined(OS_WIN) && defined(USE_ASH)
On 2013/12/12 17:03:33, Patrick Dubroy wrote:
> I think it's easier to just pass a boolean in with the localized strings. Take
a
> look at what the history page does, for example, passing in several boolean
> args: http://goo.gl/nAcjes
> 
> Then, you can just initialize showPassword_ to whatever value gets passed in
> there.

Done.

[Patrick Dubroy, 2013-12-13 14:19:09]

https://codereview.chromium.org/107023009/diff/40001/chrome/browser/resources...
File chrome/browser/resources/options/password_manager_list.js (right):

https://codereview.chromium.org/107023009/diff/40001/chrome/browser/resources...
chrome/browser/resources/options/password_manager_list.js:284: if
(loadTimeData.getBoolean('disableShowPasswords'))
Can't you just initialize this.showPasswords_ with this value?

https://codereview.chromium.org/107023009/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/password_manager_handler.cc (right):

https://codereview.chromium.org/107023009/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/password_manager_handler.cc:76: #if
defined(OS_WIN) && defined(USE_ASH)
Can you add a comment here to explain what this is doing?

[Patrick Dubroy, 2013-12-13 16:54:01]

lgtm with a suggestion.

https://codereview.chromium.org/107023009/diff/40001/chrome/browser/resources...
File chrome/browser/resources/options/password_manager_list.js (right):

https://codereview.chromium.org/107023009/diff/40001/chrome/browser/resources...
chrome/browser/resources/options/password_manager_list.js:276:
this.showPasswords_ = event.value.value;
I think you want to change this to:

  if (this.showPasswords_) {
    this.showPasswords_ = event.value.value;
    this.redraw();
  }

That is, the pref should only be obeyed if disableShowPasswords was not set.

onPreferenceChanged_() will be called after showPasswords_ is initialized, which
probably explains why it didn't work when you did it in the initialization
before.

[Will Harris, 2013-12-16 17:18:28]

As discussed, will commit with the boolean value checked in the createItem
function, because setting it at initialization caused race condition issues when
switching from normal to metro mode.

https://codereview.chromium.org/107023009/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/password_manager_handler.cc (right):

https://codereview.chromium.org/107023009/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/password_manager_handler.cc:76: #if
defined(OS_WIN) && defined(USE_ASH)
On 2013/12/13 14:19:10, Patrick Dubroy wrote:
> Can you add a comment here to explain what this is doing?

Done.

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

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

[commit-bot: I haz the power, 2013-12-16 18:02:45]

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

[commit-bot: I haz the power, 2013-12-16 22:26:24]

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

[commit-bot: I haz the power, 2013-12-17 00:29:13]

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

[commit-bot: I haz the power, 2013-12-17 02:45:10]

Change committed as 241122