Commit 2eec73ab authored by Christian Larson's avatar Christian Larson Committed by Commit Bot

Fix Crash Report - media::VideoCaptureDeviceMFWin::OnError

Updated MFVideoCallback to have a shutdown method to invalidate the
pointer to the observer_.  Shutdown is needed because MFVideoCallback is
a refcounted class and can live longer than the raw pointer to
observer_.  Added test to validate the crash before the fix and that the
code update fixes the issue.

Bug: 1099934
Change-Id: I1a80914f720638a4fda2d2155b547ec12bafd08a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2275493Reviewed-by: default avatarGuido Urdaneta <guidou@chromium.org>
Reviewed-by: default avatarBruce Dawson <brucedawson@chromium.org>
Commit-Queue: Christian Larson <chrila@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#784506}
parent 7e460a8b
...@@ -483,11 +483,19 @@ class MFVideoCallback final ...@@ -483,11 +483,19 @@ class MFVideoCallback final
} }
IFACEMETHODIMP OnEvent(IMFMediaEvent* media_event) override { IFACEMETHODIMP OnEvent(IMFMediaEvent* media_event) override {
base::AutoLock lock(lock_);
if (!observer_) {
return S_OK;
}
observer_->OnEvent(media_event); observer_->OnEvent(media_event);
return S_OK; return S_OK;
} }
IFACEMETHODIMP OnSample(IMFSample* sample) override { IFACEMETHODIMP OnSample(IMFSample* sample) override {
base::AutoLock lock(lock_);
if (!observer_) {
return S_OK;
}
if (!sample) { if (!sample) {
observer_->OnFrameDropped( observer_->OnFrameDropped(
VideoCaptureFrameDropReason::kWinMediaFoundationReceivedSampleIsNull); VideoCaptureFrameDropReason::kWinMediaFoundationReceivedSampleIsNull);
...@@ -526,10 +534,18 @@ class MFVideoCallback final ...@@ -526,10 +534,18 @@ class MFVideoCallback final
return S_OK; return S_OK;
} }
void Shutdown() {
base::AutoLock lock(lock_);
observer_ = nullptr;
}
private: private:
friend class base::RefCountedThreadSafe<MFVideoCallback>; friend class base::RefCountedThreadSafe<MFVideoCallback>;
~MFVideoCallback() {} ~MFVideoCallback() {}
VideoCaptureDeviceMFWin* observer_;
// Protects access to |observer_|.
base::Lock lock_;
VideoCaptureDeviceMFWin* observer_ GUARDED_BY(lock_);
}; };
// static // static
...@@ -736,6 +752,9 @@ VideoCaptureDeviceMFWin::~VideoCaptureDeviceMFWin() { ...@@ -736,6 +752,9 @@ VideoCaptureDeviceMFWin::~VideoCaptureDeviceMFWin() {
: false); : false);
} }
} }
if (video_callback_) {
video_callback_->Shutdown();
}
} }
bool VideoCaptureDeviceMFWin::Init() { bool VideoCaptureDeviceMFWin::Init() {
......
...@@ -1265,6 +1265,37 @@ TEST_F(VideoCaptureDeviceMFWinTest, CallClientOnFireCaptureEngineInitEarly) { ...@@ -1265,6 +1265,37 @@ TEST_F(VideoCaptureDeviceMFWinTest, CallClientOnFireCaptureEngineInitEarly) {
EXPECT_TRUE(device->Init()); EXPECT_TRUE(device->Init());
} }
// Send MFVideoCallback::OnEvent when VideoCaptureDeviceMFWin has been destroyed
TEST_F(VideoCaptureDeviceMFWinTest,
SendMFVideoCallbackAfterVideoCaptureDeviceMFWinDestructor) {
if (ShouldSkipTest())
return;
VideoCaptureDeviceDescriptor descriptor = VideoCaptureDeviceDescriptor();
Microsoft::WRL::ComPtr<MockMFMediaSource> media_source =
new MockMFMediaSource();
Microsoft::WRL::ComPtr<MockMFCaptureEngine> engine =
new MockMFCaptureEngine();
std::unique_ptr<VideoCaptureDeviceMFWin> device =
std::make_unique<VideoCaptureDeviceMFWin>(descriptor, media_source,
engine);
EXPECT_CALL(*(engine.Get()), OnInitEventGuid).WillOnce([]() {
return MF_CAPTURE_ENGINE_INITIALIZED;
});
EXPECT_CALL(*(engine.Get()), OnCorrectInitializeQueued());
EXPECT_TRUE(device->Init());
// Force ~VideoCaptureDeviceMFWin() which will invalidate
// MFVideoCallback::observer_
device.reset();
// Send event to MFVideoCallback::OnEvent
engine->FireCaptureEvent(MF_CAPTURE_ENGINE_ERROR,
MF_E_VIDEO_RECORDING_DEVICE_INVALIDATED);
}
// Allocates device with flaky methods failing with MF_E_INVALIDREQUEST and // Allocates device with flaky methods failing with MF_E_INVALIDREQUEST and
// expects the device to retry and start correctly // expects the device to retry and start correctly
TEST_F(VideoCaptureDeviceMFWinTest, AllocateAndStartWithFlakyInvalidRequest) { TEST_F(VideoCaptureDeviceMFWinTest, AllocateAndStartWithFlakyInvalidRequest) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment