Rewrite of the EnrollEnterpriseDevice PyAuto automation hook for WebUI.

Rewrite of the EnrollEnterpriseDevice PyAuto automation hook for WebUI.

The enroll device automation hook previously never returned when enrollment
failed. This rewrite will throw an EnrollmentError when enrollment fails
with a string describing the cause of the error. This rewrite will also
(hopefully) fix flakiness issues seen with the previous hook.

BUG=chromium-os:29466, chromium-os:22280, chromium-os:25640
TEST=desktopui_EnterprisePolicyServer

Change-Id: I380c7047d6feb1934811ebe66d69fd1e426039bc

[Mattias Nissler (ping if slow), 2012-05-23 19:47:50]

I like the direction in general, but I cannot see why this would fix flakiness.
Do you know why the test is flaky?

http://codereview.chromium.org/10408072/diff/1/chrome/browser/automation/auto...
File chrome/browser/automation/automation_event_observers.h (right):

http://codereview.chromium.org/10408072/diff/1/chrome/browser/automation/auto...
chrome/browser/automation/automation_event_observers.h:122: bool succeeded,
std::string error_string);
All parameters go on separate lines in declarations. Strings should be passed as
const references.

http://codereview.chromium.org/10408072/diff/1/chrome/browser/automation/auto...
chrome/browser/automation/automation_event_observers.h:125: 
nit: remove blank line.

http://codereview.chromium.org/10408072/diff/1/chrome/browser/automation/auto...
File chrome/browser/automation/automation_event_observers_chromeos.cc (right):

http://codereview.chromium.org/10408072/diff/1/chrome/browser/automation/auto...
chrome/browser/automation/automation_event_observers_chromeos.cc:63: if
(!succeeded) {
nit: no curlies required (be consistent with existing code in the file).

http://codereview.chromium.org/10408072/diff/1/chrome/browser/chromeos/login/...
File
chrome/browser/chromeos/login/enrollment/enterprise_enrollment_screen_actor.h
(right):

http://codereview.chromium.org/10408072/diff/1/chrome/browser/chromeos/login/...
chrome/browser/chromeos/login/enrollment/enterprise_enrollment_screen_actor.h:40:
bool succeeded, std::string error_string) = 0;
separate line, const ref.

http://codereview.chromium.org/10408072/diff/1/chrome/browser/chromeos/login/...
chrome/browser/chromeos/login/enrollment/enterprise_enrollment_screen_actor.h:81:
void NotifyObservers(bool succeeded, std::string error_string);
ditto.

[craigdh, 2012-05-24 21:17:25]

I do not know for certain why it was flaky before, but frankf reported a flake
rate of 1/20 and I have not seen any flakiness yet with the rewrite. If we do
see any flakiness in the future we will have a descriptive error message of what
went wrong.

https://chromiumcodereview.appspot.com/10408072/diff/1/chrome/browser/automat...
File chrome/browser/automation/automation_event_observers.h (right):

https://chromiumcodereview.appspot.com/10408072/diff/1/chrome/browser/automat...
chrome/browser/automation/automation_event_observers.h:122: bool succeeded,
std::string error_string);
On 2012/05/23 19:47:50, Mattias Nissler wrote:
> All parameters go on separate lines in declarations. Strings should be passed
as
> const references.

Done.

https://chromiumcodereview.appspot.com/10408072/diff/1/chrome/browser/automat...
chrome/browser/automation/automation_event_observers.h:125: 
On 2012/05/23 19:47:50, Mattias Nissler wrote:
> nit: remove blank line.

Done.

https://chromiumcodereview.appspot.com/10408072/diff/1/chrome/browser/automat...
File chrome/browser/automation/automation_event_observers_chromeos.cc (right):

https://chromiumcodereview.appspot.com/10408072/diff/1/chrome/browser/automat...
chrome/browser/automation/automation_event_observers_chromeos.cc:63: if
(!succeeded) {
On 2012/05/23 19:47:50, Mattias Nissler wrote:
> nit: no curlies required (be consistent with existing code in the file).

Done.

https://chromiumcodereview.appspot.com/10408072/diff/1/chrome/browser/chromeo...
File
chrome/browser/chromeos/login/enrollment/enterprise_enrollment_screen_actor.h
(right):

https://chromiumcodereview.appspot.com/10408072/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/login/enrollment/enterprise_enrollment_screen_actor.h:40:
bool succeeded, std::string error_string) = 0;
On 2012/05/23 19:47:50, Mattias Nissler wrote:
> separate line, const ref.

Done.

https://chromiumcodereview.appspot.com/10408072/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/login/enrollment/enterprise_enrollment_screen_actor.h:81:
void NotifyObservers(bool succeeded, std::string error_string);
On 2012/05/23 19:47:50, Mattias Nissler wrote:
> ditto.

Done.

