Support PNG and quality control in chrome.tabs.captureVisibleTab().

BUG=21072
TEST=ExtensionAPIClientTest.CaptureVisibleTab

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

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

2 files changed, 131 insertions, 24 deletions

diff --git a/chrome/renderer/extensions/extension_api_client_unittest.cc b/chrome/renderer/extensions/extension_api_client_unittest.cc
index 111c991..87c2505 100644
--- a/chrome/renderer/extensions/extension_api_client_unittest.cc
+++ b/chrome/renderer/extensions/extension_api_client_unittest.cc
@@ -1,9 +1,10 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
 #include "base/file_util.h"
 #include "base/json/json_reader.h"
+#include "base/json/json_writer.h"
 #include "base/path_service.h"
 #include "base/string_util.h"
 #include "chrome/common/chrome_paths.h"
@@ -13,6 +14,19 @@
 #include "chrome/test/render_view_test.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+// Make failures easier to locate by using SCOPED_TRACE() to print out the line
+// number of a failing call to ExtensionAPIClientTest::ExpectJs(Pass|Fail) .
+// Sadly, a #define is the only reasonable way to get the line number.
+#define ExpectJsFail(js, expected_failure_message) {  \
+  SCOPED_TRACE(js);                                   \
+  ExpectJsFailInternal(js, expected_failure_message); \
+}
+
+#define ExpectJsPass(js, function, arg1) {  \
+  SCOPED_TRACE(js);                         \
+  ExpectJsPassInternal(js, function, arg1); \
+}
+
 class ExtensionAPIClientTest : public RenderViewTest {
  protected:
   virtual void SetUp() {
@@ -36,15 +50,17 @@ class ExtensionAPIClientTest : public RenderViewTest {
     }
   }
 
-  void ExpectJsFail(const std::string& js, const std::string& message) {
+  // Don't call this directly.  Use the macro ExpectJsFail.
+  void ExpectJsFailInternal(const std::string& js, const std::string& message) {
     ExecuteJavaScript(js.c_str());
     EXPECT_EQ(message, GetConsoleMessage());
     render_thread_.sink().ClearMessages();
   }
 
-  void ExpectJsPass(const std::string& js,
-                    const std::string& function,
-                    const std::string& arg1) {
+  // Don't call this directly.  Use the macro ExpectJsPass.
+  void ExpectJsPassInternal(const std::string& js,
+                            const std::string& function,
+                            const std::string& arg1) {
     ExecuteJavaScript(js.c_str());
     const IPC::Message* request_msg =
       render_thread_.sink().GetUniqueMessageMatching(
@@ -61,7 +77,16 @@ class ExtensionAPIClientTest : public RenderViewTest {
 
     base::JSONReader reader;
     scoped_ptr<Value> arg1_value(reader.JsonToValue(arg1, false, false));
-    ASSERT_TRUE(args->Equals(arg1_value.get())) << js;
+    ASSERT_TRUE(arg1_value.get())
+        << "Failed to parse expected result as JSON: " << arg1;
+
+    std::string args_as_string;
+    base::JSONWriter::Write(args, false, &args_as_string);
+
+    ASSERT_TRUE(args->Equals(arg1_value.get()))
+        << js
+        << "\n Expected "<< arg1
+        << "\n Actual:  "<< args_as_string;
     render_thread_.sink().ClearMessages();
   }
 };
@@ -437,14 +462,67 @@ TEST_F(ExtensionAPIClientTest, RemoveTab) {
 
 TEST_F(ExtensionAPIClientTest, CaptureVisibleTab) {
   ExpectJsFail("chrome.tabs.captureVisibleTab(0);",
-               "Uncaught Error: Parameter 1 is required.");
+               "Uncaught Error: Parameter 2 is required.");
+
+  ExpectJsFail("chrome.tabs.captureVisibleTab(0, null);",
+               "Uncaught Error: Parameter 2 is required.");
+
+  ExpectJsFail("chrome.tabs.captureVisibleTab(null, {});",
+               "Uncaught Error: Parameter 2 is required.");
 
   ExpectJsFail("chrome.tabs.captureVisibleTab(function(){}, 0)",
                "Uncaught Error: Invalid value for argument 0. "
                "Expected 'integer' but got 'function'.");
 
-  ExpectJsPass("chrome.tabs.captureVisibleTab(null, function(img){});",
-               "tabs.captureVisibleTab", "null");
+  // Use old signiture.  Check that a null value of the options paramenter
+  // is added.
+  ExpectJsPass("chrome.tabs.captureVisibleTab(null, function(img){});", 
+               "tabs.captureVisibleTab", "[null, null]");
+
+  ExpectJsPass("chrome.tabs.captureVisibleTab(1, function(img){});",
+               "tabs.captureVisibleTab", "[1, null]");
+
+  ExpectJsPass("chrome.tabs.captureVisibleTab(null, null, function(img){});",
+               "tabs.captureVisibleTab", "[null, null]");
+
+  ExpectJsPass("chrome.tabs.captureVisibleTab("
+               "null, undefined, function(img){});",
+               "tabs.captureVisibleTab", "[null, null]");
+
+  ExpectJsPass("chrome.tabs.captureVisibleTab(null, {}, function(img){});",
+               "tabs.captureVisibleTab", "[null, {}]");
+
+  ExpectJsFail("chrome.tabs.captureVisibleTab(null, 42, function(img){});",
+               "Uncaught Error: Invalid value for argument 1. "
+               "Expected 'object' but got 'integer'.");
+
+  ExpectJsPass("chrome.tabs.captureVisibleTab("
+               "null, {\"format\": \"jpeg\"}, function(img){});",
+               "tabs.captureVisibleTab", "[null, {\"format\": \"jpeg\"}]");
+
+  ExpectJsPass("chrome.tabs.captureVisibleTab("
+               "null, {\"quality\": 100}, function(img){});",
+               "tabs.captureVisibleTab", "[null, {\"quality\": 100}]");
+
+  ExpectJsFail("chrome.tabs.captureVisibleTab("
+               "null, {\"quality\": 101}, function(img){});",
+               "Uncaught Error: Invalid value for argument 1. "
+               "Property 'quality': Value must not be greater than 100.");
+
+  ExpectJsFail("chrome.tabs.captureVisibleTab("
+               "null, {'not_a_param': 'jpeg'}, function(img){});",
+               "Uncaught Error: Invalid value for argument 1. "
+               "Property 'not_a_param': Unexpected property.");
+
+  ExpectJsFail("chrome.tabs.captureVisibleTab("
+               "null, {'format': 'invalid'}, function(img){});",
+               "Uncaught Error: Invalid value for argument 1. "
+               "Property 'format': Value must be one of: [jpeg, png].");
+
+  ExpectJsFail("chrome.tabs.captureVisibleTab("
+               "null, {'format': 42}, function(img){});",
+               "Uncaught Error: Invalid value for argument 1. "
+               "Property 'format': Value must be one of: [jpeg, png].");
 }
 
 // Bookmark API tests
diff --git a/chrome/renderer/resources/extension_process_bindings.js b/chrome/renderer/resources/extension_process_bindings.js
index 0879e91..d7b0aac 100644
--- a/chrome/renderer/resources/extension_process_bindings.js
+++ b/chrome/renderer/resources/extension_process_bindings.js
@@ -351,17 +351,27 @@ var chrome = chrome || {};
 
           var apiFunction = {};
           apiFunction.definition = functionDef;
-          apiFunction.name = apiDef.namespace + "." + functionDef.name;;
+          apiFunction.name = apiDef.namespace + "." + functionDef.name;
           apiFunctions[apiFunction.name] = apiFunction;
 
           module[functionDef.name] = bind(apiFunction, function() {
-            chromeHidden.validate(arguments, this.definition.parameters);
+            var args = arguments;
+            if (this.updateArguments) {
+              // Functions whose signature has changed can define an
+              // |updateArguments| function to transform old argument lists
+              // into the new form, preserving compatibility.
+              // TODO(skerner): Once optional args can be omitted (crbug/29215),
+              // this mechanism will become unnecessary.  Consider removing it
+              // when crbug/29215 is fixed.
+              args = this.updateArguments.apply(this, args);
+            }
+            chromeHidden.validate(args, this.definition.parameters);
 
             var retval;
             if (this.handleRequest) {
-              retval = this.handleRequest.apply(this, arguments);
+              retval = this.handleRequest.apply(this, args);
             } else {
-              retval = sendRequest(this.name, arguments,
+              retval = sendRequest(this.name, args,
                                    this.definition.parameters,
                                    this.customCallback);
             }
@@ -405,7 +415,7 @@ var chrome = chrome || {};
       var portId = OpenChannelToTab(
           tabId, chromeHidden.extensionId, name);
       return chromeHidden.Port.createPort(portId, name);
-    }
+    };
 
     apiFunctions["tabs.sendRequest"].handleRequest =
         function(tabId, request, responseCallback) {
@@ -417,7 +427,7 @@ var chrome = chrome || {};
           responseCallback(response);
         port.disconnect();
       });
-    }
+    };
 
     apiFunctions["extension.getViews"].handleRequest = function(properties) {
       var windowId = -1;
@@ -431,25 +441,25 @@ var chrome = chrome || {};
         }
       }
       return GetExtensionViews(windowId, type) || null;
-    }
+    };
 
     apiFunctions["extension.getBackgroundPage"].handleRequest = function() {
       return GetExtensionViews(-1, "BACKGROUND")[0] || null;
-    }
+    };
 
     apiFunctions["extension.getToolstrips"].handleRequest =
         function(windowId) {
       if (typeof(windowId) == "undefined")
         windowId = -1;
       return GetExtensionViews(windowId, "TOOLSTRIP");
-    }
+    };
 
     apiFunctions["extension.getExtensionTabs"].handleRequest =
         function(windowId) {
       if (typeof(windowId) == "undefined")
         windowId = -1;
       return GetExtensionViews(windowId, "TAB");
-    }
+    };
 
     apiFunctions["devtools.getTabEvents"].handleRequest = function(tabId) {
       var tabIdProxy = {};
@@ -461,7 +471,7 @@ var chrome = chrome || {};
         tabIdProxy[name] = new chrome.Event("devtools." + tabId + "." + name);
       });
       return tabIdProxy;
