Implementation of AutomationEventQueue and associated framework to support generic non-blocking aut…

chrome/browser/automation/testing_automation_provider.cc

Index: chrome/browser/automation/testing_automation_provider.cc
diff --git a/chrome/browser/automation/testing_automation_provider.cc b/chrome/browser/automation/testing_automation_provider.cc
index afc251a4d660590022094a732d548c3ad5f630a6..446b64564bb4954dcb0818f4d3d591d254e46f83 100644
--- a/chrome/browser/automation/testing_automation_provider.cc
+++ b/chrome/browser/automation/testing_automation_provider.cc
@@ -2316,6 +2316,16 @@ void TestingAutomationProvider::SendJSONRequest(int handle,
   handler_map["SetPrefs"] = &TestingAutomationProvider::SetPrefs;
   handler_map["ExecuteJavascript"] =
       &TestingAutomationProvider::ExecuteJavascriptJSON;
+  handler_map["AddDomRaisedEventObserver"] =
+      &TestingAutomationProvider::AddDomRaisedEventObserver;
+  handler_map["RemoveEventObserver"] =
+      &TestingAutomationProvider::RemoveEventObserver;
+  handler_map["GetQueuedEvent"] =
+      &TestingAutomationProvider::GetQueuedEvent;
+  handler_map["ClearQueuedEvents"] =
+      &TestingAutomationProvider::ClearQueuedEvents;
+  handler_map["ClearEventObservers"] =
+      &TestingAutomationProvider::ClearEventObservers;
   handler_map["ExecuteJavascriptInRenderView"] =
       &TestingAutomationProvider::ExecuteJavascriptInRenderView;
   handler_map["GoForward"] =
@@ -6443,6 +6453,72 @@ void TestingAutomationProvider::ExecuteJavascriptInRenderView(
                                      rvh);
 }
 
+void TestingAutomationProvider::AddDomRaisedEventObserver(
+    DictionaryValue* args,
+    IPC::Message* reply_message) {
+  if (SendErrorIfModalDialogActive(this, reply_message))
+    return;
+

[Nirnimesh, 2012/02/28 09:13:09]

Create an instance of AutomationJSONReply

[craigdh, 2012/02/28 22:42:56]

Done.

+  std::string event_name;
+  if (!args->GetString("event_name", &event_name)) {
+    AutomationJSONReply(this, reply_message)
+        .SendError("'event_name' missing or invalid");

[Nirnimesh, 2012/02/28 09:13:09]

then use it here

[craigdh, 2012/02/28 22:42:56]

Done.

+    return;
+  }
+
+  DomRaisedEventObserver* jsobserver =
+      new DomRaisedEventObserver(automation_event_queue_, event_name);

[Nirnimesh, 2012/02/28 09:13:09]

Since automation_event_queue_ owns the new observer, to make the ownership
clearer, this should probably be like:

automation_event_queue->AddEventObserver(new DomRaisedEventObserver(...))

Then it'd match line 6485 as well

[craigdh, 2012/02/28 22:42:56]

Done.

+  scoped_ptr<DictionaryValue> return_value(new DictionaryValue);
+  return_value->SetInteger("observer_id", jsobserver->GetId());
+  AutomationJSONReply(this, reply_message).SendSuccess(return_value.get());

[Nirnimesh, 2012/02/28 09:13:09]

and here

[craigdh, 2012/02/28 22:42:56]

Done.

+}
+
+void TestingAutomationProvider::RemoveEventObserver(
+    DictionaryValue* args,
+    IPC::Message* reply_message) {
+  int observer_id;

[Nirnimesh, 2012/02/28 09:13:09]

create AutomationJSONReply once, the use it in line 6482 & 6486

[craigdh, 2012/02/28 22:42:56]

Done.

+  if (!args->GetInteger("observer_id", &observer_id)) {
+    AutomationJSONReply(this, reply_message)
+        .SendError("'observer_id' missing or invalid");
+    return;
+  }
+  automation_event_queue_.RemoveObserver(observer_id);
+  AutomationJSONReply(this, reply_message).SendSuccess(NULL);
+}
+
+void TestingAutomationProvider::ClearQueuedEvents(
+    DictionaryValue* args,
+    IPC::Message* reply_message) {
+  automation_event_queue_.Clear();

[Nirnimesh, 2012/02/28 09:13:09]

Does this really free up the observers?

[craigdh, 2012/02/28 22:42:56]

I've now merged ClearEventObservers() into Clear() to simplify the API.

+  AutomationJSONReply(this, reply_message).SendSuccess(NULL);
+}
+
+void TestingAutomationProvider::ClearEventObservers(

[Nirnimesh, 2012/02/28 09:13:09]

As a user, I don't understand how this differs from the previous method

[craigdh, 2012/02/28 22:42:56]

See previous.

+    DictionaryValue* args,
+    IPC::Message* reply_message) {
+  automation_event_queue_.ClearObservers();
+  AutomationJSONReply(this, reply_message).SendSuccess(NULL);
+}
+
+void TestingAutomationProvider::GetQueuedEvent(

[Nirnimesh, 2012/02/28 09:13:09]

rename to: GetNextEvent?

[craigdh, 2012/02/28 22:42:56]

Done.

+    DictionaryValue* args,
+    IPC::Message* reply_message) {
+  int observer_id;
+  bool blocking;

[Nirnimesh, 2012/02/28 09:13:09]

define AutomationJSONReply here and use it below

[craigdh, 2012/02/28 22:42:56]

Done.

+  if (!args->GetInteger("observer_id", &observer_id)) {
+    AutomationJSONReply(this, reply_message)
+        .SendError("'observer_id' missing or invalid");
+    return;
+  }
+  if (!args->GetBoolean("blocking", &blocking)) {
+    AutomationJSONReply(this, reply_message)
+        .SendError("'blocking' missing or invalid");
+    return;
+  }
+
+  automation_event_queue_.GetEvent(this, reply_message, observer_id, blocking);

[Nirnimesh, 2012/02/28 09:13:09]

does this respond back to the client?
Why not fetch the data and return it from here?

[craigdh, 2012/02/28 22:42:56]

Because if there is no matching event in the queue then this function will
return without sending an automation reply and the AutomationEventQueue will
take ownership of the AutomationJSONReply and respond once a matching event is
added. It's similar to the observers in automation_provider_observers.h/cc.

+}
+
 void TestingAutomationProvider::GoForward(
     DictionaryValue* args,
     IPC::Message* reply_message) {