Update extension functions to always pass a list of arguments, even when one argument was passed.

BUG=36301
TEST=Updated tests and ran all the existing tests.

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

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

2 files changed, 44 insertions, 45 deletions

diff --git a/chrome/renderer/extensions/extension_api_client_unittest.cc b/chrome/renderer/extensions/extension_api_client_unittest.cc
index 776d694..c44f9ee 100644
--- a/chrome/renderer/extensions/extension_api_client_unittest.cc
+++ b/chrome/renderer/extensions/extension_api_client_unittest.cc
@@ -72,9 +72,8 @@ class ExtensionAPIClientTest : public RenderViewTest {
     ViewHostMsg_ExtensionRequest::Read(request_msg, &params);
     ASSERT_EQ(function.c_str(), params.a) << js;
 
-    Value* args = NULL;
     ASSERT_TRUE(params.b.IsType(Value::TYPE_LIST));
-    ASSERT_TRUE(static_cast<const ListValue*>(&params.b)->Get(0, &args));
+    ListValue* args = &params.b;
 
     base::JSONReader reader;
     scoped_ptr<Value> arg1_value(reader.JsonToValue(arg1, false, false));
@@ -157,7 +156,7 @@ TEST_F(ExtensionAPIClientTest, GetWindow) {
                "Expected 'function' but got 'integer'.");
 
   ExpectJsPass("chrome.windows.get(2, function(){})",
-               "windows.get", "2");
+               "windows.get", "[2]");
 }
 
 // This test flakily crashes (http://crbug.com/22248)
@@ -242,11 +241,11 @@ TEST_F(ExtensionAPIClientTest, CreateWindow) {
                "  height:200"
                "})",
                "windows.create",
-               "{\"url\":\"http://www.google.com/\","
+               "[{\"url\":\"http://www.google.com/\","
                "\"left\":0,"
                "\"top\":10,"
                "\"width\":100,"
-               "\"height\":200}");
+               "\"height\":200}]");
 }
 
 TEST_F(ExtensionAPIClientTest, UpdateWindow) {
@@ -291,10 +290,10 @@ TEST_F(ExtensionAPIClientTest, RemoveWindow) {
                "Expected 'function' but got 'integer'.");
 
   ExpectJsPass("chrome.windows.remove(2, function(){})",
-               "windows.remove", "2");
+               "windows.remove", "[2]");
 
   ExpectJsPass("chrome.windows.remove(2)",
-               "windows.remove", "2");
+               "windows.remove", "[2]");
 }
 
 // Tab API tests
@@ -315,7 +314,7 @@ TEST_F(ExtensionAPIClientTest, GetTab) {
                "Expected 'function' but got 'integer'.");
 
   ExpectJsPass("chrome.tabs.get(2, function(){})",
-               "tabs.get", "2");
+               "tabs.get", "[2]");
 }
 
 TEST_F(ExtensionAPIClientTest, GetCurrentTab) {
@@ -330,7 +329,7 @@ TEST_F(ExtensionAPIClientTest, GetCurrentTab) {
                "Expected 'function' but got 'string'.");
 
   ExpectJsPass("chrome.tabs.getCurrent(function(){})",
-               "tabs.getCurrent", "null");
+               "tabs.getCurrent", "[]");
 }
 
 TEST_F(ExtensionAPIClientTest, DetectTabLanguage) {
@@ -346,7 +345,7 @@ TEST_F(ExtensionAPIClientTest, DetectTabLanguage) {
                "Expected 'function' but got 'integer'.");
 
   ExpectJsPass("chrome.tabs.detectLanguage(null, function(){})",
-               "tabs.detectLanguage", "null");
+               "tabs.detectLanguage", "[null]");
 }
 
 TEST_F(ExtensionAPIClientTest, GetSelectedTab) {
@@ -365,10 +364,10 @@ TEST_F(ExtensionAPIClientTest, GetSelectedTab) {
                "Expected 'function' but got 'integer'.");
 
   ExpectJsPass("chrome.tabs.getSelected(2, function(){})",
-               "tabs.getSelected", "2");
+               "tabs.getSelected", "[2]");
 
   ExpectJsPass("chrome.tabs.getSelected(null, function(){})",
-               "tabs.getSelected", "null");
+               "tabs.getSelected", "[null]");
 }
 
 
