Bug 1744346 consolidate boolean device enumeration parameters into a flags parameter r=jib

The key motivator here was the misleading aForceNoPermRequest parameter. This came from an unreviewed change in response to a review request. https://phabricator.services.mozilla.com/D14575?vs=44463&id=47352#inline-82282 The newer variable name described the opposite of its use, but its usage was not negated. The logical change to the AllowPermissionRequest flag no longer passed from MediaDevices::GetSinkDevice() has little effect because any permission required would be obtained from getUserMedia() or selectAudioOutput() prior to setSinkId(). A permission request is not expected during setSinkId(). The EnumerateAudioOutputs flag removes some use of the confusing MediaSinkEnum::Other and makes the long list of parameters slightly easier to read. Differential Revision: https://phabricator.services.mozilla.com/D132875
2021-12-17 05:54:27 +00:00 · 2021-12-17 05:54:27 +00:00 · 5b0374554e
--- a/dom/media/MediaDevices.cpp
+++ b/dom/media/MediaDevices.cpp
@ -5,6 +5,7 @@
 #include "mozilla/dom/MediaDevices.h"

 #include "AudioDeviceInfo.h"
+#include "MediaEngine.h"
 #include "mozilla/dom/BrowsingContext.h"
 #include "mozilla/dom/Document.h"
 #include "mozilla/dom/MediaStreamBinding.h"
