Reland "Immediately stop media streams if users revokes consent"

This is a reland of 4e31d46a

Original change's description:
> Immediately stop media streams if users revokes consent
>
> Since camera and microphone are particularly sensitive permissions we
> should be viligent and stop streams immediately when the user revokes
> the permission.
>
> This CL uses the permission controller subscription functionality to
> listen to permission changes on the relevant origins. Each DeviceRequest
> will have up to 2 permission subscriptions (1 audio, 1 video). When
> a permission is changed to non-Granted the specific DeviceRequest is
> canceled. Subscriptions are removed when the DeviceRequest is deleted.
>
> Because MediaStreamManager is an IO thread class but
> PermissionController is a UI thread class, this CL has to do a bit of
> thread hoping:
>
> FinalizeStream(IO) > SubscribeToPermissionControllerOnUIThread(UI) >
> SetPermissionSubscriptionIDs(IO) >(maybe)>
> UnsubscribeFromPermissionControllerOnUIThread(UI)
>
> DeleteRequest(IO) > UnsubscribeFromPermissionControllerOnUIThread(UI)
>
> Bug: 1116435
>
> Change-Id: I6649125e028d607eeb7ba5701b710a715addf6f2
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2352790
> Commit-Queue: Andy Paicu <andypaicu@chromium.org>
> Reviewed-by: Guido Urdaneta <guidou@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#798564}

Bug: 1116435
Change-Id: Iede170df981bc69d025bcf66b029237977bc7a04
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2362288
Commit-Queue: Andy Paicu <andypaicu@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799549}

chrome/browser/media/webrtc/media_stream_infobar_browsertest.cc → chrome/browser/media/webrtc/media_stream_permission_browsertest.cc

@@ -5,6 +5,7 @@
 #include "base/command_line.h"
 #include "base/files/file_util.h"
 #include "base/macros.h"
