Web MIDI: implement MIDIOutput

Web MIDI: implement MIDIOutput

This change implements MIDIOutput interfaces.
send methods are not implemeneted yet.

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

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

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

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

[Takashi Toyoshima, 2013-04-26 17:29:13]

Hi Chris,
Can you take a look?

[Chris Rogers, 2013-04-29 19:24:19]

https://codereview.chromium.org/14442004/diff/4001/Source/modules/webmidi/MID...
File Source/modules/webmidi/MIDIOutput.cpp (right):

https://codereview.chromium.org/14442004/diff/4001/Source/modules/webmidi/MID...
Source/modules/webmidi/MIDIOutput.cpp:43: ASSERT(src->m_type ==
MIDIPortTypeOutput);
seems better to make a public MIDIPort::typeCode() method and use that instead
of accessing m_type directly

https://codereview.chromium.org/14442004/diff/4001/Source/modules/webmidi/MID...
Source/modules/webmidi/MIDIOutput.cpp:54: // FIXME: Implement me.
I think it would be good to add a little more detail in this comment describing
very generally how this will be implemented by using an object in "Platform
layer implementation core/platform/midi"
Even a few sentences would be good, so we know the plan for this code.


I was thinking we could output a console warning message here, but it's probably
not worth the trouble, since this will likely be implemented quite soon.

https://codereview.chromium.org/14442004/diff/4001/Source/modules/webmidi/MID...
File Source/modules/webmidi/MIDIOutput.h (right):

https://codereview.chromium.org/14442004/diff/4001/Source/modules/webmidi/MID...
Source/modules/webmidi/MIDIOutput.h:46: static PassRefPtr<MIDIOutput>
create(PassRefPtr<MIDIPort>);
Will this create() method be necessary?  You have a better idea of where it
would be called from, just curious...

[Takashi Toyoshima, 2013-05-02 09:14:08]

Thanks.

https://codereview.chromium.org/14442004/diff/4001/Source/modules/webmidi/MID...
File Source/modules/webmidi/MIDIOutput.cpp (right):

https://codereview.chromium.org/14442004/diff/4001/Source/modules/webmidi/MID...
Source/modules/webmidi/MIDIOutput.cpp:43: ASSERT(src->m_type ==
MIDIPortTypeOutput);
I removed this create function. As a result, this check is not needed.

https://codereview.chromium.org/14442004/diff/4001/Source/modules/webmidi/MID...
Source/modules/webmidi/MIDIOutput.cpp:54: // FIXME: Implement me.
On 2013/04/29 19:24:19, Chris Rogers wrote:
> I think it would be good to add a little more detail in this comment
describing
> very generally how this will be implemented by using an object in "Platform
> layer implementation core/platform/midi"
> Even a few sentences would be good, so we know the plan for this code.
> 
> 
> I was thinking we could output a console warning message here, but it's
probably
> not worth the trouble, since this will likely be implemented quite soon.

Done.

https://codereview.chromium.org/14442004/diff/4001/Source/modules/webmidi/MID...
File Source/modules/webmidi/MIDIOutput.h (right):

https://codereview.chromium.org/14442004/diff/4001/Source/modules/webmidi/MID...
Source/modules/webmidi/MIDIOutput.h:46: static PassRefPtr<MIDIOutput>
create(PassRefPtr<MIDIPort>);
I'll remove this due to the same reason of MIDIInput.

[Chris Rogers, 2013-05-02 19:32:35]

lgtm with small nit

https://codereview.chromium.org/14442004/diff/9001/Source/modules/webmidi/MID...
File Source/modules/webmidi/MIDIOutput.h (right):

https://codereview.chromium.org/14442004/diff/9001/Source/modules/webmidi/MID...
Source/modules/webmidi/MIDIOutput.h:45: static PassRefPtr<MIDIOutput>
create(ScriptExecutionContext*, const String& id, const String& manufacturer,
const String& name, const String& version);
small nit: I'd put destructor after create()

[Takashi Toyoshima, 2013-05-03 13:20:39]

Committed patchset #5 manually as r149649 (presubmit successful).

[dglazkov, 2013-05-03 16:13:13]

I am surprised you didn't run any trybots on this patch. It broke compile on
Windows, and trybots would've have shown it.

[Takashi Toyoshima, 2013-05-04 05:24:42]

reopened.

Dimitri:
Sorry, I was careless on this change.

[commit-bot: I haz the power, 2013-05-10 09:38:36]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[Takashi Toyoshima, 2013-05-10 09:40:27]

Hi, can you grant me to land this change again?
This change broke build and was reverted.
I fixed the problem to land this again.

[haraken, 2013-05-13 10:21:51]

LGTM

[commit-bot: I haz the power, 2013-05-13 11:21:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/toyoshim@chromium.org/14442004/37001

[commit-bot: I haz the power, 2013-05-13 11:55:27]

Retried try job too often on linux_layout_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_layo...

[commit-bot: I haz the power, 2013-05-13 12:51:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/toyoshim@chromium.org/14442004/37001

[commit-bot: I haz the power, 2013-05-13 13:24:13]

Retried try job too often on linux_layout_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_layo...

[commit-bot: I haz the power, 2013-05-13 13:41:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/toyoshim@chromium.org/14442004/37001

[commit-bot: I haz the power, 2013-05-13 13:42:11]

Change committed as 150219

[eseidel, 2013-05-13 17:28:08]

This appears to have broken:
webexposed/global-constructors-listing.html

I'm not sure why the CQ didn't catch it.

[eseidel, 2013-05-13 17:30:03]

It's possible that DOMWindow.* is not properly getting regenerated when any idl
is touched?

[Takashi Toyoshima, 2013-05-14 06:21:54]

Sorry, it's my bad.
This interface doesn't need a global constructor so I'll add NoInterfaceObject.

[haraken, 2013-05-14 07:28:52]

LGTM

[commit-bot: I haz the power, 2013-05-14 07:29:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/toyoshim@chromium.org/14442004/65001

[commit-bot: I haz the power, 2013-05-14 20:12:30]

Change committed as 150343