Web MIDI: implement MIDIInput

Web MIDI: implement MIDIInput

This change implements MIDIInput.
midimessage Event is not invoked yet.

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

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

[Takashi Toyoshima, 2013-04-26 17:54:34]

Hi Chris,

This is initial change for MIDIInput.
Can you take a look?

[Chris Rogers, 2013-04-26 19:55:39]

https://codereview.chromium.org/14520002/diff/1/Source/modules/webmidi/MIDIIn...
File Source/modules/webmidi/MIDIInput.cpp (right):

https://codereview.chromium.org/14520002/diff/1/Source/modules/webmidi/MIDIIn...
Source/modules/webmidi/MIDIInput.cpp:43: ASSERT(src->m_type ==
MIDIPortTypeInput);
Please make a typeCode() method and use that instead of direct access to the
member variable

https://codereview.chromium.org/14520002/diff/1/Source/modules/webmidi/MIDIIn...
File Source/modules/webmidi/MIDIInput.h (right):

https://codereview.chromium.org/14520002/diff/1/Source/modules/webmidi/MIDIIn...
Source/modules/webmidi/MIDIInput.h:46: static PassRefPtr<MIDIInput>
create(PassRefPtr<MIDIPort>);
I'm not sure I understand how this create() method will be used.  You know
better what code will call this, but I'd like to understand the idea.  It seems
like all of these ports will be created in the MIDIAccess implementation (or
WebMIDIAccess), so do we ever need this case?

https://codereview.chromium.org/14520002/diff/1/Source/modules/webmidi/MIDIIn...
Source/modules/webmidi/MIDIInput.h:48: DEFINE_ATTRIBUTE_EVENT_LISTENER(message);
Chris Wilson has told me that the name should be "onmidimessage" and that he
will be updating the spec very soon, so we should use that name.

https://codereview.chromium.org/14520002/diff/1/Source/modules/webmidi/MIDIIn...
File Source/modules/webmidi/MIDIInput.idl (right):

https://codereview.chromium.org/14520002/diff/1/Source/modules/webmidi/MIDIIn...
Source/modules/webmidi/MIDIInput.idl:35: attribute EventListener onmessage;
Chris Wilson has told me that the name should be "onmidimessage" and that he
will be updating the spec very soon, so we should use that name.

[Takashi Toyoshima, 2013-05-02 08:55:25]

Thanks!

https://codereview.chromium.org/14520002/diff/1/Source/modules/webmidi/MIDIIn...
File Source/modules/webmidi/MIDIInput.cpp (right):

https://codereview.chromium.org/14520002/diff/1/Source/modules/webmidi/MIDIIn...
Source/modules/webmidi/MIDIInput.cpp:43: ASSERT(src->m_type ==
MIDIPortTypeInput);
On 2013/04/26 19:55:39, Chris Rogers wrote:
> Please make a typeCode() method and use that instead of direct access to the
> member variable

Done.

https://codereview.chromium.org/14520002/diff/1/Source/modules/webmidi/MIDIIn...
Source/modules/webmidi/MIDIInput.cpp:43: ASSERT(src->m_type ==
MIDIPortTypeInput);
Oops. I don't need it any more because I removed create function with MIDIPort.

https://codereview.chromium.org/14520002/diff/1/Source/modules/webmidi/MIDIIn...
File Source/modules/webmidi/MIDIInput.h (right):

https://codereview.chromium.org/14520002/diff/1/Source/modules/webmidi/MIDIIn...
Source/modules/webmidi/MIDIInput.h:46: static PassRefPtr<MIDIInput>
create(PassRefPtr<MIDIPort>);
In my first implementation, this was called from MIDIAccess::getInput().

I though that MIDIAccess holds MIDI device information as a MIDIPort array and
pass the reference of it to JavaScript via getInputs(). When JavaScript calls
getInput(short or DOMString) it will be converted to getInput(MIDIPort)
internally. And create MIDIInput with the provided MIDIPort.

But, maybe we will not need this because Chris will simplify MIDIAccess. It will
hold MIDIInput and MIDIOutput array directly.

I'll remove this.

https://codereview.chromium.org/14520002/diff/1/Source/modules/webmidi/MIDIIn...
Source/modules/webmidi/MIDIInput.h:48: DEFINE_ATTRIBUTE_EVENT_LISTENER(message);
Yes.
I discussed it here.
https://github.com/WebAudio/web-midi-api/issues/50

There are some objections, but I'll adopt new midimessage for now.

https://codereview.chromium.org/14520002/diff/1/Source/modules/webmidi/MIDIIn...
File Source/modules/webmidi/MIDIInput.idl (right):

https://codereview.chromium.org/14520002/diff/1/Source/modules/webmidi/MIDIIn...
Source/modules/webmidi/MIDIInput.idl:35: attribute EventListener onmessage;
On 2013/04/26 19:55:39, Chris Rogers wrote:
> Chris Wilson has told me that the name should be "onmidimessage" and that he
> will be updating the spec very soon, so we should use that name.

Done.

[Takashi Toyoshima, 2013-05-02 08:56:26]

+haraken for core/dom/EventTarget*

[haraken, 2013-05-02 09:06:57]

LGTM

https://codereview.chromium.org/14520002/diff/10001/Source/modules/webmidi/MI...
File Source/modules/webmidi/MIDIInput.idl (right):

https://codereview.chromium.org/14520002/diff/10001/Source/modules/webmidi/MI...
Source/modules/webmidi/MIDIInput.idl:33: EventTarget

Another option is to have MIDIPort inherit EventTarget (this is a desirable
solution in terms of the IDL spec), but it might not work due to lack of
implementation support for EventTarget. I'm OK with this change for now.

[haraken, 2013-05-02 09:07:25]

Please wait for L-G-T-M from crogers for modules/

[Chris Rogers, 2013-05-02 19:26:57]

lgtm with small nit

https://codereview.chromium.org/14520002/diff/10001/Source/modules/webmidi/MI...
File Source/modules/webmidi/MIDIInput.h (right):

https://codereview.chromium.org/14520002/diff/10001/Source/modules/webmidi/MI...
Source/modules/webmidi/MIDIInput.h:44: virtual ~MIDIInput() { }
small nit: I'd put the destructor after create()

[Takashi Toyoshima, 2013-05-03 13:08:37]

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