Web MIDI: implement MIDIConnectionEvent

Web MIDI: implement MIDIConnectionEvent

This change implements MIDIConnectionEvent.

BUG=163795
TEST=none
R=crogers@google.com, haraken@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=149164

[Takashi Toyoshima, 2013-04-25 06:10:30]

Now, I'll add MIDIConnectionEvent.

Chris:
Please take a look on webmidi parts.

Haraken:
This change modify binding code base to make MIDIPort available for
[InitializedByEventConstructor]. Can you take a look?

[haraken, 2013-04-25 06:33:00]

https://codereview.chromium.org/14358032/diff/2001/LayoutTests/fast/events/co...
File LayoutTests/fast/events/constructors/midi-connection-event-constructor.html
(right):

https://codereview.chromium.org/14358032/diff/2001/LayoutTests/fast/events/co...
LayoutTests/fast/events/constructors/midi-connection-event-constructor.html:24:
// FIXME: Add a test case to pass port once MIDIPort is available.
Why can't you add the test in this patch?

https://codereview.chromium.org/14358032/diff/2001/Source/bindings/v8/Diction...
File Source/bindings/v8/Dictionary.cpp (right):

https://codereview.chromium.org/14358032/diff/2001/Source/bindings/v8/Diction...
Source/bindings/v8/Dictionary.cpp:342: if (V8MediaKeyError::HasInstance(v8Value,
m_isolate, worldType(m_isolate)))
I don't think this works. V8MediaKeyError => V8MIDIPort ?

https://codereview.chromium.org/14358032/diff/2001/Source/modules/webmidi/MID...
File Source/modules/webmidi/MIDIConnectionEvent.h (right):

https://codereview.chromium.org/14358032/diff/2001/Source/modules/webmidi/MID...
Source/modules/webmidi/MIDIConnectionEvent.h:40: MIDIConnectionEventInit()
You need to initialize port to 0.

https://codereview.chromium.org/14358032/diff/2001/Source/modules/webmidi/MID...
Source/modules/webmidi/MIDIConnectionEvent.h:71: // Event:
Nit: Remove the comment.

https://codereview.chromium.org/14358032/diff/2001/Source/modules/webmidi/MID...
Source/modules/webmidi/MIDIConnectionEvent.h:86: MIDIConnectionEvent(const
AtomicString& type, PassRefPtr<MIDIPort> port)
Do you really need all of MIDIConnectionEvent(),
MIDIConnectionEvent(AtomicString) and MIDIConnectionEvent(AtomicString,
PassRefPtr<MIDIPort>) ?

[Takashi Toyoshima, 2013-04-25 07:20:27]

https://codereview.chromium.org/14358032/diff/2001/LayoutTests/fast/events/co...
File LayoutTests/fast/events/constructors/midi-connection-event-constructor.html
(right):

https://codereview.chromium.org/14358032/diff/2001/LayoutTests/fast/events/co...
LayoutTests/fast/events/constructors/midi-connection-event-constructor.html:24:
// FIXME: Add a test case to pass port once MIDIPort is available.
Currently MIDIPort is implemented, but no way is exposed to construct or obtain
a MIDIPort object.

https://codereview.chromium.org/14358032/diff/2001/Source/bindings/v8/Diction...
File Source/bindings/v8/Dictionary.cpp (right):

https://codereview.chromium.org/14358032/diff/2001/Source/bindings/v8/Diction...
Source/bindings/v8/Dictionary.cpp:342: if (V8MediaKeyError::HasInstance(v8Value,
m_isolate, worldType(m_isolate)))
Oops. Sorry, I missed to rename it.

https://codereview.chromium.org/14358032/diff/2001/Source/modules/webmidi/MID...
File Source/modules/webmidi/MIDIConnectionEvent.h (right):

https://codereview.chromium.org/14358032/diff/2001/Source/modules/webmidi/MID...
Source/modules/webmidi/MIDIConnectionEvent.h:40: MIDIConnectionEventInit()
On 2013/04/25 06:33:00, haraken wrote:
> You need to initialize port to 0.

Done.

