[Chromoting] Move DefaultAudioDeviceChangeDetector out of AudioCapturerWin

remoting/host/win/default_audio_device_change_detector.h

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef REMOTING_HOST_WIN_DEFAULT_AUDIO_DEVICE_CHANGE_DETECTOR_H_
+#define REMOTING_HOST_WIN_DEFAULT_AUDIO_DEVICE_CHANGE_DETECTOR_H_
+
+#include <mmdeviceapi.h>
+
+#include "base/synchronization/lock.h"
+
+namespace remoting {
+
+// An IMMNotificationClient implementation to detect the change of the default
+// audio output device on the system.
+// This class does not use the default referring memory management method

[Sergey Ulanov, 2017/04/13 01:30:12]

s/default referring memory management method/reference counting/
Or say  "This class doesn't increment ref-count for the IMMDeviceEnumerator". 

But given the last sentence in this comment I don't think we need to mention
ref-counting at all.

Do we need this restriction? If you update this class to use ScopedComPtr for
enumerator_ then the capturer wouldn't have to keep mm_device_enumerator_, and
this would make destruction cleaner and less error-prone.

[Hzj_jie, 2017/04/13 17:26:16]

I have not noticed the mm_device_enumerator_ is for this class only. I will use
ScopedComPtr instead of raw pointer.

But here we have two issues, the one is the ref-count of IMMDeviceEnumerator,
the other is the ref-count of DefaultAudioDeviceChangeDetector itself. MSDN
suggested to delete this if ref-counter reaches zero in Release() function, but
this class does not implement this functionality because of the limited user
scenario.
My comment is confusing, I will update it anyway.

+// provided by IUnknown: the
+// IMMDeviceEnumerator::RegisterEndpointNotificationCallback() will be called in
+// the constructor, and
+// IMMDeviceEnumerator::UnregisterEndpointNotificationCallback() will be called

[Sergey Ulanov, 2017/04/13 01:30:12]

s/will be/is/ in the two places above.

[Hzj_jie, 2017/04/13 17:26:16]

Done.

+// in the destructor. Consumers should ensure the input IMMDeviceEnumerator
+// outlives this instance.
+class DefaultAudioDeviceChangeDetector final : public IMMNotificationClient {
+ public:
+  DefaultAudioDeviceChangeDetector(IMMDeviceEnumerator* enumerator);
+  ~DefaultAudioDeviceChangeDetector();
+
+  HRESULT __stdcall OnDefaultDeviceChanged(EDataFlow flow,

[Sergey Ulanov, 2017/04/13 01:30:12]

Move IMMNotificationClient implementation to the private section?

[Hzj_jie, 2017/04/13 17:26:16]

Done.

+                                           ERole role,
+                                           LPCWSTR pwstrDefaultDevice) override;
+
+  HRESULT __stdcall QueryInterface(REFIID iid, void** object) override;
+
+  bool GetAndReset();
+
+  // No-ops overrides.
+  HRESULT __stdcall OnDeviceAdded(LPCWSTR pwstrDeviceId) override {

[Sergey Ulanov, 2017/04/13 01:30:12]

Normally all virtual method overrides should be in the .cc file.

[Hzj_jie, 2017/04/13 17:26:16]

Done.

+    return S_OK;
+  }
+  HRESULT __stdcall OnDeviceRemoved(LPCWSTR pwstrDeviceId) override {
+    return S_OK;
+  }
+  HRESULT __stdcall OnDeviceStateChanged(LPCWSTR pwstrDeviceId,
+                                         DWORD dwNewState) override {
+    return S_OK;
+  }
+  HRESULT __stdcall OnPropertyValueChanged(LPCWSTR pwstrDeviceId,
+                                           const PROPERTYKEY key) override {
+    return S_OK;
+  }
+  ULONG __stdcall AddRef() override { return 1; }
+  ULONG __stdcall Release() override { return 1; }
+
+ private:
+  IMMDeviceEnumerator* const enumerator_;
+  bool changed_ = false;
+  base::Lock lock_;
+};
+
+}  // namespace remoting
+
+#endif  // REMOTING_HOST_WIN_DEFAULT_AUDIO_DEVICE_CHANGE_DETECTOR_H_