TabCapture: Handle audio devices errors and correctly close stream in that case.

Add more checks to make sure the tabCapture streams can only be accessed via extension API.
Fix the GetUserMedia test and give it valid id's and fix various nits.


Review URL: https://chromiumcodereview.appspot.com/12326033

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

4 files changed, 76 insertions, 41 deletions

diff --git a/chrome/browser/extensions/api/tab_capture/tab_capture_api.cc b/chrome/browser/extensions/api/tab_capture/tab_capture_api.cc
index 78d15eb..380d762 100644
--- a/chrome/browser/extensions/api/tab_capture/tab_capture_api.cc
+++ b/chrome/browser/extensions/api/tab_capture/tab_capture_api.cc
@@ -148,8 +148,8 @@ bool TabCaptureGetCapturedTabsFunction::RunImpl() {
   for (TabCaptureRegistry::CaptureRequestList::const_iterator it =
        captured_tabs.begin(); it != captured_tabs.end(); ++it) {
     scoped_ptr<tab_capture::CaptureInfo> info(new tab_capture::CaptureInfo());
-    info->tab_id = it->tab_id;
-    info->status = it->status;
+    info->tab_id = (*it)->tab_id;
+    info->status = (*it)->status;
     list->Append(info->ToValue().release());
   }
 
diff --git a/chrome/browser/extensions/api/tab_capture/tab_capture_apitest.cc b/chrome/browser/extensions/api/tab_capture/tab_capture_apitest.cc
index 7af82d7..6c0102b 100644
--- a/chrome/browser/extensions/api/tab_capture/tab_capture_apitest.cc
+++ b/chrome/browser/extensions/api/tab_capture/tab_capture_apitest.cc
@@ -2,10 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/stringprintf.h"
 #include "chrome/browser/extensions/extension_apitest.h"
+#include "chrome/browser/extensions/extension_test_message_listener.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/extensions/feature_switch.h"
 #include "chrome/common/extensions/features/feature.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/render_view_host.h"
 #include "content/public/common/content_switches.h"
 
 namespace chrome {
@@ -43,16 +47,29 @@ IN_PROC_BROWSER_TEST_F(TabCaptureApiTest, MAYBE_EndToEnd) {
                                   "end_to_end.html")) << message_;
 }
 
