Remove RuntimeObject.lib dependency in Windows 10 Web MIDI backend

Remove RuntimeObject.lib dependency in Windows 10 Web MIDI backend

Although required by WRL wrappers, linking with RuntimeObject.lib introduces
dependency to certain libraries that may not be available on Windows 7. As we
will eventually supply both WinMM and WinRT implementations for Web MIDI in the
same binary, here we eliminate the dependency by loading required library
functions at runtime with Win32 APIs and remove usages of WRL wrappers.

BUG=642602
R=toyoshim@chromium.org

Committed: https://crrev.com/7086ca44644de7beee7c5b0d5b6170b816575943
Cr-Commit-Position: refs/heads/master@{#415915}

[Shao-Chuan Lee, 2016-08-31 09:52:13]

This is easier than I thought... please review.

https://codereview.chromium.org/2301513002/diff/1/media/midi/BUILD.gn
File media/midi/BUILD.gn (right):

https://codereview.chromium.org/2301513002/diff/1/media/midi/BUILD.gn#newcode146
media/midi/BUILD.gn:146: deps += [ "//device/usb" ]
Moved here since WinRT implementation doesn't require this.

https://codereview.chromium.org/2301513002/diff/1/media/midi/midi_manager_win...
File media/midi/midi_manager_winrt.cc (right):

https://codereview.chromium.org/2301513002/diff/1/media/midi/midi_manager_win...
media/midi/midi_manager_winrt.cc:162: HRESULT hr =
class_id_hstring.Set(runtime_class_id);
Here we create new HSTRINGs with WindowsCreateString() instead of
WindowsCreateStringReference(), overhead should be small and I can reuse
HStringWrapper to make code cleaner.

[Takashi Toyoshima, 2016-08-31 10:10:29]

https://codereview.chromium.org/2301513002/diff/1/media/midi/BUILD.gn
File media/midi/BUILD.gn (right):

https://codereview.chromium.org/2301513002/diff/1/media/midi/BUILD.gn#newcode146
media/midi/BUILD.gn:146: deps += [ "//device/usb" ]
On 2016/08/31 09:52:13, Shao-Chuan Lee wrote:
> Moved here since WinRT implementation doesn't require this.

Acknowledged.

https://codereview.chromium.org/2301513002/diff/1/media/midi/midi_manager_win...
File media/midi/midi_manager_winrt.cc (right):

https://codereview.chromium.org/2301513002/diff/1/media/midi/midi_manager_win...
media/midi/midi_manager_winrt.cc:129: // Minimalist HSTRING wrapper class to
properly maintain HSTRING lifetime.
Can you use base/scoped_generic.h infra to implement this?

[Shao-Chuan Lee, 2016-09-01 02:53:09]

Patchset #2 (id:20001) has been deleted

[Shao-Chuan Lee, 2016-09-01 02:55:44]

Patchset #2 (id:40001) has been deleted

[Shao-Chuan Lee, 2016-09-01 03:06:44]

https://codereview.chromium.org/2301513002/diff/1/media/midi/midi_manager_win...
File media/midi/midi_manager_winrt.cc (right):

https://codereview.chromium.org/2301513002/diff/1/media/midi/midi_manager_win...
media/midi/midi_manager_winrt.cc:129: // Minimalist HSTRING wrapper class to
properly maintain HSTRING lifetime.
On 2016/08/31 10:10:29, toyoshim wrote:
> Can you use base/scoped_generic.h infra to implement this?

Done.

https://codereview.chromium.org/2301513002/diff/60001/media/midi/midi_manager...
File media/midi/midi_manager_winrt.cc (right):

https://codereview.chromium.org/2301513002/diff/60001/media/midi/midi_manager...
media/midi/midi_manager_winrt.cc:804: VLOG(1) << "Failed loading functions from
combase.dll: "
name consistency

[Takashi Toyoshima, 2016-09-01 05:57:04]

https://codereview.chromium.org/2301513002/diff/80001/media/midi/midi_manager...
File media/midi/midi_manager_winrt.cc (right):

https://codereview.chromium.org/2301513002/diff/80001/media/midi/midi_manager...
media/midi/midi_manager_winrt.cc:138: static HSTRING InvalidValue() { return
nullptr; }
I'm not sure, but could this be INVALID_HANDLE_VALUE?
You said nullptr means an empty string for HSTRING. I guess they have different
representation for invalid from empty.
Can you check documents?

https://codereview.chromium.org/2301513002/diff/80001/media/midi/midi_manager...
media/midi/midi_manager_winrt.cc:819: if
(!g_combase_functions.Get().LoadFunctions()) {
We may want to have this check on the com thread to reduce complexity.

[Shao-Chuan Lee, 2016-09-01 06:11:36]

https://codereview.chromium.org/2301513002/diff/80001/media/midi/midi_manager...
File media/midi/midi_manager_winrt.cc (right):

https://codereview.chromium.org/2301513002/diff/80001/media/midi/midi_manager...
media/midi/midi_manager_winrt.cc:138: static HSTRING InvalidValue() { return
nullptr; }
On 2016/09/01 05:57:04, toyoshim wrote:
> I'm not sure, but could this be INVALID_HANDLE_VALUE?
> You said nullptr means an empty string for HSTRING. I guess they have
different
> representation for invalid from empty.
> Can you check documents?

MSDN: "string [out]: A pointer to the newly created HSTRING, or NULL if an error
occurs."
https://msdn.microsoft.com/en-us/library/windows/desktop/br224630(v=vs.85).aspx

Actually if an empty string is supplied to WindowsCreateString() it also outputs
nullptr, but in our use cases we expect strings to be non-empty, so I think it's
OK to consider empty strings as invalid.

https://codereview.chromium.org/2301513002/diff/80001/media/midi/midi_manager...
media/midi/midi_manager_winrt.cc:819: if
(!g_combase_functions.Get().LoadFunctions()) {
On 2016/09/01 05:57:04, toyoshim wrote:
> We may want to have this check on the com thread to reduce complexity.

Done.

https://codereview.chromium.org/2301513002/diff/100001/media/midi/midi_manage...
File media/midi/midi_manager_winrt.cc (right):

https://codereview.chromium.org/2301513002/diff/100001/media/midi/midi_manage...
media/midi/midi_manager_winrt.cc:881: if (port_manager_in_) {
Additional checks in case InitializeOnComThread() fails early.

https://codereview.chromium.org/2301513002/diff/100001/media/midi/midi_manage...
media/midi/midi_manager_winrt.cc:886: if (port_manager_out_) {
ditto

[Takashi Toyoshima, 2016-09-01 07:07:25]

lgtm

https://codereview.chromium.org/2301513002/diff/80001/media/midi/midi_manager...
File media/midi/midi_manager_winrt.cc (right):

https://codereview.chromium.org/2301513002/diff/80001/media/midi/midi_manager...
media/midi/midi_manager_winrt.cc:138: static HSTRING InvalidValue() { return
nullptr; }
Generally speaking, H* types of Windows use INVALID_HANDLE_VALUE (-1) to
represent invalid value, for instance to represent uninitialized handle in
general.

OK, let's go with this line as is.

[Shao-Chuan Lee, 2016-09-01 07:10:19]

The CQ bit was checked by shaochuan@chromium.org

[commit-bot: I haz the power, 2016-09-01 07:10:31]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Shao-Chuan Lee, 2016-09-01 07:15:49]

On 2016/09/01 07:07:25, toyoshim wrote:
> lgtm
> 
>
https://codereview.chromium.org/2301513002/diff/80001/media/midi/midi_manager...
> File media/midi/midi_manager_winrt.cc (right):
> 
>
https://codereview.chromium.org/2301513002/diff/80001/media/midi/midi_manager...
> media/midi/midi_manager_winrt.cc:138: static HSTRING InvalidValue() { return
> nullptr; }
> Generally speaking, H* types of Windows use INVALID_HANDLE_VALUE (-1) to
> represent invalid value, for instance to represent uninitialized handle in
> general.
> 
> OK, let's go with this line as is.

HSTRING is introduced with WinRT since Windows 8, and is not typedef'd from
HANDLE (void*) like other handle types, maybe it's not following WinAPI
conventions. I think it should be OK following the documentation.

Commiting.

[commit-bot: I haz the power, 2016-09-01 08:59:23]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2016-09-01 09:01:10]

Description was changed from

==========
Remove RuntimeObject.lib dependency in Windows 10 Web MIDI backend

Although required by WRL wrappers, linking with RuntimeObject.lib introduces
dependency to certain libraries that may not be available on Windows 7. As we
will eventually supply both WinMM and WinRT implementations for Web MIDI in the
same binary, here we eliminate the dependency by loading required library
functions at runtime with Win32 APIs and remove usages of WRL wrappers.

BUG=642602
R=toyoshim@chromium.org
==========

to

==========
Remove RuntimeObject.lib dependency in Windows 10 Web MIDI backend

Although required by WRL wrappers, linking with RuntimeObject.lib introduces
dependency to certain libraries that may not be available on Windows 7. As we
will eventually supply both WinMM and WinRT implementations for Web MIDI in the
same binary, here we eliminate the dependency by loading required library
functions at runtime with Win32 APIs and remove usages of WRL wrappers.

BUG=642602
R=toyoshim@chromium.org

Committed: https://crrev.com/7086ca44644de7beee7c5b0d5b6170b816575943
Cr-Commit-Position: refs/heads/master@{#415915}
==========

[commit-bot: I haz the power, 2016-09-01 09:01:11]

Patchset 4 (id:??) landed as
https://crrev.com/7086ca44644de7beee7c5b0d5b6170b816575943
Cr-Commit-Position: refs/heads/master@{#415915}