Bug 1430993 - Clean up minor issues in CamerasParent. r?jib draft
authorAndreas Pehrson <pehrsons@mozilla.com>
Thu, 25 Jan 2018 10:41:20 +0100
changeset 748660 714858767692b785299c366c9b01e80ad7478fb7
parent 748659 3951d585e6240e4dffacf88bce417d4d4206fb5b
push id97222
push userbmo:apehrson@mozilla.com
push dateTue, 30 Jan 2018 08:09:06 +0000
reviewersjib
bugs1430993
milestone59.0a1
Bug 1430993 - Clean up minor issues in CamerasParent. r?jib This cleans up: - Superfluous namespaces - Warnings from static-analysis - Raw pointer usage MozReview-Commit-ID: 4oJ5MiBwhmn
dom/media/systemservices/CamerasParent.cpp
dom/media/systemservices/CamerasParent.h
--- a/dom/media/systemservices/CamerasParent.cpp
+++ b/dom/media/systemservices/CamerasParent.cpp
@@ -95,17 +95,17 @@ void InputObserver::OnDeviceChange() {
       return NS_OK;
     });
 
   nsIEventTarget* target = mParent->GetBackgroundEventTarget();
   MOZ_ASSERT(target != nullptr);
   target->Dispatch(ipc_runnable, NS_DISPATCH_NORMAL);
 };
 
