Make tabCapture API always use runtime.lastError to propagate errors.

This change adds error propagation from the custom bindings using the
usual chrome.runtime.lastError mechanism.  It also corrects a minor
registry logic flaw that sometimes prevented an extension from making a
second attempt to capture a tab.

Tests were updated to eliminate "runtime.lastError" console warnings and
more strictly check that runtime.lastError is being set correctly.

BUG=463679

Review URL: https://codereview.chromium.org/1761213002

Cr-Commit-Position: refs/heads/master@{#379959}

7 files changed, 70 insertions, 25 deletions

diff --git a/chrome/browser/extensions/api/tab_capture/tab_capture_registry.cc b/chrome/browser/extensions/api/tab_capture/tab_capture_registry.cc
index 1d26544..8e1e134 100644
--- a/chrome/browser/extensions/api/tab_capture/tab_capture_registry.cc
+++ b/chrome/browser/extensions/api/tab_capture/tab_capture_registry.cc
@@ -206,8 +206,8 @@ bool TabCaptureRegistry::AddRequest(content::WebContents* target_contents,
 
   // Currently, we do not allow multiple active captures for same tab.
   if (request != NULL) {
-    if (request->capture_state() != tab_capture::TAB_CAPTURE_STATE_STOPPED &&
-        request->capture_state() != tab_capture::TAB_CAPTURE_STATE_ERROR) {
+    if (request->capture_state() == tab_capture::TAB_CAPTURE_STATE_PENDING ||
+        request->capture_state() == tab_capture::TAB_CAPTURE_STATE_ACTIVE) {
       return false;
     } else {
       // Delete the request before creating its replacement (below).
diff --git a/chrome/common/extensions/api/tab_capture.idl b/chrome/common/extensions/api/tab_capture.idl
index 7d313bc..01df3c8 100644
--- a/chrome/common/extensions/api/tab_capture.idl
+++ b/chrome/common/extensions/api/tab_capture.idl
@@ -55,8 +55,10 @@ namespace tabCapture {
     // by the extension.
     //
     // |options| : Configures the returned media stream.
-    // |callback| : Callback with either the tab capture stream or
-    //   <code>null</code>.
+    // |callback| : Callback with either the tab capture MediaStream or
+    //   <code>null</code>.  <code>null</code> indicates an error has occurred
+    //   and the client may query chrome.runtime.lastError to access the error
+    //   details.
     static void capture(CaptureOptions options,
                         GetTabMediaCallback callback);
 
@@ -90,7 +92,10 @@ namespace tabCapture {
     // and may change without notice.
     //
     // |options| : Constraints for the capture and returned MediaStream.
-    // |callback| : <code>null</code> on error.
+    // |callback| : Callback with either the tab capture MediaStream or
+    //   <code>null</code>.  <code>null</code> indicates an error has occurred
+    //   and the client may query chrome.runtime.lastError to access the error
+    //   details.
     static void captureOffscreenTab(DOMString startUrl,
                                     CaptureOptions options,
                                     GetTabMediaCallback callback);
diff --git a/chrome/renderer/resources/extensions/tab_capture_custom_bindings.js b/chrome/renderer/resources/extensions/tab_capture_custom_bindings.js
index ab7ac7c..6019c69 100644
--- a/chrome/renderer/resources/extensions/tab_capture_custom_bindings.js
+++ b/chrome/renderer/resources/extensions/tab_capture_custom_bindings.js
@@ -5,6 +5,7 @@
 // Custom binding for the Tab Capture API.
 
 var binding = require('binding').Binding.create('tabCapture');
+var lastError = require('lastError');
 
 binding.registerCustomHook(function(bindingsAPI, extensionId) {
   var apiFunctions = bindingsAPI.apiFunctions;
@@ -13,24 +14,45 @@ binding.registerCustomHook(function(bindingsAPI, extensionId) {
     if (!callback)
       return;
 
-    // TODO(miu): Propagate exceptions and always provide a useful error when
-    // callback() is invoked with a null argument.  http://crbug.com/463679
-    if (response) {
-      var options = {};
-      if (response.audioConstraints)
-        options.audio = response.audioConstraints;
-      if (response.videoConstraints)
-        options.video = response.videoConstraints;
-
-      try {
-        navigator.webkitGetUserMedia(options,
-                                     function(stream) { callback(stream); },
-                                     function(exception) { callback(null); });
-      } catch (e) {
-        callback(null);
-      }
-    } else {
+    if (!response) {
+      // When the response is missing, runtime.lastError has already been set.
+      // See chrome/browser/extensions/api/tab_capture/tab_capture_api.cc.
       callback(null);
+      return;
+    }
+
+    // Convenience function for processing webkitGetUserMedia() error objects to
+    // provide runtime.lastError messages for the tab capture API.
+    function getErrorMessage(error, fallbackMessage) {
+      if (!error || (typeof error.message != 'string'))
+        return fallbackMessage;
+      return error.message.replace(/(navigator\.)?(webkit)?GetUserMedia/gi,
+                                   name);
+    }
+
+    var options = {};
+    if (response.audioConstraints)
+      options.audio = response.audioConstraints;
+    if (response.videoConstraints)
+      options.video = response.videoConstraints;
+    try {
+      navigator.webkitGetUserMedia(
+          options,
+          function onSuccess(media_stream) {
+            callback(media_stream);
+          },
+          function onError(error) {
+            lastError.run(
+                name,
+                getErrorMessage(error, "Failed to start MediaStream."),
+                request.stack,
+                function() { callback(null); });
+          });
+    } catch (error) {
+      lastError.run(name,
+                    getErrorMessage(error, "Invalid argument(s)."),
+                    request.stack,
+                    function() { callback(null); });
     }
   }
 
diff --git a/chrome/test/data/extensions/api_test/tab_capture/active_tab_permission_test.js b/chrome/test/data/extensions/api_test/tab_capture/active_tab_permission_test.js
index 4a4ab4a..d6e6f73 100644
--- a/chrome/test/data/extensions/api_test/tab_capture/active_tab_permission_test.js
+++ b/chrome/test/data/extensions/api_test/tab_capture/active_tab_permission_test.js
@@ -2,6 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+var expectedLastErrorMessage =
+    ('Extension has not been invoked for the current page (see activeTab ' +
+     'permission). Chrome pages cannot be captured.');
+
 var afterWhitelistExtension = function(msg) {
   chrome.tabCapture.capture({audio: true, video: true}, function(stream) {
     chrome.test.assertTrue(!!stream);
@@ -13,6 +17,7 @@ var afterWhitelistExtension = function(msg) {
 
 var afterOpenNewTab = function(msg) {
   chrome.tabCapture.capture({audio: true, video: true}, function(stream) {
+    chrome.test.assertLastError(expectedLastErrorMessage);
     chrome.test.assertTrue(!stream);
     chrome.test.sendMessage('ready4', afterWhitelistExtension);
   });
@@ -29,9 +34,7 @@ var afterGrantPermission = function(msg) {
 
 var afterOpenTab = function(msg) {
   chrome.tabCapture.capture({audio: true, video: true}, function(stream) {
-    chrome.test.assertLastError(
-      'Extension has not been invoked for the current page (see activeTab ' +
-      'permission). Chrome pages cannot be captured.');
+    chrome.test.assertLastError(expectedLastErrorMessage);
     chrome.test.assertTrue(!stream);
 
     chrome.test.sendMessage('ready2', afterGrantPermission);
diff --git a/chrome/test/data/extensions/api_test/tab_capture/api_tests.js b/chrome/test/data/extensions/api_test/tab_capture/api_tests.js
index c55e8f6..88efe03 100644
--- a/chrome/test/data/extensions/api_test/tab_capture/api_tests.js
+++ b/chrome/test/data/extensions/api_test/tab_capture/api_tests.js
@@ -156,6 +156,8 @@ chrome.test.runTests([
   function noAudioOrVideoRequested() {
     // If not specified, video is not requested.
     tabCapture.capture({audio: false}, function(stream) {
+      chrome.test.assertLastError(
+          'Capture failed. No audio or video requested.');
       chrome.test.assertTrue(!stream);
       chrome.test.succeed();
     });
diff --git a/chrome/test/data/extensions/api_test/tab_capture/constraints.js b/chrome/test/data/extensions/api_test/tab_capture/constraints.js
index 757949d..5bfa3ed 100644
--- a/chrome/test/data/extensions/api_test/tab_capture/constraints.js
+++ b/chrome/test/data/extensions/api_test/tab_capture/constraints.js
@@ -2,6 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+function assertBindingsPassedWebKitErrorMessage() {
+  // Note: This lastError.message is being passed from WebKit via the tabCapture
+  // API custom bindings.  Thus, there is no check for a specific error message
+  // string.  Instead, the following checks that a non-empty string message has
+  // been set, indicating an error occurred.
+  chrome.test.assertTrue(!!chrome.runtime.lastError.message);
+}
+
 chrome.test.runTests([
   function supportsMediaConstraints() {
     chrome.tabCapture.capture({
@@ -34,6 +42,7 @@ chrome.test.runTests([
         }
       }
     }, function(stream) {
+      assertBindingsPassedWebKitErrorMessage();
       chrome.test.assertTrue(!stream);
       chrome.test.succeed();
     });
@@ -49,6 +58,7 @@ chrome.test.runTests([
         }
       }
     }, function(stream) {
+      assertBindingsPassedWebKitErrorMessage();
       chrome.test.assertTrue(!stream);
 
       chrome.tabCapture.capture({
@@ -59,6 +69,7 @@ chrome.test.runTests([
           }
         }
       }, function(stream) {
+        assertBindingsPassedWebKitErrorMessage();
         chrome.test.assertTrue(!stream);
         chrome.test.succeed();
       });
diff --git a/chrome/test/data/extensions/api_test/tab_capture/max_offscreen_tabs.js b/chrome/test/data/extensions/api_test/tab_capture/max_offscreen_tabs.js
index 89b8c4d..49c9068 100644
--- a/chrome/test/data/extensions/api_test/tab_capture/max_offscreen_tabs.js
+++ b/chrome/test/data/extensions/api_test/tab_capture/max_offscreen_tabs.js
@@ -22,6 +22,8 @@ chrome.test.runTests([
          function(stream) {
            if (streamsSoFar.length == 3) {
              // 4th off-screen tab capture should fail.
+             chrome.test.assertLastError(
+                 'Extension has already started too many off-screen tabs.');
              chrome.test.assertFalse(!!stream);
              stopAllStreams(streamsSoFar);
              chrome.test.succeed();