Bug 1572281 - In DeviceChangeCallback class separate the observer from the subject functionality. r=pehrsons
authorAlex Chronopoulos <achronop@gmail.com>
Fri, 20 Sep 2019 10:11:31 +0000
changeset 494259 90286c5d1ef171475be1d02d337825ef5fcf3b14
parent 494258 394cb5645323fa4b0c61397fe2daa553157362ff
child 494260 9f15627684961ce192df91efb997b88b56e9cd98
push id114114
push userdluca@mozilla.com
push dateFri, 20 Sep 2019 22:00:08 +0000
treeherdermozilla-inbound@56e11fddf939 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspehrsons
bugs1572281
milestone71.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
dom/media/MediaManager.cpp
dom/media/MediaManager.h
dom/media/systemservices/CamerasChild.cpp
dom/media/systemservices/CamerasChild.h
dom/media/systemservices/DeviceChangeCallback.h
dom/media/webrtc/MediaEngine.h
--- a/dom/media/MediaManager.cpp
+++ b/dom/media/MediaManager.cpp
@@ -2171,27 +2171,27 @@ int MediaManager::AddDeviceChangeCallbac
     MOZ_RELEASE_ASSERT(manager);  // Must exist while media thread is alive
     // this is needed in case persistent permission is given but no gUM()
     // or enumeration() has created the real backend yet
     manager->GetBackend();
     if (fakeDeviceChangeEventOn)
       manager->GetBackend()->SetFakeDeviceChangeEvents();
   }));
 
-  return DeviceChangeCallback::AddDeviceChangeCallback(aCallback);
+  return DeviceChangeNotifier::AddDeviceChangeCallback(aCallback);
 }
 
 void MediaManager::OnDeviceChange() {
   NS_DispatchToMainThread(NS_NewRunnableFunction(
       "MediaManager::OnDeviceChange", [self = RefPtr<MediaManager>(this)]() {
         MOZ_ASSERT(NS_IsMainThread());
         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
         // devices list.
         PR_Sleep(PR_MillisecondsToInterval(200));
         auto devices = MakeRefPtr<MediaDeviceSetRefCnt>();
         self->EnumerateRawDevices(
                 0, MediaSourceEnum::Camera, MediaSourceEnum::Microphone,
@@ -3405,17 +3405,17 @@ void MediaManager::RemoveMediaDevicesCal
   MutexAutoLock lock(mCallbackMutex);
   for (DeviceChangeCallback* observer : mDeviceChangeCallbackList) {
     MediaDevices* mediadevices = static_cast<MediaDevices*>(observer);
     MOZ_ASSERT(mediadevices);
     if (mediadevices) {
       nsPIDOMWindowInner* window = mediadevices->GetOwner();
       MOZ_ASSERT(window);
       if (window && window->WindowID() == aWindowID) {
-        DeviceChangeCallback::RemoveDeviceChangeCallbackLocked(observer);
+        RemoveDeviceChangeCallbackLocked(observer);
         return;
       }
     }
   }
 }
 
 void MediaManager::AddWindowID(uint64_t aWindowId,
                                RefPtr<GetUserMediaWindowListener> aListener) {
--- a/dom/media/MediaManager.h
+++ b/dom/media/MediaManager.h
@@ -127,16 +127,17 @@ class MediaDevice : public nsIMediaDevic
 };
 
 typedef nsRefPtrHashtable<nsUint64HashKey, GetUserMediaWindowListener>
     WindowTable;
 typedef MozPromise<RefPtr<AudioDeviceInfo>, nsresult, true> SinkInfoPromise;
 
 class MediaManager final : public nsIMediaManagerService,
                            public nsIObserver,
+                           public DeviceChangeNotifier,
                            public DeviceChangeCallback {
   friend SourceListener;
 
  public:
   static already_AddRefed<MediaManager> GetInstance();
 
   // NOTE: never Dispatch(....,NS_DISPATCH_SYNC) to the MediaManager
   // thread from the MainThread, as we NS_DISPATCH_SYNC to MainThread
--- a/dom/media/systemservices/CamerasChild.cpp
+++ b/dom/media/systemservices/CamerasChild.cpp
@@ -47,17 +47,17 @@ class FakeOnDeviceChangeEventRunnable : 
       : Runnable("camera::FakeOnDeviceChangeEventRunnable"),
         mCounter(counter) {}
 
   NS_IMETHOD Run() override {
     OffTheBooksMutexAutoLock lock(CamerasSingleton::Mutex());
 
     CamerasChild* child = CamerasSingleton::Child();
     if (child) {
-      child->OnDeviceChange();
+      child->NotifyDeviceChange();
 
       if (mCounter++ < FAKE_ONDEVICECHANGE_EVENT_REPEAT_COUNT) {
         RefPtr<FakeOnDeviceChangeEventRunnable> evt =
             new FakeOnDeviceChangeEventRunnable(mCounter);
         CamerasSingleton::FakeDeviceChangeEventThread()->DelayedDispatch(
             evt.forget(), FAKE_ONDEVICECHANGE_EVENT_PERIOD_IN_MS);
       }
     }
@@ -149,17 +149,17 @@ int CamerasChild::AddDeviceChangeCallbac
 
   // In order to detect the event, we need to init the camera engine.
   // Currently EnsureInitialized(aCapEngine) is only called when one of
   // CamerasaParent api, e.g., RecvNumberOfCaptureDevices(), is called.
 
   // 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) {
   LOG(("%s", __PRETTY_FUNCTION__));
   MonitorAutoLock monitor(mReplyMonitor);
   mReceivedReply = true;
   mReplySuccess = false;
   monitor.Notify();
@@ -575,17 +575,17 @@ mozilla::ipc::IPCResult CamerasChild::Re
   } else {
     LOG(("DeliverFrame called with dead callback"));
   }
   SendReleaseFrame(std::move(shmem));
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult CamerasChild::RecvDeviceChange() {
-  this->OnDeviceChange();
+  this->NotifyDeviceChange();
   return IPC_OK();
 }
 
 int CamerasChild::SetFakeDeviceChangeEvents() {
   CamerasSingleton::Mutex().AssertCurrentThreadOwns();
 
   if (!CamerasSingleton::FakeDeviceChangeEventThread()) {
     nsresult rv = NS_NewNamedThread(
--- a/dom/media/systemservices/CamerasChild.h
+++ b/dom/media/systemservices/CamerasChild.h
@@ -138,17 +138,17 @@ int GetChildAndCall(MEM_FUN&& f, ARGS&&.
   CamerasChild* child = GetCamerasChild();
   if (child) {
     return (child->*f)(std::forward<ARGS>(args)...);
   } else {
     return -1;
   }
 }
 
-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;
 
  public:
   // We are owned by the PBackground thread only. CamerasSingleton
   // takes a non-owning reference.
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(CamerasChild)
--- a/dom/media/systemservices/DeviceChangeCallback.h
+++ b/dom/media/systemservices/DeviceChangeCallback.h
@@ -8,50 +8,56 @@
 #define mozilla_DeviceChangeCallback_h
 
 #include "mozilla/Mutex.h"
 
 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();
     }
   }
 
   virtual int AddDeviceChangeCallback(DeviceChangeCallback* aCallback) {
     MutexAutoLock lock(mCallbackMutex);
     if (mDeviceChangeCallbackList.IndexOf(aCallback) ==
         mDeviceChangeCallbackList.NoIndex)
       mDeviceChangeCallbackList.AppendElement(aCallback);
     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) !=
         mDeviceChangeCallbackList.NoIndex)
       mDeviceChangeCallbackList.RemoveElement(aCallback);
     return 0;
   }
 
-  DeviceChangeCallback()
-      : mCallbackMutex("mozilla::media::DeviceChangeCallback::mCallbackMutex") {
-  }
-
-  virtual ~DeviceChangeCallback() {}
+  virtual ~DeviceChangeNotifier() = default;
 
  protected:
   nsTArray<DeviceChangeCallback*> mDeviceChangeCallbackList;
   Mutex mCallbackMutex;
 };
 
 }  // namespace mozilla
 
--- a/dom/media/webrtc/MediaEngine.h
+++ b/dom/media/webrtc/MediaEngine.h
@@ -25,17 +25,18 @@ class MediaEngineSource;
 
 enum MediaSinkEnum {
   Speaker,
   Other,
 };
 
 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
 
   void AssertIsOnOwningThread() const { NS_ASSERT_OWNINGTHREAD(MediaEngine); }
 
   /**
    * Populate an array of sources of the requested type in the nsTArray.
@@ -44,15 +45,17 @@ class MediaEngine : public DeviceChangeC
   virtual void EnumerateDevices(uint64_t aWindowId, dom::MediaSourceEnum,
                                 MediaSinkEnum,
                                 nsTArray<RefPtr<MediaDevice>>*) = 0;
 
   virtual void Shutdown() = 0;
 
   virtual void SetFakeDeviceChangeEvents() {}
 
+  void OnDeviceChange() override { NotifyDeviceChange(); }
+
  protected:
   virtual ~MediaEngine() = default;
 };
 
 }  // namespace mozilla
 
 #endif /* MEDIAENGINE_H_ */