-class DeliverFrameRunnable : public ::mozilla::Runnable {
+class DeliverFrameRunnable : public mozilla::Runnable {
 public:
   DeliverFrameRunnable(CamerasParent* aParent,
                        CaptureEngine aEngine,
                        uint32_t aStreamId,
                        const webrtc::VideoFrame& aFrame,
                        const VideoFrameProperties& aProperties)
     : Runnable("camera::DeliverFrameRunnable")
     , mParent(aParent)
@@ -155,17 +155,17 @@ public:
     return mResult;
   }
 
 private:
   RefPtr<CamerasParent> mParent;
   CaptureEngine mCapEngine;
   uint32_t mStreamId;
   ShmemBuffer mBuffer;
-  mozilla::UniquePtr<unsigned char[]> mAlternateBuffer;
+  UniquePtr<unsigned char[]> mAlternateBuffer;
   VideoFrameProperties mProperties;
   int mResult;
 };
 
 NS_IMPL_ISUPPORTS(CamerasParent, nsIObserver)
 
 NS_IMETHODIMP
 CamerasParent::Observe(nsISupports *aSubject,
@@ -313,17 +313,17 @@ CallbackHelper::OnFrame(const webrtc::Vi
     // the DeliverFrameRunnable constructor.
   } else {
     // Shared memory buffers of the right size are available, do the copy here.
     VideoFrameUtils::CopyVideoFrameBuffers(shMemBuffer.GetBytes(),
                                            properties.bufferSize(), aVideoFrame);
     runnable = new DeliverFrameRunnable(mParent, mCapEngine, mStreamId,
                                         Move(shMemBuffer), properties);
   }
-  if (!runnable.get()) {
+  if (!runnable) {
     runnable = new DeliverFrameRunnable(mParent, mCapEngine, mStreamId,
                                         aVideoFrame, properties);
   }
   MOZ_ASSERT(mParent);
   nsIEventTarget* target = mParent->GetBackgroundEventTarget();
   MOZ_ASSERT(target != nullptr);
   target->Dispatch(runnable, NS_DISPATCH_NORMAL);
 }
@@ -336,61 +336,61 @@ CamerasParent::RecvReleaseFrame(mozilla:
 
 bool
 CamerasParent::SetupEngine(CaptureEngine aCapEngine)
 {
   LOG((__PRETTY_FUNCTION__));
   StaticRefPtr<VideoEngine>& engine = sEngines[aCapEngine];
 
   if (!engine) {
-    webrtc::CaptureDeviceInfo *captureDeviceInfo = nullptr;  
-    UniquePtr<webrtc::Config> config(new webrtc::Config);
+    UniquePtr<webrtc::CaptureDeviceInfo> captureDeviceInfo;
+    auto config = MakeUnique<webrtc::Config>();
 
     switch (aCapEngine) {
-    case ScreenEngine:
-      captureDeviceInfo =
-        new webrtc::CaptureDeviceInfo(webrtc::CaptureDeviceType::Screen);
-      break;
-    case BrowserEngine:
-      captureDeviceInfo =
-        new webrtc::CaptureDeviceInfo(webrtc::CaptureDeviceType::Browser);
-      break;
-    case WinEngine:
-      captureDeviceInfo =
-        new webrtc::CaptureDeviceInfo(webrtc::CaptureDeviceType::Window);
-      break;
-    case AppEngine:
-      captureDeviceInfo =
-        new webrtc::CaptureDeviceInfo(webrtc::CaptureDeviceType::Application);
-      break;
-    case CameraEngine:
-      captureDeviceInfo =
-        new webrtc::CaptureDeviceInfo(webrtc::CaptureDeviceType::Camera);
-      break;
-    default:
-      LOG(("Invalid webrtc Video engine"));
-      MOZ_CRASH();
-      break;
+      case ScreenEngine:
+        captureDeviceInfo = MakeUnique<webrtc::CaptureDeviceInfo>(
+            webrtc::CaptureDeviceType::Screen);
+        break;
+      case BrowserEngine:
+        captureDeviceInfo = MakeUnique<webrtc::CaptureDeviceInfo>(
+            webrtc::CaptureDeviceType::Browser);
+        break;
+      case WinEngine:
+        captureDeviceInfo = MakeUnique<webrtc::CaptureDeviceInfo>(
+            webrtc::CaptureDeviceType::Window);
+        break;
+      case AppEngine:
+        captureDeviceInfo = MakeUnique<webrtc::CaptureDeviceInfo>(
+            webrtc::CaptureDeviceType::Application);
+        break;
+      case CameraEngine:
+        captureDeviceInfo = MakeUnique<webrtc::CaptureDeviceInfo>(
+            webrtc::CaptureDeviceType::Camera);
+        break;
+      default:
+        LOG(("Invalid webrtc Video engine"));
+        MOZ_CRASH();
+        break;
     }
 
-    config->Set<webrtc::CaptureDeviceInfo>(captureDeviceInfo);
+    config->Set<webrtc::CaptureDeviceInfo>(captureDeviceInfo.release());
     engine = VideoEngine::Create(Move(config));
 
     if (!engine) {
       LOG(("VideoEngine::Create failed"));
       return false;
     }
   }
 
   if (aCapEngine == CameraEngine && !mCameraObserver) {
     mCameraObserver = new InputObserver(this);
     auto device_info = engine->GetOrCreateVideoCaptureDeviceInfo();
     MOZ_ASSERT(device_info);
     if (device_info) {
-      device_info->RegisterVideoInputFeedBack(mCameraObserver.get());
+      device_info->RegisterVideoInputFeedBack(mCameraObserver);
     }
   }
 
   return true;
 }
 
 void
 CamerasParent::CloseEngines()
@@ -410,36 +410,36 @@ CamerasParent::CloseEngines()
     Unused << ReleaseCaptureDevice(capEngine, streamNum);
   }
 
   StaticRefPtr<VideoEngine>& engine = sEngines[CameraEngine];
   if (engine && mCameraObserver) {
     auto device_info = engine->GetOrCreateVideoCaptureDeviceInfo();
     MOZ_ASSERT(device_info);
     if (device_info) {
-      device_info->DeRegisterVideoInputFeedBack(mCameraObserver.get());
+      device_info->DeRegisterVideoInputFeedBack(mCameraObserver);
     }
     mCameraObserver = nullptr;
   }
 
   // CloseEngines() is protected by sThreadMonitor
   sNumOfOpenCamerasParentEngines--;
   if (sNumOfOpenCamerasParentEngines == 0) {
-    for (auto& engine : sEngines) {
+    for (StaticRefPtr<VideoEngine>& engine : sEngines) {
       if (engine) {
         VideoEngine::Delete(engine);
         engine = nullptr;
       }
     }
   }
 
   mWebRTCAlive = false;
 }
 
-VideoEngine *
+VideoEngine*
 CamerasParent::EnsureInitialized(int aEngine)
 {
   LOG_VERBOSE((__PRETTY_FUNCTION__));
   // We're shutting down, don't try to do new WebRTC ops.
   if (!mWebRTCAlive) {
     return nullptr;
   }
   CaptureEngine capEngine = static_cast<CaptureEngine>(aEngine);
@@ -470,25 +470,26 @@ CamerasParent::RecvNumberOfCaptureDevice
           num = devInfo->NumberOfDevices();
         }
       }
       RefPtr<nsIRunnable> ipc_runnable =
         media::NewRunnableFrom([self, num]() -> nsresult {
           if (!self->mChildIsAlive) {
             return NS_ERROR_FAILURE;
           }
+
           if (num < 0) {
             LOG(("RecvNumberOfCaptureDevices couldn't find devices"));
             Unused << self->SendReplyFailure();
             return NS_ERROR_FAILURE;
-          } else {
-            LOG(("RecvNumberOfCaptureDevices: %d", num));
-            Unused << self->SendReplyNumberOfCaptureDevices(num);
-            return NS_OK;
           }
+
+          LOG(("RecvNumberOfCaptureDevices: %d", num));
+          Unused << self->SendReplyNumberOfCaptureDevices(num);
+          return NS_OK;
         });
         self->mPBackgroundEventTarget->Dispatch(ipc_runnable, NS_DISPATCH_NORMAL);
       return NS_OK;
     });
   DispatchToVideoCaptureThread(webrtc_runnable);
   return IPC_OK();
 }
 