@@ -388,10 +387,10 @@ TEST_F(ExtensionAPIClientTest, GetAllTabsInWindow) {
                "Expected 'integer' but got 'string'.");
 
   ExpectJsPass("chrome.tabs.getAllInWindow(32, function(){})",
-               "tabs.getAllInWindow", "32");
+               "tabs.getAllInWindow", "[32]");
 
   ExpectJsPass("chrome.tabs.getAllInWindow(undefined, function(){})",
-               "tabs.getAllInWindow", "null");
+               "tabs.getAllInWindow", "[null]");
 }
 
 TEST_F(ExtensionAPIClientTest, CreateTab) {
@@ -412,10 +411,10 @@ TEST_F(ExtensionAPIClientTest, CreateTab) {
                "  windowId:4"
                "})",
                "tabs.create",
-               "{\"url\":\"http://www.google.com/\","
+               "[{\"url\":\"http://www.google.com/\","
                "\"selected\":true,"
                "\"index\":2,"
-               "\"windowId\":4}");
+               "\"windowId\":4}]");
 }
 
 TEST_F(ExtensionAPIClientTest, UpdateTab) {
@@ -472,10 +471,10 @@ TEST_F(ExtensionAPIClientTest, RemoveTab) {
                "Expected 'function' but got 'integer'.");
 
   ExpectJsPass("chrome.tabs.remove(2, function(){})",
-               "tabs.remove", "2");
+               "tabs.remove", "[2]");
 
   ExpectJsPass("chrome.tabs.remove(2)",
-               "tabs.remove", "2");
+               "tabs.remove", "[2]");
 }
 
 TEST_F(ExtensionAPIClientTest, CaptureVisibleTab) {
@@ -556,16 +555,16 @@ TEST_F(ExtensionAPIClientTest, CreateBookmark) {
   ExpectJsPass(
       "chrome.bookmarks.create({parentId:'0', title:'x'}, function(){})",
       "bookmarks.create",
-      "{\"parentId\":\"0\",\"title\":\"x\"}");
+      "[{\"parentId\":\"0\",\"title\":\"x\"}]");
 }
 
 TEST_F(ExtensionAPIClientTest, GetBookmarks) {
   ExpectJsPass("chrome.bookmarks.get('0', function(){});",
                "bookmarks.get",
-               "\"0\"");
+               "[\"0\"]");
   ExpectJsPass("chrome.bookmarks.get(['0','1','2','3'], function(){});",
                "bookmarks.get",
-               "[\"0\",\"1\",\"2\",\"3\"]");
+               "[[\"0\",\"1\",\"2\",\"3\"]]");
   ExpectJsFail("chrome.bookmarks.get(null, function(){});",
                "Uncaught Error: Parameter 0 is required.");
   // TODO(erikkay) This is succeeding, when it should fail.
@@ -580,37 +579,37 @@ TEST_F(ExtensionAPIClientTest, GetBookmarks) {
 TEST_F(ExtensionAPIClientTest, GetBookmarkChildren) {
   ExpectJsPass("chrome.bookmarks.getChildren('42', function(){});",
                "bookmarks.getChildren",
-               "\"42\"");
+               "[\"42\"]");
 }
 
 TEST_F(ExtensionAPIClientTest, GetBookmarkRecent) {
   ExpectJsPass("chrome.bookmarks.getRecent(5, function(){});",
     "bookmarks.getRecent",
-    "5");
+    "[5]");
 }
 
 TEST_F(ExtensionAPIClientTest, GetBookmarkTree) {
   ExpectJsPass("chrome.bookmarks.getTree(function(){});",
                "bookmarks.getTree",
-               "null");
+               "[]");
 }
 
 TEST_F(ExtensionAPIClientTest, SearchBookmarks) {
   ExpectJsPass("chrome.bookmarks.search('hello',function(){});",
                "bookmarks.search",
-               "\"hello\"");
+               "[\"hello\"]");
 }
 
 TEST_F(ExtensionAPIClientTest, RemoveBookmark) {
   ExpectJsPass("chrome.bookmarks.remove('42');",
                "bookmarks.remove",
-               "\"42\"");
+               "[\"42\"]");
 }
 
 TEST_F(ExtensionAPIClientTest, RemoveBookmarkTree) {
   ExpectJsPass("chrome.bookmarks.removeTree('42');",
                "bookmarks.removeTree",
-               "\"42\"");
+               "[\"42\"]");
 }
 
 TEST_F(ExtensionAPIClientTest, MoveBookmark) {
@@ -649,14 +648,14 @@ TEST_F(ExtensionAPIClientTest, EnablePageAction) {
 TEST_F(ExtensionAPIClientTest, ExpandToolstrip) {
   ExpectJsPass("chrome.toolstrip.expand({height:100, url:'http://foo/'})",
                "toolstrip.expand",
-               "{\"height\":100,\"url\":\"http://foo/\"}");
+               "[{\"height\":100,\"url\":\"http://foo/\"}]");
   ExpectJsPass("chrome.toolstrip.expand({height:100}, null)",
                "toolstrip.expand",
-               "{\"height\":100}");
+               "[{\"height\":100}]");
   ExpectJsPass("chrome.toolstrip.expand({height:100,url:'http://foo/'}, "
                    "function(){})",
                "toolstrip.expand",
-               "{\"height\":100,\"url\":\"http://foo/\"}");
+               "[{\"height\":100,\"url\":\"http://foo/\"}]");
 
 
   ExpectJsFail("chrome.toolstrip.expand()",
@@ -679,13 +678,13 @@ TEST_F(ExtensionAPIClientTest, ExpandToolstrip) {
 TEST_F(ExtensionAPIClientTest, CollapseToolstrip) {
   ExpectJsPass("chrome.toolstrip.collapse({url:'http://foo/'})",
                "toolstrip.collapse",
-               "{\"url\":\"http://foo/\"}");
+               "[{\"url\":\"http://foo/\"}]");
   ExpectJsPass("chrome.toolstrip.collapse(null)",
                "toolstrip.collapse",
-               "null");
+               "[null]");
   ExpectJsPass("chrome.toolstrip.collapse({url:'http://foo/'}, function(){})",
                "toolstrip.collapse",
-               "{\"url\":\"http://foo/\"}");
+               "[{\"url\":\"http://foo/\"}]");
 
   ExpectJsFail("chrome.toolstrip.collapse(1)",
                "Uncaught Error: Invalid value for argument 0. "
@@ -711,7 +710,7 @@ TEST_F(ExtensionAPIClientTest, GetAcceptLanguages) {
                "Expected 'function' but got 'string'.");
 
   ExpectJsPass("chrome.i18n.getAcceptLanguages(function(){})",
-               "i18n.getAcceptLanguages", "null");
+               "i18n.getAcceptLanguages", "[]");
 }
 
 // TODO(cira): re-enable when we get validation going for