-IN_PROC_BROWSER_TEST_F(TabCaptureApiTest, TabCapturePermissionsTestFlagOn) {
-  extensions::FeatureSwitch::ScopedOverride tab_capture(
-      extensions::FeatureSwitch::tab_capture(), true);
-  ASSERT_TRUE(RunExtensionTest("tab_capture/permissions")) << message_;
-}
+// Test that we can't get tabCapture streams using GetUserMedia directly.
+IN_PROC_BROWSER_TEST_F(TabCaptureApiTest, GetUserMediaTest) {
+  ExtensionTestMessageListener listener("ready", true);
 
-IN_PROC_BROWSER_TEST_F(TabCaptureApiTest, TabCapturePermissionsTestFlagOff) {
-  extensions::FeatureSwitch::ScopedOverride tab_capture(
-      extensions::FeatureSwitch::tab_capture(), false);
-  ASSERT_TRUE(RunExtensionTest("tab_capture/permissions")) << message_;
+  ASSERT_TRUE(RunExtensionSubtest("tab_capture/experimental",
+                                  "get_user_media_test.html")) << message_;
+
+  EXPECT_TRUE(listener.WaitUntilSatisfied());
+
+  content::OpenURLParams params(GURL("about:blank"), content::Referrer(),
+                                NEW_FOREGROUND_TAB,
+                                content::PAGE_TRANSITION_LINK, false);
+  content::WebContents* web_contents = browser()->OpenURL(params);
+
+  content::RenderViewHost* const rvh = web_contents->GetRenderViewHost();
+  int render_process_id = rvh->GetProcess()->GetID();
+  int routing_id = rvh->GetRoutingID();
+
+  listener.Reply(StringPrintf("%i:%i", render_process_id, routing_id));
+
+  ResultCatcher catcher;
+  catcher.RestrictToProfile(browser()->profile());
+  EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
 }
 
 }  // namespace chrome
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 fe9c69a..16e52fe2 100644
--- a/chrome/browser/extensions/api/tab_capture/tab_capture_registry.cc
+++ b/chrome/browser/extensions/api/tab_capture/tab_capture_registry.cc
@@ -23,8 +23,10 @@ using content::BrowserThread;
 namespace extensions {
 
 TabCaptureRegistry::TabCaptureRequest::TabCaptureRequest(
-    std::string extension_id, int tab_id, tab_capture::TabCaptureState status)
-    : extension_id(extension_id), tab_id(tab_id), status(status) {
+    const std::string& extension_id, const int tab_id,
+    tab_capture::TabCaptureState status)
+    : extension_id(extension_id), tab_id(tab_id), status(status),
+      last_status(status) {
 }
 
 TabCaptureRegistry::TabCaptureRequest::~TabCaptureRequest() {
@@ -42,12 +44,13 @@ TabCaptureRegistry::~TabCaptureRegistry() {
 }
 
 const TabCaptureRegistry::CaptureRequestList
-    TabCaptureRegistry::GetCapturedTabs(const std::string& extension_id) {
+    TabCaptureRegistry::GetCapturedTabs(const std::string& extension_id) const {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   CaptureRequestList list;
-  for (DeviceCaptureRequestMap::iterator it = requests_.begin();
+  for (DeviceCaptureRequestMap::const_iterator it = requests_.begin();
        it != requests_.end(); ++it) {
     if (it->second.extension_id == extension_id) {
-      list.push_back(it->second);
+      list.push_back(make_linked_ptr(new TabCaptureRequest(it->second)));
     }
   }
   return list;
@@ -92,6 +95,7 @@ bool TabCaptureRegistry::AddRequest(const std::pair<int, int> key,
 
 bool TabCaptureRegistry::VerifyRequest(int render_process_id,
                                        int render_view_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DVLOG(1) << "Verifying tabCapture request for "
            << render_process_id << ":" << render_view_id;
   return requests_.find(std::make_pair(
@@ -104,12 +108,8 @@ void TabCaptureRegistry::OnRequestUpdate(
     const content::MediaStreamDevice& device,
     const content::MediaRequestState new_state) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  // TODO(justinlin): We drop audio device events since they will occur in
-  // parallel with the video device events (we would get duplicate events). When
-  // audio mirroring is implemented, we will want to grab those events when
-  // video is not requested.
-  if (device.type != content::MEDIA_TAB_VIDEO_CAPTURE)
+  if (device.type != content::MEDIA_TAB_VIDEO_CAPTURE &&
+      device.type != content::MEDIA_TAB_AUDIO_CAPTURE)
     return;
 
   EventRouter* router = profile_ ?
@@ -118,26 +118,25 @@ void TabCaptureRegistry::OnRequestUpdate(
     return;
 
   std::pair<int, int> key = std::make_pair(render_process_id, render_view_id);
-
   DeviceCaptureRequestMap::iterator request_it = requests_.find(key);
   if (request_it == requests_.end()) {
     LOG(ERROR) << "Receiving updates for invalid tab capture request.";
     return;
   }
 
-  tab_capture::TabCaptureState state = tab_capture::TAB_CAPTURE_STATE_NONE;
+  tab_capture::TabCaptureState next_state = tab_capture::TAB_CAPTURE_STATE_NONE;
   switch (new_state) {
     case content::MEDIA_REQUEST_STATE_PENDING_APPROVAL:
-      state = tab_capture::TAB_CAPTURE_STATE_PENDING;
+      next_state = tab_capture::TAB_CAPTURE_STATE_PENDING;
       break;
     case content::MEDIA_REQUEST_STATE_DONE:
-      state = tab_capture::TAB_CAPTURE_STATE_ACTIVE;
+      next_state = tab_capture::TAB_CAPTURE_STATE_ACTIVE;
       break;
     case content::MEDIA_REQUEST_STATE_CLOSING:
-      state = tab_capture::TAB_CAPTURE_STATE_STOPPED;
+      next_state = tab_capture::TAB_CAPTURE_STATE_STOPPED;
       break;
     case content::MEDIA_REQUEST_STATE_ERROR:
-      state = tab_capture::TAB_CAPTURE_STATE_ERROR;
+      next_state = tab_capture::TAB_CAPTURE_STATE_ERROR;
       break;
     case content::MEDIA_REQUEST_STATE_OPENING:
       return;
@@ -148,18 +147,33 @@ void TabCaptureRegistry::OnRequestUpdate(
   }
 
   TabCaptureRegistry::TabCaptureRequest* request_info = &request_it->second;
-  request_info->status = state;
+  if (next_state == tab_capture::TAB_CAPTURE_STATE_PENDING &&
+      request_info->status != tab_capture::TAB_CAPTURE_STATE_NONE &&
+      request_info->status != tab_capture::TAB_CAPTURE_STATE_STOPPED &&
+      request_info->status != tab_capture::TAB_CAPTURE_STATE_ERROR) {
+    // If we end up trying to grab a new stream while the previous one was never
+    // terminated, then something fishy is going on.
+    LOG(ERROR) << "Trying to capture tab with existing stream.";
+    return;
+  }
+
+  request_info->last_status = request_info->status;
+  request_info->status = next_state;
 
   scoped_ptr<tab_capture::CaptureInfo> info(new tab_capture::CaptureInfo());
   info->tab_id = request_info->tab_id;
   info->status = request_info->status;
 
-  scoped_ptr<base::ListValue> args(new ListValue());
-  args->Append(info->ToValue().release());
-  scoped_ptr<Event> event(new Event(
-      events::kOnTabCaptureStatusChanged, args.Pass()));
-  event->restrict_to_profile = profile_;
-  router->DispatchEventToExtension(request_info->extension_id, event.Pass());
+  // We will get duplicate events if we requested both audio and video, so only
+  // send new events.
+  if (request_info->last_status != request_info->status) {
+    scoped_ptr<base::ListValue> args(new ListValue());
+    args->Append(info->ToValue().release());
+    scoped_ptr<Event> event(new Event(
+        events::kOnTabCaptureStatusChanged, args.Pass()));
+    event->restrict_to_profile = profile_;
+    router->DispatchEventToExtension(request_info->extension_id, event.Pass());
+  }
 }
 
 }  // namespace extensions
diff --git a/chrome/browser/extensions/api/tab_capture/tab_capture_registry.h b/chrome/browser/extensions/api/tab_capture/tab_capture_registry.h
index 561247e..694b994 100644
--- a/chrome/browser/extensions/api/tab_capture/tab_capture_registry.h
+++ b/chrome/browser/extensions/api/tab_capture/tab_capture_registry.h
@@ -10,6 +10,7 @@
 #include <utility>
 #include <vector>
 
+#include "base/memory/linked_ptr.h"
 #include "chrome/browser/media/media_capture_devices_dispatcher.h"
 #include "chrome/browser/profiles/profile_keyed_service.h"
 #include "chrome/common/extensions/api/tab_capture.h"
@@ -29,22 +30,25 @@ class TabCaptureRegistry : public ProfileKeyedService,
                            public MediaCaptureDevicesDispatcher::Observer {
  public:
   struct TabCaptureRequest {
-    TabCaptureRequest(std::string extension_id,
-                      int tab_id,
+    TabCaptureRequest(const std::string& extension_id,
+                      const int tab_id,
                       tab_capture::TabCaptureState status);
     ~TabCaptureRequest();
 
-    std::string extension_id;
-    int tab_id;
+    const std::string extension_id;
+    const int tab_id;
     tab_capture::TabCaptureState status;
+    tab_capture::TabCaptureState last_status;
   };
-  typedef std::vector<TabCaptureRequest> CaptureRequestList;
+  typedef std::vector<linked_ptr<TabCaptureRequest> > CaptureRequestList;
 
   explicit TabCaptureRegistry(Profile* profile);
 
-  const CaptureRequestList GetCapturedTabs(const std::string& extension_id);
+  const CaptureRequestList GetCapturedTabs(
+      const std::string& extension_id) const;
   bool AddRequest(const std::pair<int, int>, const TabCaptureRequest& request);
   bool VerifyRequest(int render_process_id, int render_view_id);
+  void RemoveRequest(int render_process_id, int render_view_id);
 
  private:
   // Maps device_id to information about the media stream request. This is