autotestPrivate.loginStatus returns a dictionary instead of a string.

autotestPrivate.loginStatus returns a dictionary instead of a string.

BUG=225911
TEST=manual + browser test ExtensionApi.AutotestPrivate

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

[achuithb, 2013-04-04 00:07:44]

Matt: I'd like to change the signature of this method
(autotestPrivate.loginStatus). Do I need to get this reviewed by the extension
review committee? This api is currently unused.

[Matt Perry, 2013-04-04 00:12:03]

This is a private API, so no need to get it reviewed again for this.

LGTM with the change below.

https://codereview.chromium.org/13520005/diff/6001/chrome/common/extensions/a...
File chrome/common/extensions/api/autotest_private.idl (right):

https://codereview.chromium.org/13520005/diff/6001/chrome/common/extensions/a...
chrome/common/extensions/api/autotest_private.idl:20: long user_image;
use camelCasing style for these.

[achuithb, 2013-04-04 00:25:37]

Thanks!

[achuithb, 2013-04-04 00:42:28]

Matt, I just realized that userImage has to be of type DOMString. Could you
please re-review this CL?

https://codereview.chromium.org/13520005/diff/6001/chrome/common/extensions/a...
File chrome/common/extensions/api/autotest_private.idl (right):

https://codereview.chromium.org/13520005/diff/6001/chrome/common/extensions/a...
chrome/common/extensions/api/autotest_private.idl:20: long user_image;
On 2013/04/04 00:12:03, Matt Perry wrote:
> use camelCasing style for these.

Done.

[Matt Perry, 2013-04-04 00:56:18]

lgtm

https://codereview.chromium.org/13520005/diff/12001/chrome/common/extensions/...
File chrome/common/extensions/api/autotest_private.idl (right):

https://codereview.chromium.org/13520005/diff/12001/chrome/common/extensions/...
chrome/common/extensions/api/autotest_private.idl:20: DOMString userImage;
Might want to comment these properties to make it clearer what they mean and
what values they can take.

[xiyuan, 2013-04-04 16:25:33]

LGTM

https://codereview.chromium.org/13520005/diff/12001/chrome/browser/extensions...
File chrome/browser/extensions/api/autotest_private/autotest_private_api.cc
(right):

https://codereview.chromium.org/13520005/diff/12001/chrome/browser/extensions...
chrome/browser/extensions/api/autotest_private/autotest_private_api.cc:60:
chromeos::ScreenLocker::default_screen_locker();
nit: Suggest to explicitly convert it to a boolean here since we just use the
pointer for a boolean flag.

e.g.
const bool is_screen_locked = !!chromeos::ScreenLocker::default_screen_locker();

[achuithb, 2013-04-04 17:58:06]

Thanks for the feedback, Matt and Xiyuan!

https://codereview.chromium.org/13520005/diff/12001/chrome/browser/extensions...
File chrome/browser/extensions/api/autotest_private/autotest_private_api.cc
(right):

https://codereview.chromium.org/13520005/diff/12001/chrome/browser/extensions...
chrome/browser/extensions/api/autotest_private/autotest_private_api.cc:60:
chromeos::ScreenLocker::default_screen_locker();
On 2013/04/04 16:25:33, xiyuan wrote:
> nit: Suggest to explicitly convert it to a boolean here since we just use the
> pointer for a boolean flag.
> 
> e.g.
> const bool is_screen_locked =
!!chromeos::ScreenLocker::default_screen_locker();

Done.

https://codereview.chromium.org/13520005/diff/12001/chrome/common/extensions/...
File chrome/common/extensions/api/autotest_private.idl (right):

https://codereview.chromium.org/13520005/diff/12001/chrome/common/extensions/...
chrome/common/extensions/api/autotest_private.idl:20: DOMString userImage;
On 2013/04/04 00:56:18, Matt Perry wrote:
> Might want to comment these properties to make it clearer what they mean and
> what values they can take.

Done.

[commit-bot: I haz the power, 2013-04-04 17:59:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/achuith@chromium.org/13520005/20006

[commit-bot: I haz the power, 2013-04-04 18:03:17]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2013-04-04 18:23:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/achuith@chromium.org/13520005/30002

[commit-bot: I haz the power, 2013-04-04 18:27:11]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2013-04-05 18:55:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/achuith@chromium.org/13520005/30002

[commit-bot: I haz the power, 2013-04-05 23:55:18]

Change committed as 192655