diff --git a/chrome/renderer/extensions/extension_process_bindings.cc b/chrome/renderer/extensions/extension_process_bindings.cc
index b9f4e29..1ba0808 100644
--- a/chrome/renderer/extensions/extension_process_bindings.cc
+++ b/chrome/renderer/extensions/extension_process_bindings.cc
@@ -409,7 +409,7 @@ class ExtensionImpl : public ExtensionBase {
   // Common code for starting an API request to the browser. |value_args|
   // contains the request's arguments.
   static v8::Handle<v8::Value> StartRequestCommon(
-      const v8::Arguments& args, Value* value_args) {
+      const v8::Arguments& args, ListValue* value_args) {
     // Get the current RenderView so that we can send a routed IPC message from
     // the correct source.
     RenderView* renderview = bindings_utils::GetRenderViewForCurrentContext();
@@ -434,18 +434,13 @@ class ExtensionImpl : public ExtensionBase {
     int request_id = args[2]->Int32Value();
     bool has_callback = args[3]->BooleanValue();
 
-    // Put the args in a 1-element list for easier serialization. Maybe all
-    // requests should have a list of args?
-    ListValue args_holder;
-    args_holder.Append(value_args);
-
     v8::Persistent<v8::Context> current_context =
         v8::Persistent<v8::Context>::New(v8::Context::GetCurrent());
     DCHECK(!current_context.IsEmpty());
     GetPendingRequestMap()[request_id].reset(new PendingRequest(
         current_context, name));
 
-    renderview->SendExtensionRequest(name, args_holder, source_url,
+    renderview->SendExtensionRequest(name, *value_args, source_url,
                                      request_id, has_callback);
 
     return v8::Undefined();
@@ -460,12 +455,12 @@ class ExtensionImpl : public ExtensionBase {
 
     // Since we do the serialization in the v8 extension, we should always get
     // valid JSON.
-    if (!value_args) {
+    if (!value_args || !value_args->IsType(Value::TYPE_LIST)) {
       NOTREACHED() << "Invalid JSON passed to StartRequest.";
       return v8::Undefined();
     }
 
-    return StartRequestCommon(args, value_args);
+    return StartRequestCommon(args, static_cast<ListValue*>(value_args));
   }
 
   // A special request for setting the extension action icon. This function
@@ -473,7 +468,9 @@ class ExtensionImpl : public ExtensionBase {
   // before sending the request to the browser.
   static v8::Handle<v8::Value> SetExtensionActionIcon(
       const v8::Arguments& args) {
-    v8::Local<v8::Object> details = args[1]->ToObject();
+    v8::Local<v8::Object> extension_args = args[1]->ToObject();
+    v8::Local<v8::Object> details =
+        extension_args->Get(v8::String::New("0"))->ToObject();
     v8::Local<v8::Object> image_data =
         details->Get(v8::String::New("imageData"))->ToObject();
     v8::Local<v8::Object> data =
@@ -516,7 +513,10 @@ class ExtensionImpl : public ExtensionBase {
                        details->Get(v8::String::New("tabId"))->Int32Value());
     }
 
-    return StartRequestCommon(args, dict);
+    ListValue* list_value = new ListValue();
+    list_value->Append(dict);
+
+    return StartRequestCommon(args, list_value);
   }
 
   static v8::Handle<v8::Value> GetRenderViewId(const v8::Arguments& args) {