cros: Clean up screenlockPrivate plumbing.

cros: Clean up screenlockPrivate plumbing.

- Make SigninScreenHandler a ScreenlockBidge::LockHandler and
  get rid of middle layers;
- Remove deprecated custom button click;
- customButton -> customIcon renaming and HTML/js/css update;
- Move api test out of cros;

R=asargent@chromium.org, isherman@chromium.org, rogerta@chromium.org, tengs@chromium.org

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

[Tim Song, 2014-05-16 18:38:46]

https://codereview.chromium.org/289013002/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/login/screen_locker.cc (right):

https://codereview.chromium.org/289013002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/login/screen_locker.cc:266: if (auth_type !=
ScreenlockBridge::LockHandler::OFFLINE_PASSWORD) {
Does it still make sense to wire up the OnAuthAttempted event here rather than
the SigninScreenHandler? This event is routed from the UserManagerScreenHandler
in the non-ChromeOS case.

https://codereview.chromium.org/289013002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/login/user_pod_row.js (left):

https://codereview.chromium.org/289013002/diff/20001/chrome/browser/resources...
chrome/browser/resources/login/user_pod_row.js:191:
this.customButtonElement.addEventListener('click',
It's also worth changing the HTML and CSS, so this isn't a button anymore. This
element should be renamed something more meaningful now that it's not a button.

https://codereview.chromium.org/289013002/diff/20001/chrome/common/extensions...
File chrome/common/extensions/api/screenlock_private.idl (right):

https://codereview.chromium.org/289013002/diff/20001/chrome/common/extensions...
chrome/common/extensions/api/screenlock_private.idl:35: static void
showButton(DOMString icon);
This function should be renamed to something more descriptive (showIcon?) not
that it's not really a button. Maybe we should invent a unique name for this
specific element to disambiguate it from the multitude of other icons and images
on the login screen.

https://codereview.chromium.org/289013002/diff/20001/chrome/common/extensions...
chrome/common/extensions/api/screenlock_private.idl:59: static void
onButtonClicked();
Remove this event as it's never fired now.

[xiyuan, 2014-05-16 23:00:57]

https://codereview.chromium.org/289013002/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/login/screen_locker.cc (right):

https://codereview.chromium.org/289013002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/login/screen_locker.cc:266: if (auth_type !=
ScreenlockBridge::LockHandler::OFFLINE_PASSWORD) {
On 2014/05/16 18:38:47, Tim Song wrote:
> Does it still make sense to wire up the OnAuthAttempted event here rather than
> the SigninScreenHandler? This event is routed from the
UserManagerScreenHandler
> in the non-ChromeOS case.

Done.

https://codereview.chromium.org/289013002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/login/user_pod_row.js (left):

https://codereview.chromium.org/289013002/diff/20001/chrome/browser/resources...
chrome/browser/resources/login/user_pod_row.js:191:
this.customButtonElement.addEventListener('click',
On 2014/05/16 18:38:47, Tim Song wrote:
> It's also worth changing the HTML and CSS, so this isn't a button anymore.
This
> element should be renamed something more meaningful now that it's not a
button.

CustomButton -> CustomIcon
<button> -> <div>
<img> is removed and icon is shown as the div's background image.

https://codereview.chromium.org/289013002/diff/20001/chrome/common/extensions...
File chrome/common/extensions/api/screenlock_private.idl (right):

https://codereview.chromium.org/289013002/diff/20001/chrome/common/extensions...
chrome/common/extensions/api/screenlock_private.idl:35: static void
showButton(DOMString icon);
On 2014/05/16 18:38:47, Tim Song wrote:
> This function should be renamed to something more descriptive (showIcon?) not
> that it's not really a button. Maybe we should invent a unique name for this
> specific element to disambiguate it from the multitude of other icons and
images
> on the login screen. 

showButton -> showCustomIcon
hideButton -> hideCustomIcon

and updated ScreenlockBridge::LockHandler and js files as well.

https://codereview.chromium.org/289013002/diff/20001/chrome/common/extensions...
chrome/common/extensions/api/screenlock_private.idl:59: static void
onButtonClicked();
On 2014/05/16 18:38:47, Tim Song wrote:
> Remove this event as it's never fired now.

Done.

[Tim Song, 2014-05-16 23:27:05]

lgtm

https://codereview.chromium.org/289013002/diff/80001/extensions/browser/exten...
File extensions/browser/extension_function_histogram_value.h (right):

https://codereview.chromium.org/289013002/diff/80001/extensions/browser/exten...
extensions/browser/extension_function_histogram_value.h:713:
SCREENLOCKPRIVATE_SHOWCUSTOMICON,
Update histograms.xml as well.

[xiyuan, 2014-05-17 03:41:12]

Owners, your power of approval is needed.

asargent@: extensions/*
isherman@: histograms.xml

Thanks.

[xiyuan, 2014-05-17 06:40:16]

rogerta@, needs your approval for c/b/signin/screenlock_bridge.h

Thanks.

[Ilya Sherman, 2014-05-17 21:38:12]

histograms lgtm

[asargent_no_longer_on_chrome, 2014-05-19 17:53:46]

extensions lgtm

https://codereview.chromium.org/289013002/diff/200001/chrome/browser/extensio...
File
chrome/browser/extensions/api/screenlock_private/screenlock_private_apitest.cc
(right):

https://codereview.chromium.org/289013002/diff/200001/chrome/browser/extensio...
chrome/browser/extensions/api/screenlock_private/screenlock_private_apitest.cc:35:
// New profile managerment needs to be on for non-ChromeOS lock.
typo: managerment -> management

[xiyuan, 2014-05-19 17:57:21]

https://codereview.chromium.org/289013002/diff/200001/chrome/browser/extensio...
File
chrome/browser/extensions/api/screenlock_private/screenlock_private_apitest.cc
(right):

https://codereview.chromium.org/289013002/diff/200001/chrome/browser/extensio...
chrome/browser/extensions/api/screenlock_private/screenlock_private_apitest.cc:35:
// New profile managerment needs to be on for non-ChromeOS lock.
On 2014/05/19 17:53:47, Antony Sargent wrote:
> typo: managerment -> management

Done.

[Roger Tawa OOO till Jul 10th, 2014-05-20 13:34:09]

screenlock_bridge.h lgtm

[xiyuan, 2014-05-20 17:10:27]

The CQ bit was checked by xiyuan@chromium.org

[commit-bot: I haz the power, 2014-05-20 17:11:31]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/xiyuan@chromium.org/289013002/240001

[commit-bot: I haz the power, 2014-05-20 18:10:34]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[xiyuan, 2014-05-20 18:10:55]

The CQ bit was unchecked by xiyuan@chromium.org

[xiyuan, 2014-05-20 18:13:47]

Committed patchset #11 manually as r271714 (presubmit successful).