-    }
+    };
 
     apiFunctions["experimental.popup.show"].handleRequest =
         function(url, showDetails, callback) {
@@ -503,17 +513,17 @@ var chrome = chrome || {};
                           },
                           callback],
                          internalSchema);
-    }
+    };
 
     apiFunctions["experimental.extension.getPopupView"].handleRequest =
         function() {
       return GetPopupView();
-    }
+    };
 
     apiFunctions["experimental.popup.getParentWindow"].handleRequest =
         function() {
       return GetPopupParentWindow();
-    }
+    };
 
     var canvas;
     function setIconCommon(details, name, parameters, actionType) {
@@ -601,7 +611,26 @@ var chrome = chrome || {};
         var menuItemId = request.args[0];
         delete chromeHidden.contextMenuHandlers[menuItemId];
       }
-    }
+    };
+
+    apiFunctions["tabs.captureVisibleTab"].updateArguments = function() {
+      // Old signature:
+      //    captureVisibleTab(int windowId, function callback);
+      // New signature:
+      //    captureVisibleTab(int windowId, object details, function callback);
+      //
+      // TODO(skerner): The next step to omitting optional arguments is the
+      // replacement of this code with code that matches arguments by type.
+      // Once this is working for captureVisibleTab() it can be enabled for
+      // the rest of the API. See crbug/29215 .
+      if (arguments.length == 2 && typeof(arguments[1]) == "function") {
+        // If the old signature is used, add a null details object.
+        newArgs = [arguments[0], null, arguments[1]];
+      } else {
+        newArgs = arguments;
+      }
+      return newArgs;
+    };
 
     if (chrome.test) {
       chrome.test.getApiDefinitions = GetExtensionAPIDefinition;