Fix AutomationProvider from logging false errors by replacing the persistent

DOM operation observer with one per message. Also merge the two DOM operation
observers for the different automation calls into one.
BUG=none
TEST=none

Review URL: http://codereview.chromium.org/6623071

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@77654 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 26 insertions, 57 deletions

diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc
index 730e484..7fe89a6 100644
--- a/chrome/browser/automation/automation_provider.cc
+++ b/chrome/browser/automation/automation_provider.cc
@@ -125,7 +125,6 @@ AutomationProvider::AutomationProvider(Profile* profile)
   autocomplete_edit_tracker_.reset(
       new AutomationAutocompleteEditTracker(this));
   new_tab_ui_load_observer_.reset(new NewTabUILoadObserver(this));
-  dom_operation_observer_.reset(new DomOperationMessageSender(this));
   metric_event_duration_observer_.reset(new MetricEventDurationObserver());
   extension_test_result_observer_.reset(
       new ExtensionTestResultNotificationObserver(this));
diff --git a/chrome/browser/automation/automation_provider.h b/chrome/browser/automation/automation_provider.h
index 859cd45..53b9428 100644
--- a/chrome/browser/automation/automation_provider.h
+++ b/chrome/browser/automation/automation_provider.h
@@ -392,7 +392,6 @@ class AutomationProvider
   scoped_ptr<IPC::ChannelProxy> channel_;
   scoped_ptr<NotificationObserver> new_tab_ui_load_observer_;
   scoped_ptr<NotificationObserver> find_in_page_observer_;
-  scoped_ptr<NotificationObserver> dom_operation_observer_;
   scoped_ptr<ExtensionTestResultNotificationObserver>
       extension_test_result_observer_;
   scoped_ptr<AutomationExtensionTracker> extension_tracker_;
