Add PyAuto test for DeviceGuestModeEnabled policy

Add PyAuto test for DeviceGuestModeEnabled policy

This CL adds a PyAuto test for the DeviceGuestModeEnabled policy which
enables/disables guest login on Chrome OS devices.
As a prerequisite, the browser automation framework is fixed so that it
does not wait for the login screen to be shown when the account picker
is enabled instead.

BUG=128343
TEST=New PyAuto ChromeosDevicePolicy.testGuestModeEnabled


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=137521

[bartfab (slow), 2012-05-16 14:10:35]

Hi Nirnimesh, Nikita,

Could you please review? Thanks.

[Nirnimesh, 2012-05-16 15:44:37]

LGTM

https://chromiumcodereview.appspot.com/10382197/diff/3001/chrome/test/functio...
File chrome/test/functional/chromeos_device_policy.py (right):

https://chromiumcodereview.appspot.com/10382197/diff/3001/chrome/test/functio...
chrome/test/functional/chromeos_device_policy.py:74:
!!document.getElementById('pod-row').
is that a double negation?

https://chromiumcodereview.appspot.com/10382197/diff/3001/chrome/test/functio...
chrome/test/functional/chromeos_device_policy.py:75:
getPodWithUsername_(''));""")
It looks like this fits in the previous line. Doesn't it?
You can keep the """ on this line

https://chromiumcodereview.appspot.com/10382197/diff/3001/chrome/test/functio...
chrome/test/functional/chromeos_device_policy.py:95: # Log in in as a regular so
that the pod row contains at least one pod and
remove double 'in'

[Nikita (slow), 2012-05-16 15:56:12]

lgtm but you should use existing notification(s)

https://chromiumcodereview.appspot.com/10382197/diff/3001/chrome/browser/auto...
File chrome/browser/automation/automation_provider_observers_chromeos.cc
(right):

https://chromiumcodereview.appspot.com/10382197/diff/3001/chrome/browser/auto...
chrome/browser/automation/automation_provider_observers_chromeos.cc:56:
registrar_.Add(this, chrome::NOTIFICATION_ACCOUNT_PICKER_WEBUI_READY,
Should add these instead:
NOTIFICATION_LOGIN_USER_IMAGES_LOADED
NOTIFICATION_WIZARD_FIRST_SCREEN_SHOWN

http://code.google.com/codesearch#OAMlx_jo-ck/src/chrome/browser/chromeos/log...

https://chromiumcodereview.appspot.com/10382197/diff/3001/chrome/browser/ui/w...
File chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc (right):

https://chromiumcodereview.appspot.com/10382197/diff/3001/chrome/browser/ui/w...
chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc:920:
content::NotificationService::current()->Notify(
I don't think that you need that new notification, see another comment in this
file.

https://chromiumcodereview.appspot.com/10382197/diff/3001/chrome/browser/ui/w...
chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc:997:
chrome::NOTIFICATION_LOGIN_USER_IMAGES_LOADED,
This one is almost the same notification. It is called a bit later, when user
images are loaded.

[bartfab (slow), 2012-05-16 18:07:10]

https://chromiumcodereview.appspot.com/10382197/diff/3001/chrome/browser/auto...
File chrome/browser/automation/automation_provider_observers_chromeos.cc
(right):

https://chromiumcodereview.appspot.com/10382197/diff/3001/chrome/browser/auto...
chrome/browser/automation/automation_provider_observers_chromeos.cc:56:
registrar_.Add(this, chrome::NOTIFICATION_ACCOUNT_PICKER_WEBUI_READY,
Thanks. I had seen that code but did not realize that these are the four
notifications that a browser start into the login manager can end with. Done.

https://chromiumcodereview.appspot.com/10382197/diff/3001/chrome/browser/ui/w...
File chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc (right):

https://chromiumcodereview.appspot.com/10382197/diff/3001/chrome/browser/ui/w...
chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc:920:
content::NotificationService::current()->Notify(
On 2012/05/16 15:56:13, Nikita Kostylev wrote:
> I don't think that you need that new notification, see another comment in this
> file.

Done.

https://chromiumcodereview.appspot.com/10382197/diff/3001/chrome/browser/ui/w...
chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc:997:
chrome::NOTIFICATION_LOGIN_USER_IMAGES_LOADED,
On 2012/05/16 15:56:13, Nikita Kostylev wrote:
> This one is almost the same notification. It is called a bit later, when user
> images are loaded.

Done.

https://chromiumcodereview.appspot.com/10382197/diff/3001/chrome/test/functio...
File chrome/test/functional/chromeos_device_policy.py (right):

https://chromiumcodereview.appspot.com/10382197/diff/3001/chrome/test/functio...
chrome/test/functional/chromeos_device_policy.py:74:
!!document.getElementById('pod-row').
Yes, a common way to cast to bool in JavaScript, used many times in Chrome's
code. I can replace this with Boolean() if you prefer.

https://chromiumcodereview.appspot.com/10382197/diff/3001/chrome/test/functio...
chrome/test/functional/chromeos_device_policy.py:75:
getPodWithUsername_(''));""")
If I leave the """ on their own line, the rest fits onto the previous line.
Done.

https://chromiumcodereview.appspot.com/10382197/diff/3001/chrome/test/functio...
chrome/test/functional/chromeos_device_policy.py:95: # Log in in as a regular so
that the pod row contains at least one pod and
On 2012/05/16 15:44:37, Nirnimesh wrote:
> remove double 'in'

Done.

[commit-bot: I haz the power, 2012-05-16 19:34:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bartfab@chromium.org/10382197/8001

[Nirnimesh, 2012-05-16 19:48:26]

https://chromiumcodereview.appspot.com/10382197/diff/8001/chrome/browser/auto...
File chrome/browser/automation/automation_provider_observers_chromeos.cc
(right):

https://chromiumcodereview.appspot.com/10382197/diff/8001/chrome/browser/auto...
chrome/browser/automation/automation_provider_observers_chromeos.cc:57:
registrar_.Add(this, chrome::NOTIFICATION_LOGIN_NETWORK_ERROR_SHOWN,
When is this raised?
It takes a while for the login dialog to show up, and the network error screen 
before that for a few seconds. Is that it?

[commit-bot: I haz the power, 2012-05-16 21:32:08]

Change committed as 137521

[Nikita (slow), 2012-05-22 11:09:16]

https://chromiumcodereview.appspot.com/10382197/diff/8001/chrome/browser/auto...
File chrome/browser/automation/automation_provider_observers_chromeos.cc
(right):

https://chromiumcodereview.appspot.com/10382197/diff/8001/chrome/browser/auto...
chrome/browser/automation/automation_provider_observers_chromeos.cc:57:
registrar_.Add(this, chrome::NOTIFICATION_LOGIN_NETWORK_ERROR_SHOWN,
On 2012/05/16 19:48:26, Nirnimesh wrote:
> When is this raised?
> It takes a while for the login dialog to show up, and the network error screen

> before that for a few seconds. Is that it?

Yes, it is that one. So in that case you'll see
NOTIFICATION_LOGIN_NETWORK_ERROR_SHOWN then
NOTIFICATION_LOGIN_WEBUI_READY

Now I've realized that for the purposes of autotest we probably don't want to
treat that as "LoginWebuiReady" as it is not ready but in fact in some error or
temporary (while network is connecting) state.

Sorry for confusion.

[bartfab (slow), 2012-05-22 11:13:35]

No, you actually had that right from the start. You never listed that
notification as one I should listen for. I just copied the whole list from
WebUILoginView::Observe(), picking up this extra notification.