Bug 1572281 - In DeviceChangeCallback class separate the observer from the subject functionality. r=pehrsons

DeviceChangeCallback class implements the observer pattern. However, the role of the observer and the subject is integrated into the same class which makes use of virtual methods to allow a separation of the roles. This makes code reading difficult. Also, it does not allow from a class to inherit only the observer role or the subject role. This patch breaks the DeviceChangeCallback class into two classes according to the observer or subject functionality.

Differential Revision: https://phabricator.services.mozilla.com/D46270

--HG--
extra : moz-landing-system : lando

dom/media/MediaManager.cpp

@@ -2176,7 +2176,7 @@ int MediaManager::AddDeviceChangeCallback(DeviceChangeCallback* aCallback) {
       manager->GetBackend()->SetFakeDeviceChangeEvents();
   }));
 
-  return DeviceChangeCallback::AddDeviceChangeCallback(aCallback);
+  return DeviceChangeNotifier::AddDeviceChangeCallback(aCallback);
 }
 
 void MediaManager::OnDeviceChange() {
@@ -2186,7 +2186,7 @@ void MediaManager::OnDeviceChange() {
         if (sHasShutdown) {
           return;
         }
-        self->DeviceChangeCallback::OnDeviceChange();
+        self->NotifyDeviceChange();
 
         // On some Windows machine, if we call EnumerateRawDevices immediately
         // after receiving devicechange event, sometimes we would get outdated
@@ -3410,7 +3410,7 @@ void MediaManager::RemoveMediaDevicesCallback(uint64_t aWindowID) {
       nsPIDOMWindowInner* window = mediadevices->GetOwner();
       MOZ_ASSERT(window);
       if (window && window->WindowID() == aWindowID) {
-        DeviceChangeCallback::RemoveDeviceChangeCallbackLocked(observer);
+        RemoveDeviceChangeCallbackLocked(observer);
         return;
       }
     }

dom/media/MediaManager.h

@@ -132,6 +132,7 @@ typedef MozPromise<RefPtr<AudioDeviceInfo>, nsresult, true> SinkInfoPromise;
 
 class MediaManager final : public nsIMediaManagerService,
                            public nsIObserver,
+                           public DeviceChangeNotifier,
                            public DeviceChangeCallback {
   friend SourceListener;
 

dom/media/systemservices/CamerasChild.cpp

@@ -52,7 +52,7 @@ class FakeOnDeviceChangeEventRunnable : public Runnable {
 
     CamerasChild* child = CamerasSingleton::Child();
     if (child) {
-      child->OnDeviceChange();
+      child->NotifyDeviceChange();
 
       if (mCounter++ < FAKE_ONDEVICECHANGE_EVENT_REPEAT_COUNT) {
         RefPtr<FakeOnDeviceChangeEventRunnable> evt =
@@ -154,7 +154,7 @@ int CamerasChild::AddDeviceChangeCallback(DeviceChangeCallback* aCallback) {
   // So here we setup camera engine via EnsureInitialized(aCapEngine)
 
   EnsureInitialized(CameraEngine);
-  return DeviceChangeCallback::AddDeviceChangeCallback(aCallback);
+  return DeviceChangeNotifier::AddDeviceChangeCallback(aCallback);
 }
 
 mozilla::ipc::IPCResult CamerasChild::RecvReplyFailure(void) {
@@ -580,7 +580,7 @@ mozilla::ipc::IPCResult CamerasChild::RecvDeliverFrame(
 }
 
 mozilla::ipc::IPCResult CamerasChild::RecvDeviceChange() {
-  this->OnDeviceChange();
+  this->NotifyDeviceChange();
   return IPC_OK();
 }
 

dom/media/systemservices/CamerasChild.h

@@ -143,7 +143,7 @@ int GetChildAndCall(MEM_FUN&& f, ARGS&&... args) {
   }
 }
 
-class CamerasChild final : public PCamerasChild, public DeviceChangeCallback {
+class CamerasChild final : public PCamerasChild, public DeviceChangeNotifier {
   friend class mozilla::ipc::BackgroundChildImpl;
   template <class T>
   friend class mozilla::camera::LockAndDispatch;

dom/media/systemservices/DeviceChangeCallback.h

@@ -13,7 +13,17 @@ namespace mozilla {
 
 class DeviceChangeCallback {
  public:
-  virtual void OnDeviceChange() {
+  virtual ~DeviceChangeCallback() = default;
+  virtual void OnDeviceChange() = 0;
+};
+
+class DeviceChangeNotifier {
+ public:
+  DeviceChangeNotifier()
+      : mCallbackMutex("mozilla::DeviceChangeCallback::mCallbackMutex") {
+  }
+
+  void NotifyDeviceChange() {
     MutexAutoLock lock(mCallbackMutex);
     for (DeviceChangeCallback* observer : mDeviceChangeCallbackList) {
       observer->OnDeviceChange();
@@ -28,12 +38,12 @@ class DeviceChangeCallback {
     return 0;
   }
 
-  virtual int RemoveDeviceChangeCallback(DeviceChangeCallback* aCallback) {
+  int RemoveDeviceChangeCallback(DeviceChangeCallback* aCallback) {
     MutexAutoLock lock(mCallbackMutex);
     return RemoveDeviceChangeCallbackLocked(aCallback);
   }
 
-  virtual int RemoveDeviceChangeCallbackLocked(
+  int RemoveDeviceChangeCallbackLocked(
       DeviceChangeCallback* aCallback) {
     mCallbackMutex.AssertCurrentThreadOwns();
     if (mDeviceChangeCallbackList.IndexOf(aCallback) !=
@@ -42,11 +52,7 @@ class DeviceChangeCallback {
     return 0;
   }
 
-  DeviceChangeCallback()
-      : mCallbackMutex("mozilla::media::DeviceChangeCallback::mCallbackMutex") {
-  }
-
-  virtual ~DeviceChangeCallback() {}
+  virtual ~DeviceChangeNotifier() = default;
 
  protected:
   nsTArray<DeviceChangeCallback*> mDeviceChangeCallbackList;

dom/media/webrtc/MediaEngine.h

@@ -30,7 +30,8 @@ enum MediaSinkEnum {
 
 enum { kVideoTrack = 1, kAudioTrack = 2, kTrackCount };
 
-class MediaEngine : public DeviceChangeCallback {
+class MediaEngine : public DeviceChangeNotifier,
+                    public DeviceChangeCallback {
  public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(MediaEngine)
   NS_DECL_OWNINGTHREAD
@@ -49,6 +50,8 @@ class MediaEngine : public DeviceChangeCallback {
 
   virtual void SetFakeDeviceChangeEvents() {}
 
+  void OnDeviceChange() override { NotifyDeviceChange(); }
+
  protected:
   virtual ~MediaEngine() = default;
 };