@@ -502,25 +503,26 @@ CamerasParent::RecvEnsureInitialized(con
     media::NewRunnableFrom([self, aCapEngine]() -> nsresult {
       bool result = self->EnsureInitialized(aCapEngine);
 
       RefPtr<nsIRunnable> ipc_runnable =
         media::NewRunnableFrom([self, result]() -> nsresult {
           if (!self->mChildIsAlive) {
             return NS_ERROR_FAILURE;
           }
+
           if (!result) {
             LOG(("RecvEnsureInitialized failed"));
             Unused << self->SendReplyFailure();
             return NS_ERROR_FAILURE;
-          } else {
-            LOG(("RecvEnsureInitialized succeeded"));
-            Unused << self->SendReplySuccess();
-            return NS_OK;
           }
+
+          LOG(("RecvEnsureInitialized succeeded"));
+          Unused << self->SendReplySuccess();
+          return NS_OK;
         });
         self->mPBackgroundEventTarget->Dispatch(ipc_runnable, NS_DISPATCH_NORMAL);
       return NS_OK;
     });
   DispatchToVideoCaptureThread(webrtc_runnable);
   return IPC_OK();
 }
 
@@ -540,23 +542,24 @@ CamerasParent::RecvNumberOfCapabilities(
           num = devInfo->NumberOfCapabilities(unique_id.get());
         }
       }
       RefPtr<nsIRunnable> ipc_runnable =
         media::NewRunnableFrom([self, num]() -> nsresult {
           if (!self->mChildIsAlive) {
             return NS_ERROR_FAILURE;
           }
+
           if (num < 0) {
             LOG(("RecvNumberOfCapabilities couldn't find capabilities"));
             Unused << self->SendReplyFailure();
             return NS_ERROR_FAILURE;
-          } else {
-            LOG(("RecvNumberOfCapabilities: %d", num));
           }
+
+          LOG(("RecvNumberOfCapabilities: %d", num));
           Unused << self->SendReplyNumberOfCapabilities(num);
           return NS_OK;
         });
       self->mPBackgroundEventTarget->Dispatch(ipc_runnable, NS_DISPATCH_NORMAL);
       return NS_OK;
     });
   DispatchToVideoCaptureThread(webrtc_runnable);
   return IPC_OK();
