Latent issue (not presently triggered) in camera control code: nsGonkCameraControl::OnTakePictureComplete copies the picture blob data into new buffer CameraControlImpl::OnTakePictureComplete forwards buffer to listeners DOMCameraControlListener::OnTakePictureComplete posts to the main thread and wraps the buffer via File::CreateMemoryFile The copy should happen for each listener instead of once in nsGonkCameraControl, because File::CreateMemoryFile assumes that it owns the buffer pointer and will free it when it goes out of scope.

Comment on attachment 8597733 [details] [diff] [review] bug1129051.patch, V2 Review of attachment 8597733 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me! try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8af61ffd252a

royang, could you update the patch with a description? See one of the patches in bug 994912 as an example. It should look something like: > Bug XXX - Description. r=aosmond > > diff --git ... > ... Also please request Commit Access (Level 1) so that you can push to the try server yourself. Follow the instructions at https://www.mozilla.org/en-US/about/governance/policies/commit/. Add a needinfo for me and I will vouch for you.

Comment on attachment 8597733 [details] [diff] [review] bug1129051.patch, V2 From: Roger Yang <royang51@gmail.com> Bug 1129051 - Fix Camera Control Listener double free. Fix webrtc memory leak. >diff --git a/dom/camera/CameraControlListener.h b/dom/camera/CameraControlListener.h >index 532fdaf..e489c11 100644 >--- a/dom/camera/CameraControlListener.h >+++ b/dom/camera/CameraControlListener.h >@@ -84,17 +84,17 @@ public: > public: > uint32_t mMaxMeteringAreas; > uint32_t mMaxFocusAreas; > }; > virtual void OnConfigurationChange(const CameraListenerConfiguration& aConfiguration) { } > > virtual void OnAutoFocusComplete(bool aAutoFocusSucceeded) { } > virtual void OnAutoFocusMoving(bool aIsMoving) { } >- virtual void OnTakePictureComplete(uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) { } >+ virtual void OnTakePictureComplete(const uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) { } > virtual void OnFacesDetected(const nsTArray<ICameraControl::Face>& aFaces) { } > > enum UserContext > { > kInStartCamera, > kInStopCamera, > kInAutoFocus, > kInStartFaceDetection, >diff --git a/dom/camera/DOMCameraControlListener.cpp b/dom/camera/DOMCameraControlListener.cpp >index 96f55ab..7a586c4 100644 >--- a/dom/camera/DOMCameraControlListener.cpp >+++ b/dom/camera/DOMCameraControlListener.cpp >@@ -336,28 +336,30 @@ DOMCameraControlListener::OnAutoFocusComplete(bool aAutoFocusSucceeded) > protected: > bool mAutoFocusSucceeded; > }; > > NS_DispatchToMainThread(new Callback(mDOMCameraControl, aAutoFocusSucceeded)); > } > > void >-DOMCameraControlListener::OnTakePictureComplete(uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) >+DOMCameraControlListener::OnTakePictureComplete(const uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) > { > class Callback : public DOMCallback > { > public: > Callback(nsMainThreadPtrHandle<nsISupports> aDOMCameraControl, >- uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) >+ const uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) > : DOMCallback(aDOMCameraControl) >- , mData(aData) > , mLength(aLength) > , mMimeType(aMimeType) >- { } >+ { >+ mData = (uint8_t*) malloc(aLength); >+ memcpy(mData, aData, aLength); >+ } > > void > RunCallback(nsDOMCameraControl* aDOMCameraControl) override > { > nsCOMPtr<nsIDOMBlob> picture = > File::CreateMemoryFile(mDOMCameraControl.get(), > static_cast<void*>(mData), > static_cast<uint64_t>(mLength), >diff --git a/dom/camera/DOMCameraControlListener.h b/dom/camera/DOMCameraControlListener.h >index a5c30c6..c1a1be8 100644 >--- a/dom/camera/DOMCameraControlListener.h >+++ b/dom/camera/DOMCameraControlListener.h >@@ -16,17 +16,17 @@ class CameraPreviewMediaStream; > class DOMCameraControlListener : public CameraControlListener > { > public: > DOMCameraControlListener(nsDOMCameraControl* aDOMCameraControl, CameraPreviewMediaStream* aStream); > > virtual void OnAutoFocusComplete(bool aAutoFocusSucceeded) override; > virtual void OnAutoFocusMoving(bool aIsMoving) override; > virtual void OnFacesDetected(const nsTArray<ICameraControl::Face>& aFaces) override; >- virtual void OnTakePictureComplete(uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) override; >+ virtual void OnTakePictureComplete(const uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) override; > > virtual void OnHardwareStateChange(HardwareState aState, nsresult aReason) override; > virtual void OnPreviewStateChange(PreviewState aState) override; > virtual void OnRecorderStateChange(RecorderState aState, int32_t aStatus, int32_t aTrackNum) override; > virtual void OnConfigurationChange(const CameraListenerConfiguration& aConfiguration) override; > virtual void OnShutter() override; > virtual void OnRateLimitPreview(bool aLimit) override; > virtual bool OnNewPreviewFrame(layers::Image* aImage, uint32_t aWidth, uint32_t aHeight) override; >diff --git a/dom/camera/GonkCameraControl.cpp b/dom/camera/GonkCameraControl.cpp >index 3e1eee7..41a15e5 100644 >--- a/dom/camera/GonkCameraControl.cpp >+++ b/dom/camera/GonkCameraControl.cpp >@@ -1529,24 +1529,20 @@ nsGonkCameraControl::OnFacesDetected(camera_frame_metadata_t* aMetaData) > CameraControlImpl::OnFacesDetected(faces); > } > > void > nsGonkCameraControl::OnTakePictureComplete(uint8_t* aData, uint32_t aLength) > { > ReentrantMonitorAutoEnter mon(mReentrantMonitor); > >- uint8_t* data = new uint8_t[aLength]; >- >- memcpy(data, aData, aLength); >- > nsString s(NS_LITERAL_STRING("image/")); > s.Append(mFileFormat); > DOM_CAMERA_LOGI("Got picture, type '%s', %u bytes\n", NS_ConvertUTF16toUTF8(s).get(), aLength); >- OnTakePictureComplete(data, aLength, s); >+ OnTakePictureComplete(aData, aLength, s); > > if (mResumePreviewAfterTakingPicture) { > nsresult rv = StartPreview(); > if (NS_FAILED(rv)) { > DOM_CAMERA_LOGE("Failed to restart camera preview (%x)\n", rv); > OnPreviewStateChange(CameraControlListener::kPreviewStopped); > } > } >diff --git a/dom/media/webrtc/MediaEngineGonkVideoSource.cpp b/dom/media/webrtc/MediaEngineGonkVideoSource.cpp >index 3243807..141e60b 100644 >--- a/dom/media/webrtc/MediaEngineGonkVideoSource.cpp >+++ b/dom/media/webrtc/MediaEngineGonkVideoSource.cpp >@@ -585,28 +585,28 @@ MediaEngineGonkVideoSource::OnUserError(UserContext aContext, nsresult aError) > MonitorAutoLock lock(mMonitor); > if (mPhotoCallbacks.Length()) { > NS_DispatchToMainThread(new TakePhotoError(mPhotoCallbacks, aError)); > } > } > } > > void >-MediaEngineGonkVideoSource::OnTakePictureComplete(uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) >+MediaEngineGonkVideoSource::OnTakePictureComplete(const uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) > { > // It needs to start preview because Gonk camera will stop preview while > // taking picture. > mCameraControl->StartPreview(); > > // Create a main thread runnable to generate a blob and call all current queued > // PhotoCallbacks. > class GenerateBlobRunnable : public nsRunnable { > public: > GenerateBlobRunnable(nsTArray<nsRefPtr<PhotoCallback>>& aCallbacks, >- uint8_t* aData, >+ const uint8_t* aData, > uint32_t aLength, > const nsAString& aMimeType) > : mPhotoDataLength(aLength) > { > mCallbacks.SwapElements(aCallbacks); > mPhotoData = (uint8_t*) malloc(aLength); > memcpy(mPhotoData, aData, mPhotoDataLength); > mMimeType = aMimeType; >diff --git a/dom/media/webrtc/MediaEngineGonkVideoSource.h b/dom/media/webrtc/MediaEngineGonkVideoSource.h >index 12d0372..b072f19 100644 >--- a/dom/media/webrtc/MediaEngineGonkVideoSource.h >+++ b/dom/media/webrtc/MediaEngineGonkVideoSource.h >@@ -69,17 +69,17 @@ public: > SourceMediaStream* aSource, > TrackID aId, > StreamTime aDesiredTime) override; > > void OnHardwareStateChange(HardwareState aState, nsresult aReason) override; > void GetRotation(); > bool OnNewPreviewFrame(layers::Image* aImage, uint32_t aWidth, uint32_t aHeight) override; > void OnUserError(UserContext aContext, nsresult aError) override; >- void OnTakePictureComplete(uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) override; >+ void OnTakePictureComplete(const uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) override; > > void AllocImpl(); > void DeallocImpl(); > void StartImpl(webrtc::CaptureCapability aCapability); > void StopImpl(); > uint32_t ConvertPixelFormatToFOURCC(int aFormat); > void RotateImage(layers::Image* aImage, uint32_t aWidth, uint32_t aHeight); > void Notify(const mozilla::hal::ScreenConfiguration& aConfiguration);

Comment on attachment 8597733 [details] [diff] [review] bug1129051.patch, V2 From: Roger Yang <royang51@gmail.com> Bug 1129051 - Fix double free in Camera Control Listener. Fix webrtc memory leak. >diff --git a/dom/camera/CameraControlListener.h b/dom/camera/CameraControlListener.h >index 532fdaf..e489c11 100644 >--- a/dom/camera/CameraControlListener.h >+++ b/dom/camera/CameraControlListener.h >@@ -84,17 +84,17 @@ public: > public: > uint32_t mMaxMeteringAreas; > uint32_t mMaxFocusAreas; > }; > virtual void OnConfigurationChange(const CameraListenerConfiguration& aConfiguration) { } > > virtual void OnAutoFocusComplete(bool aAutoFocusSucceeded) { } > virtual void OnAutoFocusMoving(bool aIsMoving) { } >- virtual void OnTakePictureComplete(uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) { } >+ virtual void OnTakePictureComplete(const uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) { } > virtual void OnFacesDetected(const nsTArray<ICameraControl::Face>& aFaces) { } > > enum UserContext > { > kInStartCamera, > kInStopCamera, > kInAutoFocus, > kInStartFaceDetection, >diff --git a/dom/camera/DOMCameraControlListener.cpp b/dom/camera/DOMCameraControlListener.cpp >index 96f55ab..7a586c4 100644 >--- a/dom/camera/DOMCameraControlListener.cpp >+++ b/dom/camera/DOMCameraControlListener.cpp >@@ -336,28 +336,30 @@ DOMCameraControlListener::OnAutoFocusComplete(bool aAutoFocusSucceeded) > protected: > bool mAutoFocusSucceeded; > }; > > NS_DispatchToMainThread(new Callback(mDOMCameraControl, aAutoFocusSucceeded)); > } > > void >-DOMCameraControlListener::OnTakePictureComplete(uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) >+DOMCameraControlListener::OnTakePictureComplete(const uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) > { > class Callback : public DOMCallback > { > public: > Callback(nsMainThreadPtrHandle<nsISupports> aDOMCameraControl, >- uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) >+ const uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) > : DOMCallback(aDOMCameraControl) >- , mData(aData) > , mLength(aLength) > , mMimeType(aMimeType) >- { } >+ { >+ mData = (uint8_t*) malloc(aLength); >+ memcpy(mData, aData, aLength); >+ } > > void > RunCallback(nsDOMCameraControl* aDOMCameraControl) override > { > nsCOMPtr<nsIDOMBlob> picture = > File::CreateMemoryFile(mDOMCameraControl.get(), > static_cast<void*>(mData), > static_cast<uint64_t>(mLength), >diff --git a/dom/camera/DOMCameraControlListener.h b/dom/camera/DOMCameraControlListener.h >index a5c30c6..c1a1be8 100644 >--- a/dom/camera/DOMCameraControlListener.h >+++ b/dom/camera/DOMCameraControlListener.h >@@ -16,17 +16,17 @@ class CameraPreviewMediaStream; > class DOMCameraControlListener : public CameraControlListener > { > public: > DOMCameraControlListener(nsDOMCameraControl* aDOMCameraControl, CameraPreviewMediaStream* aStream); > > virtual void OnAutoFocusComplete(bool aAutoFocusSucceeded) override; > virtual void OnAutoFocusMoving(bool aIsMoving) override; > virtual void OnFacesDetected(const nsTArray<ICameraControl::Face>& aFaces) override; >- virtual void OnTakePictureComplete(uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) override; >+ virtual void OnTakePictureComplete(const uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) override; > > virtual void OnHardwareStateChange(HardwareState aState, nsresult aReason) override; > virtual void OnPreviewStateChange(PreviewState aState) override; > virtual void OnRecorderStateChange(RecorderState aState, int32_t aStatus, int32_t aTrackNum) override; > virtual void OnConfigurationChange(const CameraListenerConfiguration& aConfiguration) override; > virtual void OnShutter() override; > virtual void OnRateLimitPreview(bool aLimit) override; > virtual bool OnNewPreviewFrame(layers::Image* aImage, uint32_t aWidth, uint32_t aHeight) override; >diff --git a/dom/camera/GonkCameraControl.cpp b/dom/camera/GonkCameraControl.cpp >index 3e1eee7..41a15e5 100644 >--- a/dom/camera/GonkCameraControl.cpp >+++ b/dom/camera/GonkCameraControl.cpp >@@ -1529,24 +1529,20 @@ nsGonkCameraControl::OnFacesDetected(camera_frame_metadata_t* aMetaData) > CameraControlImpl::OnFacesDetected(faces); > } > > void > nsGonkCameraControl::OnTakePictureComplete(uint8_t* aData, uint32_t aLength) > { > ReentrantMonitorAutoEnter mon(mReentrantMonitor); > >- uint8_t* data = new uint8_t[aLength]; >- >- memcpy(data, aData, aLength); >- > nsString s(NS_LITERAL_STRING("image/")); > s.Append(mFileFormat); > DOM_CAMERA_LOGI("Got picture, type '%s', %u bytes\n", NS_ConvertUTF16toUTF8(s).get(), aLength); >- OnTakePictureComplete(data, aLength, s); >+ OnTakePictureComplete(aData, aLength, s); > > if (mResumePreviewAfterTakingPicture) { > nsresult rv = StartPreview(); > if (NS_FAILED(rv)) { > DOM_CAMERA_LOGE("Failed to restart camera preview (%x)\n", rv); > OnPreviewStateChange(CameraControlListener::kPreviewStopped); > } > } >diff --git a/dom/media/webrtc/MediaEngineGonkVideoSource.cpp b/dom/media/webrtc/MediaEngineGonkVideoSource.cpp >index 3243807..141e60b 100644 >--- a/dom/media/webrtc/MediaEngineGonkVideoSource.cpp >+++ b/dom/media/webrtc/MediaEngineGonkVideoSource.cpp >@@ -585,28 +585,28 @@ MediaEngineGonkVideoSource::OnUserError(UserContext aContext, nsresult aError) > MonitorAutoLock lock(mMonitor); > if (mPhotoCallbacks.Length()) { > NS_DispatchToMainThread(new TakePhotoError(mPhotoCallbacks, aError)); > } > } > } > > void >-MediaEngineGonkVideoSource::OnTakePictureComplete(uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) >+MediaEngineGonkVideoSource::OnTakePictureComplete(const uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) > { > // It needs to start preview because Gonk camera will stop preview while > // taking picture. > mCameraControl->StartPreview(); > > // Create a main thread runnable to generate a blob and call all current queued > // PhotoCallbacks. > class GenerateBlobRunnable : public nsRunnable { > public: > GenerateBlobRunnable(nsTArray<nsRefPtr<PhotoCallback>>& aCallbacks, >- uint8_t* aData, >+ const uint8_t* aData, > uint32_t aLength, > const nsAString& aMimeType) > : mPhotoDataLength(aLength) > { > mCallbacks.SwapElements(aCallbacks); > mPhotoData = (uint8_t*) malloc(aLength); > memcpy(mPhotoData, aData, mPhotoDataLength); > mMimeType = aMimeType; >diff --git a/dom/media/webrtc/MediaEngineGonkVideoSource.h b/dom/media/webrtc/MediaEngineGonkVideoSource.h >index 12d0372..b072f19 100644 >--- a/dom/media/webrtc/MediaEngineGonkVideoSource.h >+++ b/dom/media/webrtc/MediaEngineGonkVideoSource.h >@@ -69,17 +69,17 @@ public: > SourceMediaStream* aSource, > TrackID aId, > StreamTime aDesiredTime) override; > > void OnHardwareStateChange(HardwareState aState, nsresult aReason) override; > void GetRotation(); > bool OnNewPreviewFrame(layers::Image* aImage, uint32_t aWidth, uint32_t aHeight) override; > void OnUserError(UserContext aContext, nsresult aError) override; >- void OnTakePictureComplete(uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) override; >+ void OnTakePictureComplete(const uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) override; > > void AllocImpl(); > void DeallocImpl(); > void StartImpl(webrtc::CaptureCapability aCapability); > void StopImpl(); > uint32_t ConvertPixelFormatToFOURCC(int aFormat); > void RotateImage(layers::Image* aImage, uint32_t aWidth, uint32_t aHeight); > void Notify(const mozilla::hal::ScreenConfiguration& aConfiguration);

Comment on attachment 8597733 [details] [diff] [review] bug1129051.patch, V2 >From: Roger Yang <royang51@gmail.com> > >Bug 1129051 - Fix Camera Control Listener double free. Fix webrtc memory leak r=royang51 > >diff --git a/dom/camera/CameraControlListener.h b/dom/camera/CameraControlListener.h >index 532fdaf..e489c11 100644 >--- a/dom/camera/CameraControlListener.h >+++ b/dom/camera/CameraControlListener.h >@@ -84,17 +84,17 @@ public: > public: > uint32_t mMaxMeteringAreas; > uint32_t mMaxFocusAreas; > }; > virtual void OnConfigurationChange(const CameraListenerConfiguration& aConfiguration) { } > > virtual void OnAutoFocusComplete(bool aAutoFocusSucceeded) { } > virtual void OnAutoFocusMoving(bool aIsMoving) { } >- virtual void OnTakePictureComplete(uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) { } >+ virtual void OnTakePictureComplete(const uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) { } > virtual void OnFacesDetected(const nsTArray<ICameraControl::Face>& aFaces) { } > > enum UserContext > { > kInStartCamera, > kInStopCamera, > kInAutoFocus, > kInStartFaceDetection, >diff --git a/dom/camera/DOMCameraControlListener.cpp b/dom/camera/DOMCameraControlListener.cpp >index 96f55ab..7a586c4 100644 >--- a/dom/camera/DOMCameraControlListener.cpp >+++ b/dom/camera/DOMCameraControlListener.cpp >@@ -336,28 +336,30 @@ DOMCameraControlListener::OnAutoFocusComplete(bool aAutoFocusSucceeded) > protected: > bool mAutoFocusSucceeded; > }; > > NS_DispatchToMainThread(new Callback(mDOMCameraControl, aAutoFocusSucceeded)); > } > > void >-DOMCameraControlListener::OnTakePictureComplete(uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) >+DOMCameraControlListener::OnTakePictureComplete(const uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) > { > class Callback : public DOMCallback > { > public: > Callback(nsMainThreadPtrHandle<nsISupports> aDOMCameraControl, >- uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) >+ const uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) > : DOMCallback(aDOMCameraControl) >- , mData(aData) > , mLength(aLength) > , mMimeType(aMimeType) >- { } >+ { >+ mData = (uint8_t*) malloc(aLength); >+ memcpy(mData, aData, aLength); >+ } > > void > RunCallback(nsDOMCameraControl* aDOMCameraControl) override > { > nsCOMPtr<nsIDOMBlob> picture = > File::CreateMemoryFile(mDOMCameraControl.get(), > static_cast<void*>(mData), > static_cast<uint64_t>(mLength), >diff --git a/dom/camera/DOMCameraControlListener.h b/dom/camera/DOMCameraControlListener.h >index a5c30c6..c1a1be8 100644 >--- a/dom/camera/DOMCameraControlListener.h >+++ b/dom/camera/DOMCameraControlListener.h >@@ -16,17 +16,17 @@ class CameraPreviewMediaStream; > class DOMCameraControlListener : public CameraControlListener > { > public: > DOMCameraControlListener(nsDOMCameraControl* aDOMCameraControl, CameraPreviewMediaStream* aStream); > > virtual void OnAutoFocusComplete(bool aAutoFocusSucceeded) override; > virtual void OnAutoFocusMoving(bool aIsMoving) override; > virtual void OnFacesDetected(const nsTArray<ICameraControl::Face>& aFaces) override; >- virtual void OnTakePictureComplete(uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) override; >+ virtual void OnTakePictureComplete(const uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) override; > > virtual void OnHardwareStateChange(HardwareState aState, nsresult aReason) override; > virtual void OnPreviewStateChange(PreviewState aState) override; > virtual void OnRecorderStateChange(RecorderState aState, int32_t aStatus, int32_t aTrackNum) override; > virtual void OnConfigurationChange(const CameraListenerConfiguration& aConfiguration) override; > virtual void OnShutter() override; > virtual void OnRateLimitPreview(bool aLimit) override; > virtual bool OnNewPreviewFrame(layers::Image* aImage, uint32_t aWidth, uint32_t aHeight) override; >diff --git a/dom/camera/GonkCameraControl.cpp b/dom/camera/GonkCameraControl.cpp >index 3e1eee7..41a15e5 100644 >--- a/dom/camera/GonkCameraControl.cpp >+++ b/dom/camera/GonkCameraControl.cpp >@@ -1529,24 +1529,20 @@ nsGonkCameraControl::OnFacesDetected(camera_frame_metadata_t* aMetaData) > CameraControlImpl::OnFacesDetected(faces); > } > > void > nsGonkCameraControl::OnTakePictureComplete(uint8_t* aData, uint32_t aLength) > { > ReentrantMonitorAutoEnter mon(mReentrantMonitor); > >- uint8_t* data = new uint8_t[aLength]; >- >- memcpy(data, aData, aLength); >- > nsString s(NS_LITERAL_STRING("image/")); > s.Append(mFileFormat); > DOM_CAMERA_LOGI("Got picture, type '%s', %u bytes\n", NS_ConvertUTF16toUTF8(s).get(), aLength); >- OnTakePictureComplete(data, aLength, s); >+ OnTakePictureComplete(aData, aLength, s); > > if (mResumePreviewAfterTakingPicture) { > nsresult rv = StartPreview(); > if (NS_FAILED(rv)) { > DOM_CAMERA_LOGE("Failed to restart camera preview (%x)\n", rv); > OnPreviewStateChange(CameraControlListener::kPreviewStopped); > } > } >diff --git a/dom/media/webrtc/MediaEngineGonkVideoSource.cpp b/dom/media/webrtc/MediaEngineGonkVideoSource.cpp >index 3243807..141e60b 100644 >--- a/dom/media/webrtc/MediaEngineGonkVideoSource.cpp >+++ b/dom/media/webrtc/MediaEngineGonkVideoSource.cpp >@@ -585,28 +585,28 @@ MediaEngineGonkVideoSource::OnUserError(UserContext aContext, nsresult aError) > MonitorAutoLock lock(mMonitor); > if (mPhotoCallbacks.Length()) { > NS_DispatchToMainThread(new TakePhotoError(mPhotoCallbacks, aError)); > } > } > } > > void >-MediaEngineGonkVideoSource::OnTakePictureComplete(uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) >+MediaEngineGonkVideoSource::OnTakePictureComplete(const uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) > { > // It needs to start preview because Gonk camera will stop preview while > // taking picture. > mCameraControl->StartPreview(); > > // Create a main thread runnable to generate a blob and call all current queued > // PhotoCallbacks. > class GenerateBlobRunnable : public nsRunnable { > public: > GenerateBlobRunnable(nsTArray<nsRefPtr<PhotoCallback>>& aCallbacks, >- uint8_t* aData, >+ const uint8_t* aData, > uint32_t aLength, > const nsAString& aMimeType) > : mPhotoDataLength(aLength) > { > mCallbacks.SwapElements(aCallbacks); > mPhotoData = (uint8_t*) malloc(aLength); > memcpy(mPhotoData, aData, mPhotoDataLength); > mMimeType = aMimeType; >diff --git a/dom/media/webrtc/MediaEngineGonkVideoSource.h b/dom/media/webrtc/MediaEngineGonkVideoSource.h >index 12d0372..b072f19 100644 >--- a/dom/media/webrtc/MediaEngineGonkVideoSource.h >+++ b/dom/media/webrtc/MediaEngineGonkVideoSource.h >@@ -69,17 +69,17 @@ public: > SourceMediaStream* aSource, > TrackID aId, > StreamTime aDesiredTime) override; > > void OnHardwareStateChange(HardwareState aState, nsresult aReason) override; > void GetRotation(); > bool OnNewPreviewFrame(layers::Image* aImage, uint32_t aWidth, uint32_t aHeight) override; > void OnUserError(UserContext aContext, nsresult aError) override; >- void OnTakePictureComplete(uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) override; >+ void OnTakePictureComplete(const uint8_t* aData, uint32_t aLength, const nsAString& aMimeType) override; > > void AllocImpl(); > void DeallocImpl(); > void StartImpl(webrtc::CaptureCapability aCapability); > void StopImpl(); > uint32_t ConvertPixelFormatToFOURCC(int aFormat); > void RotateImage(layers::Image* aImage, uint32_t aWidth, uint32_t aHeight); > void Notify(const mozilla::hal::ScreenConfiguration& aConfiguration);

Comment on attachment 8597733 [details] [diff] [review] bug1129051.patch, V2 Review of attachment 8597733 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/camera/GonkCameraControl.cpp @@ +1537,4 @@ > nsString s(NS_LITERAL_STRING("image/")); > s.Append(mFileFormat); > DOM_CAMERA_LOGI("Got picture, type '%s', %u bytes\n", NS_ConvertUTF16toUTF8(s).get(), aLength); > + OnTakePictureComplete(aData, aLength, s); I just realized this is missing a change in CameraControlImpl.h and CameraControlImpl.cpp to add const to CameraControlImpl::OnTakePictureComplete (called here). That should generate a compiler warning (apparently non-fatal, that's weird).

Comment on attachment 8597733 [details] [diff] [review] bug1129051.patch, V2 Review of attachment 8597733 [details] [diff] [review]: ----------------------------------------------------------------- you're correct. I will update the patch. Thanks,

From: Roger Yang <royang51@gmail.com> Bug 1129051 - Fix Camera Control Listener double free. Fix webrtc memory leak r=royang51

Comment on attachment 8598364 [details] [diff] [review] bug1129051.patch, V3 Review of attachment 8598364 [details] [diff] [review]: ----------------------------------------------------------------- Much better, thanks. I'll try landing this again today.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3915f3a18669