diff --git a/chrome/browser/automation/automation_provider_observers.cc b/chrome/browser/automation/automation_provider_observers.cc
index 080aa5e..57cc7df 100644
--- a/chrome/browser/automation/automation_provider_observers.cc
+++ b/chrome/browser/automation/automation_provider_observers.cc
@@ -1025,24 +1025,30 @@ void DomOperationObserver::Observe(
 }
 
 DomOperationMessageSender::DomOperationMessageSender(
-    AutomationProvider* automation)
-    : automation_(automation->AsWeakPtr()) {
+    AutomationProvider* automation,
+    IPC::Message* reply_message,
+    bool use_json_interface)
+    : automation_(automation->AsWeakPtr()),
+      reply_message_(reply_message),
+      use_json_interface_(use_json_interface) {
 }
 
 DomOperationMessageSender::~DomOperationMessageSender() {}
 
 void DomOperationMessageSender::OnDomOperationCompleted(
     const std::string& json) {
-  if (!automation_)
-    return;
-
-  IPC::Message* reply_message = automation_->reply_message_release();
-  if (reply_message) {
-    AutomationMsg_DomOperation::WriteReplyParams(reply_message, json);
-    automation_->Send(reply_message);
-  } else {
-    LOG(ERROR) << "DOM operation completed, but no reply message";
+  if (automation_) {
+    if (use_json_interface_) {
+      DictionaryValue dict;
+      dict.SetString("result", json);
+      AutomationJSONReply(automation_, reply_message_.release())
+          .SendSuccess(&dict);
+    } else {
+      AutomationMsg_DomOperation::WriteReplyParams(reply_message_.get(), json);
+      automation_->Send(reply_message_.release());
+    }
   }
+  delete this;
 }
 
 DocumentPrintedNotificationObserver::DocumentPrintedNotificationObserver(
@@ -2137,24 +2143,3 @@ void WaitForProcessLauncherThreadToGoIdleObserver::RunOnUIThread() {
     automation_->Send(reply_message_.release());
   Release();
 }
-
-ExecuteJavascriptObserver::ExecuteJavascriptObserver(
-    AutomationProvider* automation,
-    IPC::Message* reply_message)
-    : automation_(automation->AsWeakPtr()),
-      reply_message_(reply_message) {
-}
-
-ExecuteJavascriptObserver::~ExecuteJavascriptObserver() {
-}
-
-void ExecuteJavascriptObserver::OnDomOperationCompleted(
-    const std::string& json) {
-  if (automation_) {
-    DictionaryValue dict;
-    dict.SetString("result", json);
-    AutomationJSONReply(automation_, reply_message_.release())
-        .SendSuccess(&dict);
-  }
-  delete this;
-}
diff --git a/chrome/browser/automation/automation_provider_observers.h b/chrome/browser/automation/automation_provider_observers.h
index 5eed0ad..f884664 100644
--- a/chrome/browser/automation/automation_provider_observers.h
+++ b/chrome/browser/automation/automation_provider_observers.h
@@ -528,15 +528,21 @@ class DomOperationObserver : public NotificationObserver {
   DISALLOW_COPY_AND_ASSIGN(DomOperationObserver);
 };
 
+// Sends a message back to the automation client with the results of the DOM
+// operation.
 class DomOperationMessageSender : public DomOperationObserver {
  public:
-  explicit DomOperationMessageSender(AutomationProvider* automation);
+  explicit DomOperationMessageSender(AutomationProvider* automation,
+                                     IPC::Message* relpy_message,
+                                     bool use_json_interface);
   virtual ~DomOperationMessageSender();
 
   virtual void OnDomOperationCompleted(const std::string& json);
 
  private:
   base::WeakPtr<AutomationProvider> automation_;
+  scoped_ptr<IPC::Message> reply_message_;
+  bool use_json_interface_;
 
   DISALLOW_COPY_AND_ASSIGN(DomOperationMessageSender);
 };
@@ -1175,22 +1181,4 @@ class WaitForProcessLauncherThreadToGoIdleObserver
   DISALLOW_COPY_AND_ASSIGN(WaitForProcessLauncherThreadToGoIdleObserver);
 };
 
-// Observes the result of execution of Javascript and sends a JSON reply.
-class ExecuteJavascriptObserver : public DomOperationObserver {
- public:
-  ExecuteJavascriptObserver(AutomationProvider* automation,
-                            IPC::Message* reply_message);
-  virtual ~ExecuteJavascriptObserver();
-
- private:
-  // Overriden from DomOperationObserver.
-  virtual void OnDomOperationCompleted(const std::string& json);
-
-  base::WeakPtr<AutomationProvider> automation_;
-  scoped_ptr<IPC::Message> reply_message_;
-
-  DISALLOW_COPY_AND_ASSIGN(ExecuteJavascriptObserver);
-};
-
-
 #endif  // CHROME_BROWSER_AUTOMATION_AUTOMATION_PROVIDER_OBSERVERS_H_
diff --git a/chrome/browser/automation/testing_automation_provider.cc b/chrome/browser/automation/testing_automation_provider.cc
index 8bf7fb7..9c24c4e 100644
--- a/chrome/browser/automation/testing_automation_provider.cc
+++ b/chrome/browser/automation/testing_automation_provider.cc
@@ -1256,9 +1256,7 @@ void TestingAutomationProvider::ExecuteJavascript(
                       "window.domAutomationController.setAutomationId(%d);",
                       reply_message->routing_id());
 
-  DCHECK(!reply_message_);
-  reply_message_ = reply_message;
-
+  new DomOperationMessageSender(this, reply_message, false);
   tab_contents->render_view_host()->ExecuteJavascriptInWebFrame(
       WideToUTF16Hack(frame_xpath), UTF8ToUTF16(set_automation_id));
   tab_contents->render_view_host()->ExecuteJavascriptInWebFrame(
@@ -4925,7 +4923,7 @@ void TestingAutomationProvider::ExecuteJavascriptJSON(
                       "window.domAutomationController.setAutomationId(%d);",
                       reply_message->routing_id());
 
-  new ExecuteJavascriptObserver(this, reply_message);
+  new DomOperationMessageSender(this, reply_message, true);
   tab_contents->render_view_host()->ExecuteJavascriptInWebFrame(
       frame_xpath, UTF8ToUTF16(set_automation_id));
   tab_contents->render_view_host()->ExecuteJavascriptInWebFrame(