Revert "Immediately stop media streams if users revokes consent"

This reverts commit 4e31d46a.

Reason for revert: MediaStreamPermissionTest.DenyingPermissionStopsStreamWhenRelevant flakes on "Win7 Tests (1)":
https://ci.chromium.org/p/chromium/builders/ci/Win7%20Tests%20%281%29/106475

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}

TBR=guidou@chromium.org,andypaicu@chromium.org

Change-Id: I59eabef07aa990fd22bebba445c36c273c1a8342
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1116435
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2360119Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798753}

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

@@ -5,7 +5,6 @@
 #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"
@@ -66,9 +65,7 @@ class MediaStreamPermissionTest : public WebRtcTestBase {
 
  private:
   content::WebContents* LoadTestPageInBrowser(Browser* browser) {
-    if (!embedded_test_server()->Started()) {
-      EXPECT_TRUE(embedded_test_server()->Start());
-    }
+    EXPECT_TRUE(embedded_test_server()->Start());
 
     // Uses the default server.
     GURL url = test_page_url();
@@ -79,6 +76,12 @@ 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);
 };
 
@@ -166,50 +169,3 @@ 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,14 +473,6 @@ 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,11 +182,7 @@ 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_permission_browsertest.cc",
+      "../browser/media/webrtc/media_stream_infobar_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

@@ -78,25 +78,6 @@ function isVideoPlaying() {
   returnToTest('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)
-    returnToTest('video-not-stopped');
-
-  if (allElementsRoughlyEqualTo_(gFingerprints.slice(-5),
-                                 gFingerprints[gFingerprints.length - 1])) {
-    returnToTest('video-stopped');
-  }
-
-  returnToTest('video-not-stopped');
-}
-
 /**
  * Queries for the stream size (not necessarily the size at which the video tag
  * is rendered).

content/browser/renderer_host/media/media_stream_manager.cc

@@ -33,7 +33,6 @@
 #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"
@@ -668,10 +667,6 @@ 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_;
@@ -1324,18 +1319,7 @@ 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;
     }
   }
@@ -1778,19 +1762,6 @@ 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(
@@ -2643,144 +2614,4 @@ 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,7 +55,6 @@
 #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;
@@ -73,7 +72,6 @@ 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
@@ -536,48 +534,6 @@ 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_;