@ -25,6 +26,7 @@

 namespace mozilla::dom {

+using EnumerationFlag = MediaManager::EnumerationFlag;
 using DeviceEnumerationType = MediaManager::DeviceEnumerationType;

 MediaDevices::MediaDevices(nsPIDOMWindowInner* aWindow)
@ -456,9 +458,9 @@ RefPtr<MediaDevices::SinkInfoPromise> MediaDevices::GetSinkDevice(
  auto devices = MakeRefPtr<MediaManager::MediaDeviceSetRefCnt>();
  return MediaManager::Get()
      ->EnumerateDevicesImpl(GetOwner(), MediaSourceEnum::Other, audioInputType,
-                             MediaSinkEnum::Speaker,
                             DeviceEnumerationType::Normal,
-                             DeviceEnumerationType::Normal, true, devices)
+                             DeviceEnumerationType::Normal,
+                             EnumerationFlag::EnumerateAudioOutputs, devices)
      ->Then(
          GetCurrentSerialEventTarget(), __func__,
          [aDeviceId, isExposed, devices](bool) mutable {
--- a/dom/media/MediaManager.cpp
+++ b/dom/media/MediaManager.cpp
@ -1793,13 +1793,13 @@ void MediaManager::GuessVideoDeviceGroupIDs(MediaDeviceSet& aDevices,

 RefPtr<MediaManager::MgrPromise> MediaManager::EnumerateRawDevices(
    MediaSourceEnum aVideoInputType, MediaSourceEnum aAudioInputType,
-    MediaSinkEnum aAudioOutputType, DeviceEnumerationType aVideoInputEnumType,
-    DeviceEnumerationType aAudioInputEnumType, bool aForceNoPermRequest,
+    DeviceEnumerationType aVideoInputEnumType,
+    DeviceEnumerationType aAudioInputEnumType, EnumerationFlags aFlags,
    const RefPtr<MediaDeviceSetRefCnt>& aOutDevices) {
  MOZ_ASSERT(NS_IsMainThread());
  MOZ_ASSERT(aVideoInputType != MediaSourceEnum::Other ||
             aAudioInputType != MediaSourceEnum::Other ||
-             aAudioOutputType != MediaSinkEnum::Other);
+             aFlags.contains(EnumerationFlag::EnumerateAudioOutputs));
  // Since the enums can take one of several values, the following asserts rely
  // on short circuting behavior. E.g. aVideoInputEnumType != Fake will be true
  // if the requested device is not fake and thus the assert will pass. However,
@ -1830,7 +1830,8 @@ RefPtr<MediaManager::MgrPromise> MediaManager::EnumerateRawDevices(

  const bool hasVideo = aVideoInputType != MediaSourceEnum::Other;
  const bool hasAudio = aAudioInputType != MediaSourceEnum::Other;
-  const bool hasAudioOutput = aAudioOutputType == MediaSinkEnum::Speaker;
+  const bool hasAudioOutput =
+      aFlags.contains(EnumerationFlag::EnumerateAudioOutputs);

  // True of at least one of video input or audio input is a fake device
  const bool fakeDeviceRequested =
@ -1944,7 +1945,8 @@ RefPtr<MediaManager::MgrPromise> MediaManager::EnumerateRawDevices(
        holder.Resolve(false, __func__);
      });

-  if (realDeviceRequested && aForceNoPermRequest &&
+  if (realDeviceRequested &&
+      aFlags.contains(EnumerationFlag::AllowPermissionRequest) &&
      Preferences::GetBool("media.navigator.permission.device", false)) {
    // Need to ask permission to retrieve list of all devices;
    // notify frontend observer and wait for callback notification to post task.
@ -2248,8 +2250,8 @@ void MediaManager::DeviceListChanged() {
            }
            return EnumerateRawDevices(
                MediaSourceEnum::Camera, MediaSourceEnum::Microphone,
-                MediaSinkEnum::Speaker, DeviceEnumerationType::Normal,
-                DeviceEnumerationType::Normal, false, devices);
+                DeviceEnumerationType::Normal, DeviceEnumerationType::Normal,
+                EnumerationFlag::EnumerateAudioOutputs, devices);
          },
          []() {
            // Timer was canceled by us, or we're in shutdown.
@ -2716,8 +2718,8 @@ RefPtr<MediaManager::StreamPromise> MediaManager::GetUserMedia(
  RefPtr<MediaManager> self = this;
  auto devices = MakeRefPtr<MediaDeviceSetRefCnt>();
  return EnumerateDevicesImpl(aWindow, videoType, audioType,
-                              MediaSinkEnum::Other, videoEnumerationType,
-                              audioEnumerationType, true, devices)
+                              videoEnumerationType, audioEnumerationType,
+                              EnumerationFlag::AllowPermissionRequest, devices)
      ->Then(
          GetCurrentSerialEventTarget(), __func__,
          [self, windowID, c, windowListener, aCallerType, devices](bool) {
@ -2935,9 +2937,8 @@ nsresult MediaManager::AnonymizeId(nsAString& aId,

 RefPtr<MediaManager::MgrPromise> MediaManager::EnumerateDevicesImpl(
    nsPIDOMWindowInner* aWindow, MediaSourceEnum aVideoInputType,
-    MediaSourceEnum aAudioInputType, MediaSinkEnum aAudioOutputType,
-    DeviceEnumerationType aVideoInputEnumType,
-    DeviceEnumerationType aAudioInputEnumType, bool aForceNoPermRequest,
+    MediaSourceEnum aAudioInputType, DeviceEnumerationType aVideoInputEnumType,
+    DeviceEnumerationType aAudioInputEnumType, EnumerationFlags aFlags,
    const RefPtr<MediaDeviceSetRefCnt>& aOutDevices) {
  MOZ_ASSERT(NS_IsMainThread());

@ -2987,9 +2988,9 @@ RefPtr<MediaManager::MgrPromise> MediaManager::EnumerateDevicesImpl(
  return media::GetPrincipalKey(principalInfo, persist)
      ->Then(
          GetMainThreadSerialEventTarget(), __func__,
-          [windowId, aVideoInputType, aAudioInputType, aAudioOutputType,
-           aVideoInputEnumType, aAudioInputEnumType, aForceNoPermRequest,
-           aOutDevices, originKey](const nsCString& aOriginKey) {
+          [windowId, aVideoInputType, aAudioInputType, aVideoInputEnumType,
+           aAudioInputEnumType, aFlags, aOutDevices,
+           originKey](const nsCString& aOriginKey) {
            MOZ_ASSERT(NS_IsMainThread());
            originKey->Assign(aOriginKey);
            MediaManager* mgr = MediaManager::GetIfExists();
@ -3000,9 +3001,8 @@ RefPtr<MediaManager::MgrPromise> MediaManager::EnumerateDevicesImpl(
                  __func__);
            }
            return mgr->EnumerateRawDevices(
-                aVideoInputType, aAudioInputType, aAudioOutputType,
-                aVideoInputEnumType, aAudioInputEnumType, aForceNoPermRequest,
-                aOutDevices);
+                aVideoInputType, aAudioInputType, aVideoInputEnumType,
+                aAudioInputEnumType, aFlags, aOutDevices);
          },
          [](nsresult rs) {
            NS_WARNING(
@ -3078,16 +3078,15 @@ RefPtr<MediaManager::DeviceSetPromise> MediaManager::EnumerateDevices(
      FeaturePolicyUtils::IsFeatureAllowed(doc, u"microphone"_ns)
          ? MediaSourceEnum::Microphone
          : MediaSourceEnum::Other;
-  MediaSinkEnum audioOutputType =
-      Preferences::GetBool("media.setsinkid.enabled") &&
-              FeaturePolicyUtils::IsFeatureAllowed(doc, u"speaker-selection"_ns)
-          ? MediaSinkEnum::Speaker
-          : MediaSinkEnum::Other;
+  EnumerationFlags flags;
+  if (Preferences::GetBool("media.setsinkid.enabled") &&
+      FeaturePolicyUtils::IsFeatureAllowed(doc, u"speaker-selection"_ns)) {
+    flags += EnumerationFlag::EnumerateAudioOutputs;
+  }

  auto devices = MakeRefPtr<MediaDeviceSetRefCnt>();
  if (audioType == MediaSourceEnum::Other &&
-      videoType == MediaSourceEnum::Other &&
-      audioOutputType == MediaSinkEnum::Other) {
+      videoType == MediaSourceEnum::Other && flags.isEmpty()) {
    return DeviceSetPromise::CreateAndResolve(devices, __func__);
  }

@ -3123,8 +3122,8 @@ RefPtr<MediaManager::DeviceSetPromise> MediaManager::EnumerateDevices(
    }
  }

-  return EnumerateDevicesImpl(aWindow, videoType, audioType, audioOutputType,
-                              videoEnumerationType, audioEnumerationType, false,
+  return EnumerateDevicesImpl(aWindow, videoType, audioType,
+                              videoEnumerationType, audioEnumerationType, flags,
                              devices)
      ->Then(
          GetCurrentSerialEventTarget(), __func__,
@ -3171,10 +3170,12 @@ RefPtr<MediaManager::DevicePromise> MediaManager::SelectAudioOutput(
  }
  uint64_t windowID = aWindow->WindowID();
  auto devices = MakeRefPtr<MediaDeviceSetRefCnt>();
-  return EnumerateDevicesImpl(aWindow, MediaSourceEnum::Other,
-                              MediaSourceEnum::Other, MediaSinkEnum::Speaker,
-                              DeviceEnumerationType::Normal,
-                              DeviceEnumerationType::Normal, true, devices)
+  return EnumerateDevicesImpl(
+             aWindow, MediaSourceEnum::Other, MediaSourceEnum::Other,
+             DeviceEnumerationType::Normal, DeviceEnumerationType::Normal,
+             {EnumerationFlag::EnumerateAudioOutputs,
+              EnumerationFlag::AllowPermissionRequest},
+             devices)
      ->Then(
          GetCurrentSerialEventTarget(), __func__,
          [self = RefPtr<MediaManager>(this), windowID, devices, aOptions,
--- a/dom/media/MediaManager.h
+++ b/dom/media/MediaManager.h
@ -5,7 +5,6 @@
 #ifndef MOZILLA_MEDIAMANAGER_H
 #define MOZILLA_MEDIAMANAGER_H

-#include "MediaEngine.h"
 #include "MediaEnginePrefs.h"
 #include "MediaEventSource.h"
 #include "mozilla/dom/GetUserMediaRequest.h"
@ -38,16 +37,20 @@
 #  include "transport/runnable_utils.h"
 #endif

+class AudioDeviceInfo;
 class nsIPrefBranch;

 namespace mozilla {
+class MediaEngine;
 class TaskQueue;
 class MediaTimer;
+class MediaTrack;
 namespace dom {
 struct AudioOutputOptions;
 struct MediaStreamConstraints;
 struct MediaTrackConstraints;
 struct MediaTrackConstraintSet;
+struct MediaTrackSettings;
 enum class CallerType : uint32_t;
 enum class MediaDeviceKind : uint8_t;
 }  // namespace dom
@ -90,7 +93,7 @@ class MediaDevice : public nsIMediaDevice {
                    const MediaEnginePrefs& aPrefs, uint64_t aWindowId,
                    const char** aOutBadConstraint);
  void SetTrack(const RefPtr<MediaTrack>& aTrack,
-                const PrincipalHandle& aPrincipal);
+                const nsMainThreadPtrHandle<nsIPrincipal>& aPrincipal);
  nsresult Start();
  nsresult Reconfigure(const dom::MediaTrackConstraints& aConstraints,
                       const MediaEnginePrefs& aPrefs,
@ -240,11 +243,13 @@ class MediaManager final : public nsIMediaManagerService,
    Loopback /* Enumeration should return loopback device(s) (possibly in
             addition to normal devices) */
  };
+  enum class EnumerationFlag { AllowPermissionRequest, EnumerateAudioOutputs };
+  using EnumerationFlags = EnumSet<EnumerationFlag>;
  RefPtr<MgrPromise> EnumerateDevicesImpl(
      nsPIDOMWindowInner* aWindow, dom::MediaSourceEnum aVideoInputType,
-      dom::MediaSourceEnum aAudioInputType, MediaSinkEnum aAudioOutputType,
+      dom::MediaSourceEnum aAudioInputType,
      DeviceEnumerationType aVideoInputEnumType,
-      DeviceEnumerationType aAudioInputEnumType, bool aForceNoPermRequest,
+      DeviceEnumerationType aAudioInputEnumType, EnumerationFlags aFlags,
      const RefPtr<MediaDeviceSetRefCnt>& aOutDevices);

  RefPtr<DevicePromise> SelectAudioOutput(
@ -286,9 +291,9 @@ class MediaManager final : public nsIMediaManagerService,
 private:
  RefPtr<MgrPromise> EnumerateRawDevices(
      dom::MediaSourceEnum aVideoInputType,
-      dom::MediaSourceEnum aAudioInputType, MediaSinkEnum aAudioOutputType,
+      dom::MediaSourceEnum aAudioInputType,
      DeviceEnumerationType aVideoInputEnumType,
-      DeviceEnumerationType aAudioInputEnumType, bool aForceNoPermRequest,
+      DeviceEnumerationType aAudioInputEnumType, EnumerationFlags aFlags,
      const RefPtr<MediaDeviceSetRefCnt>& aOutDevices);

  RefPtr<BadConstraintsPromise> SelectSettings(
--- a/dom/media/gtest/TestGroupId.cpp
+++ b/dom/media/gtest/TestGroupId.cpp
@ -10,6 +10,7 @@
 #include "gtest/gtest.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/UniquePtr.h"
+#include "mozilla/dom/MediaTrackSettingsBinding.h"
 #include "nsTArray.h"
 #include "webrtc/MediaEngineSource.h"

--- a/dom/media/webrtc/MediaEnginePrefs.h
+++ b/dom/media/webrtc/MediaEnginePrefs.h
@ -7,6 +7,9 @@
 #ifndef MediaEnginePrefs_h
 #define MediaEnginePrefs_h

+#include <stdint.h>
+#include <string.h>
+
 namespace mozilla {

 /**
--- a/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
+++ b/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
@ -9,6 +9,7 @@
 #include "Layers.h"
 #include "MediaManager.h"
 #include "MediaTrackConstraints.h"
+#include "mozilla/dom/MediaTrackSettingsBinding.h"
 #include "mozilla/ErrorNames.h"
 #include "mozilla/gfx/Point.h"
 #include "mozilla/RefPtr.h"