[Mattias Nissler (ping if slow), 2012-05-25 13:17:22]

LGTM once you fix the remaining nits

http://codereview.chromium.org/10408072/diff/6001/chrome/browser/automation/a...
File chrome/browser/automation/automation_event_observers_chromeos.cc (right):

http://codereview.chromium.org/10408072/diff/6001/chrome/browser/automation/a...
chrome/browser/automation/automation_event_observers_chromeos.cc:60: bool
succeeded, const std::string& error_string) {
each parameter on separate line.

http://codereview.chromium.org/10408072/diff/6001/chrome/browser/automation/t...
File chrome/browser/automation/testing_automation_provider_chromeos.cc (right):

http://codereview.chromium.org/10408072/diff/6001/chrome/browser/automation/t...
chrome/browser/automation/testing_automation_provider_chromeos.cc:950: }
The steps for getting the enrollment screen are now duplicated. Move to a helper
function?

http://codereview.chromium.org/10408072/diff/6001/chrome/browser/chromeos/log...
File
chrome/browser/chromeos/login/enrollment/enterprise_enrollment_screen_actor.cc
(right):

http://codereview.chromium.org/10408072/diff/6001/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/enrollment/enterprise_enrollment_screen_actor.cc:22:
bool succeeded, const std::string& error_string) {
separate lines

http://codereview.chromium.org/10408072/diff/6001/chrome/browser/ui/webui/chr...
File
chrome/browser/ui/webui/chromeos/login/enterprise_oauth_enrollment_screen_handler.cc
(right):

http://codereview.chromium.org/10408072/diff/6001/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/login/enterprise_oauth_enrollment_screen_handler.cc:388:
NotifyObservers(false, "Oauth enrollment cancelled");
Should be "OAuth" (capital A). Also, I feel a bit bad of having literal string
in the code that's just for automation. Ccould you add a comment saying that
this string is just for automation?

[craigdh, 2012-05-25 23:24:47]

https://chromiumcodereview.appspot.com/10408072/diff/6001/chrome/browser/auto...
File chrome/browser/automation/automation_event_observers_chromeos.cc (right):

https://chromiumcodereview.appspot.com/10408072/diff/6001/chrome/browser/auto...
chrome/browser/automation/automation_event_observers_chromeos.cc:60: bool
succeeded, const std::string& error_string) {
On 2012/05/25 13:17:22, Mattias Nissler wrote:
> each parameter on separate line.

Done.

https://chromiumcodereview.appspot.com/10408072/diff/6001/chrome/browser/auto...
File chrome/browser/automation/testing_automation_provider_chromeos.cc (right):

https://chromiumcodereview.appspot.com/10408072/diff/6001/chrome/browser/auto...
chrome/browser/automation/testing_automation_provider_chromeos.cc:950: }
On 2012/05/25 13:17:22, Mattias Nissler wrote:
> The steps for getting the enrollment screen are now duplicated. Move to a
helper
> function?

Done.

https://chromiumcodereview.appspot.com/10408072/diff/6001/chrome/browser/chro...
File
chrome/browser/chromeos/login/enrollment/enterprise_enrollment_screen_actor.cc
(right):

https://chromiumcodereview.appspot.com/10408072/diff/6001/chrome/browser/chro...
chrome/browser/chromeos/login/enrollment/enterprise_enrollment_screen_actor.cc:22:
bool succeeded, const std::string& error_string) {
On 2012/05/25 13:17:22, Mattias Nissler wrote:
> separate lines

Done.

https://chromiumcodereview.appspot.com/10408072/diff/6001/chrome/browser/ui/w...
File
chrome/browser/ui/webui/chromeos/login/enterprise_oauth_enrollment_screen_handler.cc
(right):

https://chromiumcodereview.appspot.com/10408072/diff/6001/chrome/browser/ui/w...
chrome/browser/ui/webui/chromeos/login/enterprise_oauth_enrollment_screen_handler.cc:388:
NotifyObservers(false, "Oauth enrollment cancelled");
On 2012/05/25 13:17:22, Mattias Nissler wrote:
> Should be "OAuth" (capital A). Also, I feel a bit bad of having literal string
> in the code that's just for automation. Ccould you add a comment saying that
> this string is just for automation? 

Done. Including a string literal is certainly not ideal, but it beats having
tests fail with no error message.

[bartfab (slow), 2012-11-30 12:51:28]

I assume that this CL has been deprecated along with PyAuto. Removing myself as
reviewer to unclutter the "Reviewable by me" list in code review :).

[craigdh, 2012-11-30 17:30:46]

On 2012/11/30 12:51:28, bartfab wrote:
> I assume that this CL has been deprecated along with PyAuto. Removing myself
as
> reviewer to unclutter the "Reviewable by me" list in code review :).

Abandoned. PyAuto has been deprecated.