Add wait logic to Init() on MF_CAPTURE_ENGINE_INITIALIZED event.

Before completing the VideoCaptureDeviceMFWin::Init() function we
should wait on MF_CAPTURE_ENGINE_INITIALIZED.  The Capture Source is
not fully constructed until the MF_CAPTURE_ENGINE_INITIALIZED event
fires and calling methods before this time can cause unstable behavior.

Bug: 1076730
Change-Id: I239077a4d9a599643b9e2c4ed0e26219a2dbb58f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2238879Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: Christian Larson <chrila@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#780498}

media/capture/video/win/video_capture_device_mf_win.cc

@@ -683,7 +683,12 @@ VideoCaptureDeviceMFWin::VideoCaptureDeviceMFWin(
       has_sent_on_started_to_client_(false),
       exposure_mode_manual_(false),
       focus_mode_manual_(false),
-      white_balance_mode_manual_(false) {
+      white_balance_mode_manual_(false),
+      capture_initialize_(base::WaitableEvent::ResetPolicy::AUTOMATIC,
+                          base::WaitableEvent::InitialState::NOT_SIGNALED),
+      // We never want to reset |capture_error_|.
+      capture_error_(base::WaitableEvent::ResetPolicy::MANUAL,
+                     base::WaitableEvent::InitialState::NOT_SIGNALED) {
   DETACH_FROM_SEQUENCE(sequence_checker_);
 }
 
@@ -731,6 +736,13 @@ bool VideoCaptureDeviceMFWin::Init() {
     LogError(FROM_HERE, hr);
     return false;
   }
+
+  hr = WaitOnCaptureEvent(MF_CAPTURE_ENGINE_INITIALIZED);
+  if (FAILED(hr)) {
+    LogError(FROM_HERE, hr);
+    return false;
+  }
+
   is_initialized_ = true;
   return true;
 }
@@ -1320,7 +1332,21 @@ void VideoCaptureDeviceMFWin::OnEvent(IMFMediaEvent* media_event) {
   base::AutoLock lock(lock_);
 
   HRESULT hr;
+  GUID capture_event_guid = GUID_NULL;
+
   media_event->GetStatus(&hr);
+  media_event->GetExtendedType(&capture_event_guid);
+  // TODO(http://crbug.com/1093521): Add cases for Start
+  // MF_CAPTURE_ENGINE_PREVIEW_STARTED and MF_CAPTURE_ENGINE_PREVIEW_STOPPED
+  // When MF_CAPTURE_ENGINE_ERROR is returned the captureengine object is no
+  // longer valid.
+  if (capture_event_guid == MF_CAPTURE_ENGINE_ERROR || FAILED(hr)) {
+    capture_error_.Signal();
+    // There should always be a valid error
+    hr = SUCCEEDED(hr) ? E_UNEXPECTED : hr;
+  } else if (capture_event_guid == MF_CAPTURE_ENGINE_INITIALIZED) {
+    capture_initialize_.Signal();
+  }
 
   if (FAILED(hr))
     OnError(VideoCaptureError::kWinMediaFoundationGetMediaEventStatusFailed,
@@ -1350,4 +1376,35 @@ void VideoCaptureDeviceMFWin::SendOnStartedIfNotYetSent() {
   client_->OnStarted();
 }
 
+HRESULT VideoCaptureDeviceMFWin::WaitOnCaptureEvent(GUID capture_event_guid) {
+  HRESULT hr = S_OK;
+  HANDLE events[] = {nullptr, capture_error_.handle()};
+
+  // TODO(http://crbug.com/1093521): Add cases for Start
+  // MF_CAPTURE_ENGINE_PREVIEW_STARTED and MF_CAPTURE_ENGINE_PREVIEW_STOPPED
+  if (capture_event_guid == MF_CAPTURE_ENGINE_INITIALIZED) {
+    events[0] = capture_initialize_.handle();
+  } else {
+    // no registered event handle for the event requested
+    hr = E_NOTIMPL;
+    LogError(FROM_HERE, hr);
+    return hr;
+  }
+
+  DWORD wait_result =
+      ::WaitForMultipleObjects(base::size(events), events, FALSE, INFINITE);
+  switch (wait_result) {
+    case WAIT_OBJECT_0:
+      break;
+    case WAIT_FAILED:
+      hr = HRESULT_FROM_WIN32(::GetLastError());
+      LogError(FROM_HERE, hr);
+      break;
+    default:
+      hr = E_UNEXPECTED;
+      LogError(FROM_HERE, hr);
+      break;
+  }
+  return hr;
+}
 }  // namespace media

media/capture/video/win/video_capture_device_mf_win.h

@@ -117,6 +117,7 @@ class CAPTURE_EXPORT VideoCaptureDeviceMFWin : public VideoCaptureDevice {
                const base::Location& from_here,
                const char* message);
   void SendOnStartedIfNotYetSent();
+  HRESULT WaitOnCaptureEvent(GUID capture_event_guid);
 
   VideoFacingMode facing_mode_;
   CreateMFPhotoCallbackCB create_mf_photo_callback_;
@@ -145,6 +146,8 @@ class CAPTURE_EXPORT VideoCaptureDeviceMFWin : public VideoCaptureDevice {
   bool focus_mode_manual_;
   bool white_balance_mode_manual_;
   base::queue<TakePhotoCallback> video_stream_take_photo_callbacks_;
+  base::WaitableEvent capture_initialize_;
+  base::WaitableEvent capture_error_;
 
   SEQUENCE_CHECKER(sequence_checker_);
 

media/capture/video/win/video_capture_device_mf_win_unittest.cc

@@ -8,6 +8,8 @@
 #include <wincodec.h>
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/test/task_environment.h"
 #include "media/capture/video/win/sink_filter_win.h"
 #include "media/capture/video/win/video_capture_device_factory_win.h"
 #include "media/capture/video/win/video_capture_device_mf_win.h"
@@ -413,11 +415,34 @@ class MockMFCaptureEngine
     EXPECT_TRUE(pAttributes);
     EXPECT_TRUE(pVideoSource);
     event_callback = pEventCallback;
-    OnCorrectInitialize();
+    OnCorrectInitializeQueued();
+
+    ON_CALL(*this, OnInitStatus).WillByDefault(Return(S_OK));
+    ON_CALL(*this, OnInitEventGuid)
+        .WillByDefault(Return(MF_CAPTURE_ENGINE_INITIALIZED));
+    // HW Cameras usually add about 500ms latency on init
+    ON_CALL(*this, InitEventDelay)
+        .WillByDefault(Return(base::TimeDelta::FromMilliseconds(500)));
+
+    base::TimeDelta event_delay = InitEventDelay();
+
+    base::ThreadPool::PostDelayedTask(
+        FROM_HERE,
+        base::BindOnce(&MockMFCaptureEngine::FireCaptureEvent, this,
+                       OnInitEventGuid(), OnInitStatus()),
+        event_delay);
+    // if zero is passed ensure event fires before wait starts
+    if (event_delay == base::TimeDelta::FromMilliseconds(0)) {
+      base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(200));
+    }
+
     return S_OK;
   }
 
-  MOCK_METHOD0(OnCorrectInitialize, void(void));
+  MOCK_METHOD0(OnCorrectInitializeQueued, void(void));
+  MOCK_METHOD0(OnInitEventGuid, GUID(void));
+  MOCK_METHOD0(OnInitStatus, HRESULT(void));
+  MOCK_METHOD0(InitEventDelay, base::TimeDelta(void));
 
   IFACEMETHODIMP StartPreview(void) override {
     OnStartPreview();
@@ -456,8 +481,14 @@ class MockMFCaptureEngine
   }
   MOCK_METHOD0(DoGetSource, IMFCaptureSource*());
 
+  void FireCaptureEvent(GUID event, HRESULT hrStatus) {
+    ComPtr<IMFMediaEvent> captureEvent;
+    MFCreateMediaEvent(MEExtendedType, event, hrStatus, nullptr, &captureEvent);
+    if (event_callback) {
+      event_callback->OnEvent(captureEvent.Get());
+    }
+  }
   scoped_refptr<IMFCaptureEngineOnEventCallback> event_callback;
-
  private:
   friend class base::RefCountedThreadSafe<MockMFCaptureEngine>;
   virtual ~MockMFCaptureEngine() = default;
@@ -872,7 +903,7 @@ class VideoCaptureDeviceMFWinTest : public ::testing::Test {
     device_->set_max_retry_count_for_testing(3);
     device_->set_retry_delay_in_ms_for_testing(1);
 
-    EXPECT_CALL(*(engine_.Get()), OnCorrectInitialize());
+    EXPECT_CALL(*(engine_.Get()), OnCorrectInitializeQueued());
     EXPECT_TRUE(device_->Init());
     EXPECT_CALL(*(engine_.Get()), DoGetSource())
         .WillRepeatedly(Invoke([this]() {
@@ -1079,6 +1110,7 @@ class VideoCaptureDeviceMFWinTest : public ::testing::Test {
 
   scoped_refptr<MockMFCaptureSource> capture_source_;
   scoped_refptr<MockCapturePreviewSink> capture_preview_sink_;
+  base::test::TaskEnvironment task_environment_;
 
  private:
   const bool media_foundation_supported_;
@@ -1118,6 +1150,60 @@ TEST_F(VideoCaptureDeviceMFWinTest, CallClientOnErrorMediaEvent) {
   engine_->event_callback->OnEvent(media_event_error.get());
 }
 
+// Expects Init to fail due to OnError() event
+TEST_F(VideoCaptureDeviceMFWinTest, CallClientOnErrorDurringInit) {
+  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;
+  });
+  // E_ACCESSDENIED is thrown if application is denied access in settings UI
+  EXPECT_CALL(*(engine.Get()), OnInitStatus).WillOnce([]() {
+    return E_ACCESSDENIED;
+  });
+
+  EXPECT_CALL(*(engine.Get()), OnCorrectInitializeQueued());
+
+  EXPECT_FALSE(device->Init());
+}
+
+// Expects Init to succeed but MF_CAPTURE_ENGINE_INITIALIZED fired before
+// WaitOnCaptureEvent is called.
+TEST_F(VideoCaptureDeviceMFWinTest, CallClientOnFireCaptureEngineInitEarly) {
+  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()), InitEventDelay).WillOnce([]() {
+    return base::TimeDelta::FromMilliseconds(0);
+  });
+
+  EXPECT_CALL(*(engine.Get()), OnCorrectInitializeQueued());
+
+  EXPECT_TRUE(device->Init());
+}
+
 // Allocates device with flaky methods failing with MF_E_INVALIDREQUEST and
 // expects the device to retry and start correctly
 TEST_F(VideoCaptureDeviceMFWinTest, AllocateAndStartWithFlakyInvalidRequest) {