@@ -762,24 +765,25 @@ CamerasParent::RecvAllocateCaptureDevice
             }
           });
         }
         RefPtr<nsIRunnable> ipc_runnable =
           media::NewRunnableFrom([self, numdev, error]() -> nsresult {
             if (!self->mChildIsAlive) {
               return NS_ERROR_FAILURE;
             }
+
             if (error) {
               Unused << self->SendReplyFailure();
               return NS_ERROR_FAILURE;
-            } else {
-              LOG(("Allocated device nr %d", numdev));
-              Unused << self->SendReplyAllocateCaptureDevice(numdev);
-              return NS_OK;
             }
+
+            LOG(("Allocated device nr %d", numdev));
+            Unused << self->SendReplyAllocateCaptureDevice(numdev);
+            return NS_OK;
           });
         self->mPBackgroundEventTarget->Dispatch(ipc_runnable, NS_DISPATCH_NORMAL);
         return NS_OK;
         });
       self->DispatchToVideoCaptureThread(webrtc_runnable);
       return NS_OK;
     });
   NS_DispatchToMainThread(mainthread_runnable);
@@ -808,25 +812,26 @@ CamerasParent::RecvReleaseCaptureDevice(
   RefPtr<Runnable> webrtc_runnable =
     media::NewRunnableFrom([self, aCapEngine, numdev]() -> nsresult {
       int error = self->ReleaseCaptureDevice(aCapEngine, numdev);
       RefPtr<nsIRunnable> ipc_runnable =
         media::NewRunnableFrom([self, error, numdev]() -> nsresult {
           if (!self->mChildIsAlive) {
             return NS_ERROR_FAILURE;
           }
+
           if (error) {
             Unused << self->SendReplyFailure();
             LOG(("Failed to free device nr %d", numdev));
             return NS_ERROR_FAILURE;
-          } else {
-            Unused << self->SendReplySuccess();
-            LOG(("Freed device nr %d", numdev));
-            return NS_OK;
           }
+
+          Unused << self->SendReplySuccess();
+          LOG(("Freed device nr %d", numdev));
+          return NS_OK;
         });
       self->mPBackgroundEventTarget->Dispatch(ipc_runnable, NS_DISPATCH_NORMAL);
       return NS_OK;
     });
   DispatchToVideoCaptureThread(webrtc_runnable);
   return IPC_OK();
 }
 
@@ -837,24 +842,22 @@ CamerasParent::RecvStartCapture(const Ca
 {
   LOG((__PRETTY_FUNCTION__));
 
   RefPtr<CamerasParent> self(this);
   RefPtr<Runnable> webrtc_runnable =
     media::NewRunnableFrom([self, aCapEngine, capnum, ipcCaps]() -> nsresult {
       LOG((__PRETTY_FUNCTION__));
       CallbackHelper** cbh;
-      VideoEngine* engine = nullptr;
       int error = -1;
       if (self->EnsureInitialized(aCapEngine)) {
         cbh = self->mCallbacks.AppendElement(
           new CallbackHelper(static_cast<CaptureEngine>(aCapEngine), capnum, self));
 
-        engine = self->sEngines[aCapEngine];
-        engine->WithEntry(capnum,
+        self->sEngines[aCapEngine]->WithEntry(capnum,
           [&capnum, &aCapEngine, &error, &ipcCaps, &cbh, self]
           (VideoEngine::CaptureEntry& cap) {
           webrtc::VideoCaptureCapability capability;
           capability.width = ipcCaps.width();
           capability.height = ipcCaps.height();
           capability.maxFPS = ipcCaps.maxFPS();
           capability.expectedCaptureDelay = ipcCaps.expectedCaptureDelay();
           capability.rawType = static_cast<webrtc::RawVideoType>(ipcCaps.rawType());
@@ -878,19 +881,19 @@ CamerasParent::RecvStartCapture(const Ca
                 capability.maxFPS = std::max(
                   capability.maxFPS, sAllRequestedCapabilities[it.first].maxFPS);
               }
             }
 
             auto candidateCapabilities = self->mAllCandidateCapabilities.find(
               nsCString(cap.VideoCapture()->CurrentDeviceName()));
             MOZ_DIAGNOSTIC_ASSERT(candidateCapabilities != self->mAllCandidateCapabilities.end());
-            MOZ_DIAGNOSTIC_ASSERT(candidateCapabilities->second.size() > 0);
+            MOZ_DIAGNOSTIC_ASSERT(!candidateCapabilities->second.empty());
             if ((candidateCapabilities != self->mAllCandidateCapabilities.end()) &&
-                (candidateCapabilities->second.size() > 0)) {
+                (!candidateCapabilities->second.empty())) {
               int32_t minIdx = -1;
               uint64_t minDistance = UINT64_MAX;
 
               for (auto & candidateCapability : candidateCapabilities->second) {
                 if (candidateCapability.second.rawType != capability.rawType) {
                   continue;
                 }
                 // The first priority is finding a suitable resolution.
@@ -930,23 +933,24 @@ CamerasParent::RecvStartCapture(const Ca
           }
         });
       }
       RefPtr<nsIRunnable> ipc_runnable =
         media::NewRunnableFrom([self, error]() -> nsresult {
           if (!self->mChildIsAlive) {
             return NS_ERROR_FAILURE;
           }
+
           if (!error) {
             Unused << self->SendReplySuccess();
             return NS_OK;
-          } else {
-            Unused << self->SendReplyFailure();
-            return NS_ERROR_FAILURE;
           }
+
+          Unused << self->SendReplyFailure();
+          return NS_ERROR_FAILURE;
         });
       self->mPBackgroundEventTarget->Dispatch(ipc_runnable, NS_DISPATCH_NORMAL);
       return NS_OK;
     });
   DispatchToVideoCaptureThread(webrtc_runnable);
   return IPC_OK();
 }
 
@@ -1109,14 +1113,13 @@ CamerasParent::~CamerasParent()
     delete sThreadMonitor;
     sThreadMonitor = nullptr;
   }
 }
 
 already_AddRefed<CamerasParent>
 CamerasParent::Create() {
   mozilla::ipc::AssertIsOnBackgroundThread();
-  RefPtr<CamerasParent> camerasParent = new CamerasParent();
-  return camerasParent.forget();
+  return MakeAndAddRef<CamerasParent>();
 }
 
-}
-}
+} // namespace camera
+} // namespace mozilla
--- a/dom/media/systemservices/CamerasParent.h
+++ b/dom/media/systemservices/CamerasParent.h
@@ -142,33 +142,33 @@ protected:
   // sNumOfCamerasParent and sThreadMonitor create & delete are protected by sMutex
   // sNumOfOpenCamerasParentEngines and sVideoCaptureThread are protected by sThreadMonitor
   static StaticRefPtr<VideoEngine> sEngines[CaptureEngine::MaxEngine];
   static int32_t sNumOfOpenCamerasParentEngines;
   static int32_t sNumOfCamerasParents;
   nsTArray<CallbackHelper*> mCallbacks;
 
   // image buffers
-  mozilla::ShmemPool mShmemPool;
+  ShmemPool mShmemPool;
 
   // PBackground parent thread
   nsCOMPtr<nsISerialEventTarget> mPBackgroundEventTarget;
 
   static StaticMutex sMutex;
   static Monitor* sThreadMonitor;
 
   // video processing thread - where webrtc.org capturer code runs
   static base::Thread* sVideoCaptureThread;
 
   // Shutdown handling
   bool mChildIsAlive;
   bool mDestroyed;
   // Above 2 are PBackground only, but this is potentially
   // read cross-thread.
-  mozilla::Atomic<bool> mWebRTCAlive;
+  Atomic<bool> mWebRTCAlive;
   RefPtr<InputObserver> mCameraObserver;
   std::map<nsCString, std::map<uint32_t, webrtc::VideoCaptureCapability>>
     mAllCandidateCapabilities;
 };
 
 PCamerasParent* CreateCamerasParent();
 
 } // namespace camera