Added a check for the utf8 string input being empty (NULL).

Minor style and typo clean-ups. Also, a minor change to the test to close a page after it is done to clean up state associated with it.

BUG=76162
TEST=SearchProviderTest.TestIsSearchProviderInstalledWithException


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

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

4 files changed, 77 insertions, 10 deletions

diff --git a/chrome/renderer/external_extension.cc b/chrome/renderer/external_extension.cc
index f101333..6d54afc 100644
--- a/chrome/renderer/external_extension.cc
+++ b/chrome/renderer/external_extension.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -143,10 +143,10 @@ v8::Handle<v8::Value> ExternalExtensionWrapper::AddSearchProvider(
 v8::Handle<v8::Value> ExternalExtensionWrapper::IsSearchProviderInstalled(
     const v8::Arguments& args) {
   if (!args.Length()) return v8::Undefined();
+  v8::String::Utf8Value utf8name(args[0]);
+  if (!utf8name.length()) return v8::Undefined();
 
-  std::string name = std::string(*v8::String::Utf8Value(args[0]));
-  if (!name.length()) return v8::Undefined();
-
+  std::string name = std::string(*utf8name);
   RenderView* render_view = GetRenderView();
   if (!render_view) return v8::Undefined();
 
diff --git a/chrome/renderer/external_extension_uitest.cc b/chrome/renderer/external_extension_uitest.cc
index 427ec16..a5d8475 100644
--- a/chrome/renderer/external_extension_uitest.cc
+++ b/chrome/renderer/external_extension_uitest.cc
@@ -28,6 +28,7 @@ class SearchProviderTest : public UITest {
       const IsSearchProviderTestData& data);
 
   net::TestServer test_server_;
+  GURL search_provider_test_url_;
   bool test_server_started_;
 
  private:
@@ -50,6 +51,10 @@ SearchProviderTest::SearchProviderTest()
   // Map all hosts to our local server.
   std::string host_rule("MAP * " + test_server_.host_port_pair().ToString());
   launch_arguments_.AppendSwitchASCII(switches::kHostRules, host_rule);
+
+  // Get the url for the test page.
+  search_provider_test_url_ =
+      test_server_.GetURL("files/is_search_provider_installed.html");
 }
 
 struct IsSearchProviderTestData {
@@ -93,10 +98,8 @@ IsSearchProviderTestData SearchProviderTest::StartIsSearchProviderInstalledTest(
   }
 
   // Go to the test page.
-  GURL local_url =
-      test_server_.GetURL("files/is_search_provider_installed.html");
-  GURL test_url(std::string("http://") + host + local_url.path() +
-                "#" + expected_result);
+  GURL test_url(std::string("http://") + host +
+                search_provider_test_url_.path() + "#" + expected_result);
   EXPECT_TRUE(tab->NavigateToURLAsync(test_url));
 
   // Bundle up information needed to verify the result.
@@ -122,6 +125,7 @@ void SearchProviderTest::FinishIsSearchProviderInstalledTest(
   value += "\n";
   ReplaceSubstringsAfterOffset(&value, 0, "\r", "");
   EXPECT_STREQ("1\n", value.c_str());
+  EXPECT_TRUE(data.tab->Close(true));
 }
 
 // http://code.google.com/p/chromium/issues/detail?id=62777
@@ -173,3 +177,14 @@ TEST_F(SearchProviderTest, FLAKY_TestIsSearchProviderInstalled) {
   }
 #endif
 }
+
+TEST_F(SearchProviderTest, TestIsSearchProviderInstalledWithException) {
+  // Change the url for the test page to one that throws an exception when
+  // toString is called on the argument given to isSearchProviderInstalled.
+  search_provider_test_url_ = test_server_.GetURL(
+      "files/is_search_provider_installed_with_exception.html");
+
+  scoped_refptr<BrowserProxy> browser(automation()->GetBrowserWindow(0));
+  FinishIsSearchProviderInstalledTest(StartIsSearchProviderInstalledTest(
+      browser, "www.google.com", ""));
+}
diff --git a/chrome/test/data/is_search_provider_installed.html b/chrome/test/data/is_search_provider_installed.html
index 5e51c17..85de2a9 100644
--- a/chrome/test/data/is_search_provider_installed.html
+++ b/chrome/test/data/is_search_provider_installed.html
@@ -52,7 +52,7 @@ try {
   var originWithDifferentPort = document.location.protocol + "//" +
       document.location.hostname + differentPort + "/";
 
-  // Verify existance of the api.
+  // Verify existence of the api.
   var foundApi = false;
   try {
     if (window.external.IsSearchProviderInstalled)
@@ -83,7 +83,8 @@ try {
 
   writeResult();
 } catch (e) {
-  logFailed("An exception occurred. Name: " + e.name + " Message: " + e.message);
+  logFailed("An exception occurred. Name: " + e.name + " Message: " +
+            e.message);
   writeResult();
 }
 </script>
diff --git a/chrome/test/data/is_search_provider_installed_with_exception.html b/chrome/test/data/is_search_provider_installed_with_exception.html
new file mode 100644
index 0000000..73c5118
--- /dev/null
+++ b/chrome/test/data/is_search_provider_installed_with_exception.html
@@ -0,0 +1,51 @@
+<html>
+<body>
+<p>Test IsProviderInstalled.<p>
+<div id=result>
+</div>
+<script>
+var passedAll = true;
+
+function log(message) {
+  document.getElementById("result").innerHTML += message + "<br>";
+}
+
+function logPassed(message) {
+  log("PASS: " + message);
+}
+
+function logFailed(message) {
+  passedAll = false;
+  log("FAIL: " + message);
+}
+
+function writeResult() {
+  var result = "1";
+  if (passedAll)
+    logPassed("Everything passed.");
+  else {
+    logFailed("At least one test failed.");
+    result = " " + document.body.innerText; // Add a space to ensure that the
+					    // result doesn't resemble success.
+  }
+  document.cookie = document.location.hostname + "testResult=" + escape(result);
+}
+
+function MyObject() {
+}
+
+try {
+  MyObject.prototype.toString = function() { throw "test"; }
+  var object = new MyObject();
+
+  // Verify the search provider state for the current page.
+  var installed = window.external.IsSearchProviderInstalled(object)
+  writeResult();
+} catch (e) {
+  logFailed("An exception occurred. Name: " + e.name + " Message: " +
+            e.message);
+  writeResult();
+}
+</script>
+</body>
+</html>