C++ readability review with changelist for providing dom automation support to browser tests.

BUG=none
TEST=none
Review URL: http://codereview.chromium.org/1135003

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

6 files changed, 32 insertions, 15 deletions

diff --git a/chrome/test/automation/dom_automation_browsertest.cc b/chrome/test/automation/dom_automation_browsertest.cc
index 85ef229..f922e0e 100644
--- a/chrome/test/automation/dom_automation_browsertest.cc
+++ b/chrome/test/automation/dom_automation_browsertest.cc
@@ -12,6 +12,8 @@
 
 namespace {
 
+// Tests the DOMAutomation framework for manipulating DOMElements within
+// browser tests.
 class DOMAutomationTest : public InProcessBrowserTest {
  public:
   DOMAutomationTest() {
diff --git a/chrome/test/automation/dom_element_proxy.cc b/chrome/test/automation/dom_element_proxy.cc
index feeeebf..894429f 100644
--- a/chrome/test/automation/dom_element_proxy.cc
+++ b/chrome/test/automation/dom_element_proxy.cc
@@ -94,7 +94,10 @@ DOMElementProxyRef DOMElementProxy::FindElement(const By& by) {
 
 bool DOMElementProxy::FindElements(const By& by,
                                    std::vector<DOMElementProxyRef>* elements) {
-  DCHECK(elements);
+  if (!elements) {
+    NOTREACHED();
+    return false;
+  }
   if (!is_valid())
     return false;
 
@@ -114,7 +117,10 @@ bool DOMElementProxy::FindElements(const By& by,
 
 bool DOMElementProxy::WaitForVisibleElementCount(
     const By& by, int count, std::vector<DOMElementProxyRef>* elements) {
-  DCHECK(elements);
+  if (!elements) {
+    NOTREACHED();
+    return false;
+  }
   if (!is_valid())
     return false;
 
@@ -185,7 +191,10 @@ bool DOMElementProxy::SetText(const std::string& text) {
 
 bool DOMElementProxy::GetProperty(const std::string& property,
                                   std::string* out) {
-  DCHECK(out);
+  if (!out) {
+    NOTREACHED();
+    return false;
+  }
   if (!is_valid())
     return false;
 
@@ -197,7 +206,10 @@ bool DOMElementProxy::GetProperty(const std::string& property,
 
 bool DOMElementProxy::GetAttribute(const std::string& attribute,
                                    std::string* out) {
-  DCHECK(out);
+  if (!out) {
+    NOTREACHED();
+    return false;
+  }
   if (!is_valid())
     return false;
 
@@ -271,13 +283,16 @@ void DOMElementProxy::EnsureAttributeEventuallyMatches(
 }
 
 template <typename T>
-bool DOMElementProxy::GetValue(const std::string& type, T* out) {
-  DCHECK(out);
+bool DOMElementProxy::GetValue(const std::string& type, T* value) {
+  if (!value) {
+    NOTREACHED();
+    return false;
+  }
   if (!is_valid())
     return false;
 
   const char* script = "domAutomation.getValue("
                        "domAutomation.getObject(%s), %s);";
   return executor_->ExecuteJavaScriptAndGetReturn(
-      JavaScriptPrintf(script, this->handle(), type), out);
+      JavaScriptPrintf(script, this->handle(), type), value);
 }
diff --git a/chrome/test/automation/dom_element_proxy.h b/chrome/test/automation/dom_element_proxy.h
index 06d2af3..84bdd55 100644
--- a/chrome/test/automation/dom_element_proxy.h
+++ b/chrome/test/automation/dom_element_proxy.h
@@ -93,7 +93,6 @@ class DOMElementProxy : public JavaScriptObjectProxy {
                                           const std::string& frame_name2 = "",
                                           const std::string& frame_name3 = "");
 
-
   // Finds the first element found by the given locator method |by|, or NULL
   // if no element was found.
   DOMElementProxyRef FindElement(const By& by);
@@ -157,7 +156,7 @@ class DOMElementProxy : public JavaScriptObjectProxy {
   // Retrieves the element's visibility. Returns true on success.
   bool GetVisibility(bool* visilibity);
 
-  // Ensures that no elements can be found by the given locator method.
+  // Asserts that no elements can be found by the given locator method.
   void EnsureFindNoElements(const By& by);
 
   // Asserts that |expected_text| matches all the text in this element. This
diff --git a/chrome/test/automation/javascript_execution_controller.cc b/chrome/test/automation/javascript_execution_controller.cc
index f743c9e..0c266eb 100644
--- a/chrome/test/automation/javascript_execution_controller.cc
+++ b/chrome/test/automation/javascript_execution_controller.cc
@@ -44,7 +44,7 @@ std::string JavaScriptExecutionController::WrapAsyncJavaScript(
     const std::string& original_script) {
   if (timeout_ms_ == -1) {
     NOTREACHED() << "Timeout for asynchronous JavaScript methods has not been "
-                 << "set. Please use JavaScriptExecutionController::"
+                 << "set. Call JavaScriptExecutionController::"
                  << "set_timeout(timeout_in_ms).";
   }
   const char* script =
@@ -74,10 +74,11 @@ bool JavaScriptExecutionController::ExecuteAndParseHelper(
   //   - result (string): the result of the evaluation (in JSON), or the
   //       exact error if an error occurred (in JSON)
   if (!root_value.get()) {
-    if (parsing_error.length())
+    if (parsing_error.length()) {
       LOG(ERROR) << "Cannot parse JSON response: " << parsing_error;
-    else
+    } else {
       LOG(ERROR) << "JSON response is empty";
+    }
     return false;
   }
 
diff --git a/chrome/test/automation/javascript_execution_controller.h b/chrome/test/automation/javascript_execution_controller.h
index 745f90f..ec6a80f 100644
--- a/chrome/test/automation/javascript_execution_controller.h
+++ b/chrome/test/automation/javascript_execution_controller.h
@@ -54,7 +54,7 @@ class JavaScriptExecutionController
   bool ExecuteAsyncJavaScript(const std::string& script);
 
   // Returns the proxy associated with |handle|, creating one if necessary.
-  // The proxy must be a type of JavaScriptObjectProxy.
+  // The proxy must be inherit JavaScriptObjectProxy.
   template<class JavaScriptObject>
   JavaScriptObject* GetObjectProxy(int handle) {
     JavaScriptObject* obj = NULL;
diff --git a/chrome/test/automation/javascript_message_utils.h b/chrome/test/automation/javascript_message_utils.h
index a571b07..aa462a0 100644
--- a/chrome/test/automation/javascript_message_utils.h
+++ b/chrome/test/automation/javascript_message_utils.h
@@ -16,8 +16,8 @@
 #include "chrome/test/automation/dom_element_proxy.h"
 
 // ValueConversionTraits contains functions for creating a value from a
-// type, and setting a type from a value.
-// This is general-purpose and can be moved to a common location if needed.
+// type, and setting a type from a value. This is general-purpose and can
+// be moved to a common location if needed.
 template <class T>
 struct ValueConversionTraits {
 };