+#include "base/run_loop.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/media/webrtc/webrtc_browsertest_base.h"
@@ -65,7 +66,9 @@ class MediaStreamPermissionTest : public WebRtcTestBase {
 
  private:
   content::WebContents* LoadTestPageInBrowser(Browser* browser) {
-    EXPECT_TRUE(embedded_test_server()->Start());
+    if (!embedded_test_server()->Started()) {
+      EXPECT_TRUE(embedded_test_server()->Start());
+    }
 
     // Uses the default server.
     GURL url = test_page_url();
@@ -76,12 +79,6 @@ class MediaStreamPermissionTest : public WebRtcTestBase {
     return browser->tab_strip_model()->GetActiveWebContents();
   }
 
-  // Dummy callback for when we deny the current request directly.
-  static void OnMediaStreamResponse(
-      const blink::MediaStreamDevices& devices,
-      blink::mojom::MediaStreamRequestResult result,
-      std::unique_ptr<content::MediaStreamUI> ui) {}
-
   DISALLOW_COPY_AND_ASSIGN(MediaStreamPermissionTest);
 };
 
@@ -169,3 +166,50 @@ IN_PROC_BROWSER_TEST_F(MediaStreamPermissionTest,
   EXPECT_TRUE(GetUserMediaWithSpecificConstraintsAndAccept(
       tab_contents, kAudioOnlyCallConstraints));
 }
+
+IN_PROC_BROWSER_TEST_F(MediaStreamPermissionTest,
+                       DenyingPermissionStopsStreamWhenRelevant) {
+  struct {
+    std::string constraints;
+    ContentSettingsType setting_to_clear;
+    bool should_video_stop;
+  } kTests[] = {
+      {kAudioVideoCallConstraints, ContentSettingsType::MEDIASTREAM_CAMERA,
+       true},
+      {kAudioVideoCallConstraints, ContentSettingsType::MEDIASTREAM_MIC, true},
+      {kVideoOnlyCallConstraints, ContentSettingsType::MEDIASTREAM_CAMERA,
+       true},
+      {kVideoOnlyCallConstraints, ContentSettingsType::MEDIASTREAM_MIC, false},
+  };
+
+  HostContentSettingsMap* settings_map =
+      HostContentSettingsMapFactory::GetForProfile(browser()->profile());
+
+  for (const auto& kTest : kTests) {
+    content::WebContents* tab_contents = LoadTestPageInTab();
+
+    EXPECT_TRUE(GetUserMediaWithSpecificConstraintsAndAcceptIfPrompted(
+        tab_contents, kTest.constraints));
+
+    StartDetectingVideo(tab_contents, "local-view");
+    EXPECT_TRUE(WaitForVideoToPlay(tab_contents));
+
+    settings_map->ClearSettingsForOneType(kTest.setting_to_clear);
+
+    // Let all the cross-thread tasks do their work.
+    base::RunLoop().RunUntilIdle();
+
+    StartDetectingVideo(tab_contents, "local-view");
+
+    if (kTest.should_video_stop) {
+      EXPECT_TRUE(WaitForVideoToStop(tab_contents));
+    } else {
+      EXPECT_TRUE(WaitForVideoToPlay(tab_contents));
+    }
+
+    // Clean up settings for the following tests.
+    settings_map->ClearSettingsForOneType(ContentSettingsType::MEDIASTREAM_MIC);
+    settings_map->ClearSettingsForOneType(
+        ContentSettingsType::MEDIASTREAM_CAMERA);
+  }
+}

chrome/browser/media/webrtc/webrtc_browsertest_base.cc

@@ -473,6 +473,14 @@ bool WebRtcTestBase::WaitForVideoToPlay(
   return is_video_playing;
 }
 
+bool WebRtcTestBase::WaitForVideoToStop(
+    content::WebContents* tab_contents) const {
+  bool is_video_stopped =
+      test::PollingWaitUntil("isVideoStopped()", "video-stopped", tab_contents);
+  EXPECT_TRUE(is_video_stopped);
+  return is_video_stopped;
+}
+
 std::string WebRtcTestBase::GetStreamSize(
     content::WebContents* tab_contents,
     const std::string& video_element) const {

chrome/browser/media/webrtc/webrtc_browsertest_base.h

@@ -182,7 +182,11 @@ class WebRtcTestBase : public InProcessBrowserTest {
   // make that work). Looks at a 320x240 area of the target video tag.
   void StartDetectingVideo(content::WebContents* tab_contents,
                            const std::string& video_element) const;
+
+  // Wait for a video to start/stop playing. StartDetectingVideo must have
+  // been called already.
   bool WaitForVideoToPlay(content::WebContents* tab_contents) const;
+  bool WaitForVideoToStop(content::WebContents* tab_contents) const;
 
   // Returns the stream size as a string on the format <width>x<height>.
   std::string GetStreamSize(content::WebContents* tab_contents,

chrome/test/BUILD.gn

@@ -970,7 +970,7 @@ if (!is_android) {
       "../browser/media/test_license_server_config.h",
       "../browser/media/unified_autoplay_browsertest.cc",
       "../browser/media/webrtc/media_stream_devices_controller_browsertest.cc",
-      "../browser/media/webrtc/media_stream_infobar_browsertest.cc",
+      "../browser/media/webrtc/media_stream_permission_browsertest.cc",
       "../browser/media/webrtc/test_stats_dictionary.cc",
       "../browser/media/webrtc/test_stats_dictionary.h",
       "../browser/media/webrtc/test_stats_dictionary_unittest.cc",

chrome/test/data/webrtc/video_detector.js

@@ -68,14 +68,33 @@ function isVideoPlaying() {
       if (!allElementsRoughlyEqualTo_(gFingerprints, gFingerprints[0])) {
         clearInterval(gDetectorInterval);
         gDetectorInterval = null;
-        returnToTest('video-playing');
+        silentReturnToTest('video-playing');
         return;
       }
     }
   } catch (exception) {
     throw failTest('Failed to detect video: ' + exception.message);
   }
-  returnToTest('video-not-playing');
+  silentReturnToTest('video-not-playing');
+}
+
+/**
+ * Checks if the video has stopped
+ *
+ * @return {string} video-stopped or video-not-stopped.
+ */
+function isVideoStopped() {
+  // Video is considered to be stopped if the last 5 fingerprints are the same.
+  // We only check for rough equality though to account for rounding errors.
+  if (gFingerprints.length < 5)
+    silentReturnToTest('video-not-stopped');
+
+  if (allElementsRoughlyEqualTo_(gFingerprints.slice(-5),
+                                 gFingerprints[gFingerprints.length - 1])) {
+    silentReturnToTest('video-stopped');
+  }
+
+  silentReturnToTest('video-not-stopped');
 }
 
 /**

content/browser/renderer_host/media/media_stream_manager.cc

@@ -33,6 +33,7 @@
 #include "content/browser/gpu/gpu_process_host.h"
 #include "content/browser/media/capture/desktop_capture_device_uma_types.h"
 #include "content/browser/media/media_devices_permission_checker.h"
+#include "content/browser/permissions/permission_controller_impl.h"
 #include "content/browser/renderer_host/media/audio_input_device_manager.h"
 #include "content/browser/renderer_host/media/audio_service_listener.h"
 #include "content/browser/renderer_host/media/in_process_video_capture_provider.h"
@@ -667,6 +668,10 @@ class MediaStreamManager::DeviceRequest {
 
   std::string tab_capture_device_id;
 
+  int audio_subscription_id = PermissionControllerImpl::kNoPendingOperation;
+
+  int video_subscription_id = PermissionControllerImpl::kNoPendingOperation;
+
  private:
   std::vector<MediaRequestState> state_;
   std::unique_ptr<MediaStreamRequest> ui_request_;
@@ -1319,7 +1324,18 @@ void MediaStreamManager::DeleteRequest(const std::string& label) {
   for (auto request_it = requests_.begin(); request_it != requests_.end();
        ++request_it) {
     if (request_it->first == label) {
+      // Clean up permission controller subscription.
+      GetUIThreadTaskRunner({})->PostTask(
+          FROM_HERE,
+          base::BindOnce(&MediaStreamManager::
+                             UnsubscribeFromPermissionControllerOnUIThread,
+                         request_it->second->requesting_process_id,
+                         request_it->second->requesting_frame_id,
+                         request_it->second->audio_subscription_id,
+                         request_it->second->video_subscription_id));
+
       requests_.erase(request_it);
+
       return;
     }
   }
@@ -1762,6 +1778,19 @@ void MediaStreamManager::FinalizeGenerateStream(const std::string& label,
       NOTREACHED();
   }
 
+  // Subscribe to follow permission changes in order to close streams when the
+  // user denies mic/camera.
+  // It is safe to bind base::Unretained(this) because MediaStreamManager is
+  // owned by BrowserMainLoop.
+  GetUIThreadTaskRunner({})->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          &MediaStreamManager::SubscribeToPermissionControllerOnUIThread,
+          base::Unretained(this), label, request->requesting_process_id,
+          request->requesting_frame_id, request->requester_id,
+          request->page_request_id, audio_devices.size() > 0,
+          video_devices.size() > 0, request->salt_and_origin.origin.GetURL()));
+
   // It is safe to bind base::Unretained(this) because MediaStreamManager is
   // owned by BrowserMainLoop and so outlives the IO thread.
   GetUIThreadTaskRunner({})->PostTaskAndReplyWithResult(
@@ -2614,4 +2643,144 @@ void MediaStreamManager::OnStreamStarted(const std::string& label) {
   }
 }
 
+// static
+PermissionControllerImpl* MediaStreamManager::GetPermissionController(
+    int requesting_process_id,
+    int requesting_frame_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  RenderFrameHost* rfh =
+      RenderFrameHost::FromID(requesting_process_id, requesting_frame_id);
+  if (!rfh)
+    return nullptr;
+
+  return PermissionControllerImpl::FromBrowserContext(rfh->GetBrowserContext());
+}
+
+void MediaStreamManager::SubscribeToPermissionControllerOnUIThread(
+    const std::string& label,
+    int requesting_process_id,
+    int requesting_frame_id,
+    int requester_id,
+    int page_request_id,
+    bool is_audio_request,
+    bool is_video_request,
+    const GURL& origin) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  PermissionControllerImpl* controller =
+      GetPermissionController(requesting_process_id, requesting_frame_id);
+  if (!controller)
+    return;
+
+  int audio_subscription_id = PermissionControllerImpl::kNoPendingOperation;
+  int video_subscription_id = PermissionControllerImpl::kNoPendingOperation;
+
+  if (is_audio_request) {
+    // It is safe to bind base::Unretained(this) because MediaStreamManager is
+    // owned by BrowserMainLoop.
+    audio_subscription_id = controller->SubscribePermissionStatusChange(
+        PermissionType::AUDIO_CAPTURE,
+        RenderFrameHost::FromID(requesting_process_id, requesting_frame_id),
+        origin,
+        base::BindRepeating(&MediaStreamManager::PermissionChangedCallback,
+                            base::Unretained(this), requesting_process_id,
+                            requesting_frame_id, requester_id,
+                            page_request_id));
+  }
+
+  if (is_video_request) {
+    // It is safe to bind base::Unretained(this) because MediaStreamManager is
+    // owned by BrowserMainLoop.
+    video_subscription_id = controller->SubscribePermissionStatusChange(
+        PermissionType::VIDEO_CAPTURE,
+        RenderFrameHost::FromID(requesting_process_id, requesting_frame_id),
+        origin,
+        base::BindRepeating(&MediaStreamManager::PermissionChangedCallback,
+                            base::Unretained(this), requesting_process_id,
+                            requesting_frame_id, requester_id,
+                            page_request_id));
+  }
+
+  // It is safe to bind base::Unretained(this) because MediaStreamManager is
+  // owned by BrowserMainLoop.
+  GetIOThreadTaskRunner({})->PostTask(
+      FROM_HERE,
+      base::BindOnce(&MediaStreamManager::SetPermissionSubscriptionIDs,
+                     base::Unretained(this), label, requesting_process_id,
+                     requesting_frame_id, audio_subscription_id,
+                     video_subscription_id));
+}
+
+void MediaStreamManager::SetPermissionSubscriptionIDs(
+    const std::string& label,
+    int requesting_process_id,
+    int requesting_frame_id,
+    int audio_subscription_id,
+    int video_subscription_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  DeviceRequest* const request = FindRequest(label);
+  if (!request) {
+    // Something happened with the request while the permission subscription was
+    // created, unsubscribe to clean up.
+    // It is safe to bind base::Unretained(this) because MediaStreamManager is
+    // owned by BrowserMainLoop.
+    GetUIThreadTaskRunner({})->PostTask(
+        FROM_HERE,
+        base::BindOnce(
+            &MediaStreamManager::UnsubscribeFromPermissionControllerOnUIThread,
+            requesting_process_id, requesting_frame_id, audio_subscription_id,
+            video_subscription_id));
+
+    return;
+  }
+
+  request->audio_subscription_id = audio_subscription_id;
+  request->video_subscription_id = video_subscription_id;
+}
+
+// static
+void MediaStreamManager::UnsubscribeFromPermissionControllerOnUIThread(
+    int requesting_process_id,
+    int requesting_frame_id,
+    int audio_subscription_id,
+    int video_subscription_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  PermissionControllerImpl* controller =
+      GetPermissionController(requesting_process_id, requesting_frame_id);
+  if (!controller)
+    return;
+
+  controller->UnsubscribePermissionStatusChange(audio_subscription_id);
+  controller->UnsubscribePermissionStatusChange(video_subscription_id);
+}
+
+void MediaStreamManager::PermissionChangedCallback(
+    int requesting_process_id,
+    int requesting_frame_id,
+    int requester_id,
+    int page_request_id,
+    blink::mojom::PermissionStatus status) {
+  if (status == blink::mojom::PermissionStatus::GRANTED)
+    return;
+
+  if (!BrowserThread::CurrentlyOn(BrowserThread::IO)) {
+    // It is safe to bind base::Unretained(this) because MediaStreamManager is
+    // owned by BrowserMainLoop.
+    GetIOThreadTaskRunner({})->PostTask(
+        FROM_HERE,
+        base::BindOnce(&MediaStreamManager::PermissionChangedCallback,
+                       base::Unretained(this), requesting_process_id,
+                       requesting_frame_id, requester_id, page_request_id,
+                       status));
+
+    return;
+  }
+
+  CancelRequest(requesting_process_id, requesting_frame_id, requester_id,
+                page_request_id);
+}
+
 }  // namespace content

content/browser/renderer_host/media/media_stream_manager.h

@@ -55,6 +55,7 @@
 #include "third_party/blink/public/common/mediastream/media_stream_controls.h"
 #include "third_party/blink/public/common/mediastream/media_stream_request.h"
 #include "third_party/blink/public/mojom/mediastream/media_stream.mojom.h"
+#include "third_party/blink/public/mojom/permissions/permission_status.mojom.h"
 
 namespace media {
 class AudioSystem;
@@ -72,6 +73,7 @@ class FakeMediaStreamUIProxy;
 class MediaStreamUIProxy;
 class VideoCaptureManager;
 class VideoCaptureProvider;
+class PermissionControllerImpl;
 
 // MediaStreamManager is used to generate and close new media devices, not to
 // start the media flow. The classes requesting new media streams are answered
@@ -534,6 +536,48 @@ class CONTENT_EXPORT MediaStreamManager
   // Activate the specified tab and bring it to the front.
   void ActivateTabOnUIThread(const DesktopMediaID source);
 
+  // Get the permission controller for a particular RFH. Must be called on the
+  // UI thread.
+  static PermissionControllerImpl* GetPermissionController(
+      int requesting_process_id,
+      int requesting_frame_id);
+
+  // Subscribe to the permission controller in order to monitor camera/mic
+  // permission updates for a particular DeviceRequest. All the additional
+  // information is needed because `FindRequest` can't be called on the UI
+  // thread.
+  void SubscribeToPermissionControllerOnUIThread(const std::string& label,
+                                                 int requesting_process_id,
+                                                 int requesting_frame_id,
+                                                 int requester_id,
+                                                 int page_request_id,
+                                                 bool is_audio_request,
+                                                 bool is_video_request,
+                                                 const GURL& origin);
+
+  // Store the subscription ids on a DeviceRequest in order to allow
+  // unsubscribing when the request is deleted.
+  void SetPermissionSubscriptionIDs(const std::string& label,
+                                    int requesting_process_id,
+                                    int requesting_frame_id,
+                                    int audio_subscription_id,
+                                    int video_subscription_id);
+
+  // Unsubscribe from following permission updates for the two specified
+  // subscription IDs. Called when a request is deleted.
+  static void UnsubscribeFromPermissionControllerOnUIThread(
+      int requesting_process_id,
+      int requesting_frame_id,
+      int audio_subscription_id,
+      int video_subscription_id);
+
+  // Callback that the PermissionController calls when a permission is updated.
+  void PermissionChangedCallback(int requesting_process_id,
+                                 int requesting_frame_id,
+                                 int requester_id,
+                                 int page_request_id,
+                                 blink::mojom::PermissionStatus status);
+
   media::AudioSystem* const audio_system_;  // not owned
   scoped_refptr<AudioInputDeviceManager> audio_input_device_manager_;
   scoped_refptr<VideoCaptureManager> video_capture_manager_;