https://codereview.chromium.org/14358032/diff/2001/Source/modules/webmidi/MID...
Source/modules/webmidi/MIDIConnectionEvent.h:71: // Event:
On 2013/04/25 06:33:00, haraken wrote:
> Nit: Remove the comment.

Done.

https://codereview.chromium.org/14358032/diff/2001/Source/modules/webmidi/MID...
Source/modules/webmidi/MIDIConnectionEvent.h:86: MIDIConnectionEvent(const
AtomicString& type, PassRefPtr<MIDIPort> port)
First one is used by auto-generated code.
The second one is used for testing in my local branch but I can omit this.
The third one will be used in the real implementation.

[haraken, 2013-04-25 07:36:47]

https://codereview.chromium.org/14358032/diff/2001/LayoutTests/fast/events/co...
File LayoutTests/fast/events/constructors/midi-connection-event-constructor.html
(right):

https://codereview.chromium.org/14358032/diff/2001/LayoutTests/fast/events/co...
LayoutTests/fast/events/constructors/midi-connection-event-constructor.html:24:
// FIXME: Add a test case to pass port once MIDIPort is available.
Why isn't it exposed? You've written Dictionary::get() for MIDIPort objects and
MIDIConnectionEvent exposes a port attribute. Can't you get a MIDIPort via the
port attribute?

[Takashi Toyoshima, 2013-04-25 07:47:50]

https://codereview.chromium.org/14358032/diff/2001/LayoutTests/fast/events/co...
File LayoutTests/fast/events/constructors/midi-connection-event-constructor.html
(right):

https://codereview.chromium.org/14358032/diff/2001/LayoutTests/fast/events/co...
LayoutTests/fast/events/constructors/midi-connection-event-constructor.html:24:
// FIXME: Add a test case to pass port once MIDIPort is available.
As line 14 is, it's always null for now :(
MIDIPort object is binded to real MIDI device ports. So, we can not use it until
I write a pseudo something for testing in chromium side. Sorry, I didn't have a
concrete plan of pseudo platform for testing yet.

[haraken, 2013-04-25 07:51:05]

LGTM for all code.

You need to get LGTM from modules/ OWNERs.

[Takashi Toyoshima, 2013-04-25 08:03:36]

Thanks!
I'll wait for Chris on modules/webmidi.

[Chris Rogers, 2013-04-25 22:35:42]

lgtm

[abarth-chromium, 2013-04-26 04:26:51]

LGTM3

https://codereview.chromium.org/14358032/diff/5003/LayoutTests/fast/events/co...
File LayoutTests/fast/events/constructors/midi-connection-event-constructor.html
(right):

https://codereview.chromium.org/14358032/diff/5003/LayoutTests/fast/events/co...
LayoutTests/fast/events/constructors/midi-connection-event-constructor.html:14:
shouldBe("new MIDIConnectionEvent('eventType').port", "null");
Thank you for adding tests.  I was worried that your earlier CLs didn't have
tests, but I know it takes a bit before there's enough around to test.

https://codereview.chromium.org/14358032/diff/5003/Source/bindings/v8/Diction...
File Source/bindings/v8/Dictionary.h (right):

https://codereview.chromium.org/14358032/diff/5003/Source/bindings/v8/Diction...
Source/bindings/v8/Dictionary.h:99: bool get(const String&,
RefPtr<VoidCallback>&) const;
At some point we need to autogenerate this madness so this file doesn't turn
into a list of every feature in the engine.

[Takashi Toyoshima, 2013-04-26 05:15:58]

https://codereview.chromium.org/14358032/diff/5003/LayoutTests/fast/events/co...
File LayoutTests/fast/events/constructors/midi-connection-event-constructor.html
(right):

https://codereview.chromium.org/14358032/diff/5003/LayoutTests/fast/events/co...
LayoutTests/fast/events/constructors/midi-connection-event-constructor.html:14:
shouldBe("new MIDIConnectionEvent('eventType').port", "null");
Thank you for advice.
I'm landing from independent interfaces and some interfaces can not be
accessible from JavaScript for now. I'll add test once they can be accessible
via other interfaces. Fortunately, these MIDI*Event class have constructors
which is useful for testing.

[Takashi Toyoshima, 2013-04-26 05:27:22]

Committed patchset #4 manually as r149164 (presubmit successful).