Web MIDI: final IDL updates to conform the latest WD for shipping

Web MIDI: final IDL updates to conform the latest WD for shipping

The latest WD that the shipping Web MIDI should follow was published.
This patch updates IDLs to follow the latest WD.

Note: this does not change users and developers facing APIs that are
already visible in stable and beta channels by default.

See http://www.w3.org/TR/2015/WD-webmidi-20150317/

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

[Takashi Toyoshima, 2015-03-30 06:43:24]

toyoshim@chromium.org changed reviewers:
+ tkent@chromium.org, yhirano@chromium.org

[Takashi Toyoshima, 2015-03-30 07:51:10]

Hi, can you take a look?
This is planned change to conform the latest spec WD to ship the API.

[tkent, 2015-03-30 07:55:02]

https://codereview.chromium.org/1043863002/diff/40001/Source/modules/webmidi/...
File Source/modules/webmidi/MIDIOutput.idl (right):

https://codereview.chromium.org/1043863002/diff/40001/Source/modules/webmidi/...
Source/modules/webmidi/MIDIOutput.idl:38: void clear();
Please do not add this before the implementation gets ready.

[Takashi Toyoshima, 2015-03-30 15:08:32]

ptal

https://codereview.chromium.org/1043863002/diff/40001/Source/modules/webmidi/...
File Source/modules/webmidi/MIDIOutput.idl (right):

https://codereview.chromium.org/1043863002/diff/40001/Source/modules/webmidi/...
Source/modules/webmidi/MIDIOutput.idl:38: void clear();
On 2015/03/30 07:55:02, tkent wrote:
> Please do not add this before the implementation gets ready.
> 

Done.

[tkent, 2015-03-30 23:16:30]

https://codereview.chromium.org/1043863002/diff/60001/Source/modules/webmidi/...
File Source/modules/webmidi/MIDIOutput.idl (right):

https://codereview.chromium.org/1043863002/diff/60001/Source/modules/webmidi/...
Source/modules/webmidi/MIDIOutput.idl:38: // FIXME: implement void clear();
nit: FIXME: -> TODO(toyoshim):

https://codereview.chromium.org/1043863002/diff/60001/Source/modules/webmidi/...
File Source/modules/webmidi/MIDIPort.cpp (right):

https://codereview.chromium.org/1043863002/diff/60001/Source/modules/webmidi/...
Source/modules/webmidi/MIDIPort.cpp:73: default:
Please do not add "default:"  It prevents compile-time check for enum coverage.

https://codereview.chromium.org/1043863002/diff/60001/Source/modules/webmidi/...
Source/modules/webmidi/MIDIPort.cpp:107: // FIXME: Implement the latest open()
algorithm.
nit: FIXME -> TODO(toyoshim)

https://codereview.chromium.org/1043863002/diff/60001/Source/modules/webmidi/...
Source/modules/webmidi/MIDIPort.cpp:123: // FIXME: Implement the latest close()
algorithm.
Ditto.

https://codereview.chromium.org/1043863002/diff/60001/Source/modules/webmidi/...
Source/modules/webmidi/MIDIPort.cpp:169:
dispatchEvent(MIDIConnectionEvent::create(this));
Is it possible to delete |this| by JavaScript code kicked by the event?

https://codereview.chromium.org/1043863002/diff/60001/Source/modules/webmidi/...
File Source/modules/webmidi/MIDIPort.h (right):

https://codereview.chromium.org/1043863002/diff/60001/Source/modules/webmidi/...
Source/modules/webmidi/MIDIPort.h:82: enum MIDIPortConnectionState {
nit: Because this is in MIDIPort class, you may omit the 'MIDIPort' prefix.

[Takashi Toyoshima, 2015-03-31 01:19:48]

https://codereview.chromium.org/1043863002/diff/60001/Source/modules/webmidi/...
File Source/modules/webmidi/MIDIOutput.idl (right):

https://codereview.chromium.org/1043863002/diff/60001/Source/modules/webmidi/...
Source/modules/webmidi/MIDIOutput.idl:38: // FIXME: implement void clear();
On 2015/03/30 23:16:30, tkent wrote:
> nit: FIXME: -> TODO(toyoshim):

Done.

https://codereview.chromium.org/1043863002/diff/60001/Source/modules/webmidi/...
File Source/modules/webmidi/MIDIPort.cpp (right):

https://codereview.chromium.org/1043863002/diff/60001/Source/modules/webmidi/...
Source/modules/webmidi/MIDIPort.cpp:73: default:
On 2015/03/30 23:16:30, tkent wrote:
> Please do not add "default:"  It prevents compile-time check for enum
coverage.
> 

Done.

https://codereview.chromium.org/1043863002/diff/60001/Source/modules/webmidi/...
Source/modules/webmidi/MIDIPort.cpp:107: // FIXME: Implement the latest open()
algorithm.
On 2015/03/30 23:16:30, tkent wrote:
> nit: FIXME -> TODO(toyoshim)

Done.

https://codereview.chromium.org/1043863002/diff/60001/Source/modules/webmidi/...
Source/modules/webmidi/MIDIPort.cpp:123: // FIXME: Implement the latest close()
algorithm.
On 2015/03/30 23:16:30, tkent wrote:
> Ditto.

Done.

https://codereview.chromium.org/1043863002/diff/60001/Source/modules/webmidi/...
Source/modules/webmidi/MIDIPort.cpp:169:
dispatchEvent(MIDIConnectionEvent::create(this));
Ah, I see. JavaScript can release the last reference in the handler.
Before Oilpan, we retain a reference to be finish function safely, but what's
the standard way in Oilpan? Just having a local reference of "this" in callers
will fix the problem? Please review the new code that is expected to fix the
issue.

https://codereview.chromium.org/1043863002/diff/60001/Source/modules/webmidi/...
File Source/modules/webmidi/MIDIPort.h (right):

https://codereview.chromium.org/1043863002/diff/60001/Source/modules/webmidi/...
Source/modules/webmidi/MIDIPort.h:82: enum MIDIPortConnectionState {
On 2015/03/30 23:16:30, tkent wrote:
> nit: Because this is in MIDIPort class, you may omit the 'MIDIPort' prefix.

Done.

[Takashi Toyoshima, 2015-03-31 01:23:09]

Patch Set 5 mistakenly contains rebase, and delta diff of
LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt show
unrelated diff line.

[tkent, 2015-03-31 01:26:10]

https://codereview.chromium.org/1043863002/diff/80001/Source/modules/webmidi/...
File Source/modules/webmidi/MIDIPort.cpp (right):

https://codereview.chromium.org/1043863002/diff/80001/Source/modules/webmidi/...
Source/modules/webmidi/MIDIPort.cpp:163: MIDIPort* self = this;
Ah, MIDIPort is already on Oilpan.  Then you need to do nothing.  Oilpan ensures
|this| is not collected while the stack or a CPU register contains |this|.

[tkent, 2015-03-31 01:32:37]

On 2015/03/31 01:26:10, tkent wrote:
>
https://codereview.chromium.org/1043863002/diff/80001/Source/modules/webmidi/...
> File Source/modules/webmidi/MIDIPort.cpp (right):
> 
>
https://codereview.chromium.org/1043863002/diff/80001/Source/modules/webmidi/...
> Source/modules/webmidi/MIDIPort.cpp:163: MIDIPort* self = this;
> Ah, MIDIPort is already on Oilpan.  Then you need to do nothing.  Oilpan
ensures
> |this| is not collected while the stack or a CPU register contains |this|.

I mean, |void setStates()| in Patch Set 4 was ok as is.

[Takashi Toyoshima, 2015-03-31 01:46:46]

Thanks. That sounds great. I updated the patch set 6 that revert "MIDIPort*
setStates()" changes.

[tkent, 2015-03-31 01:48:13]

lgtm

[yhirano, 2015-03-31 02:23:37]

https://codereview.chromium.org/1043863002/diff/80001/Source/modules/webmidi/...
File Source/modules/webmidi/MIDIPort.cpp (right):

https://codereview.chromium.org/1043863002/diff/80001/Source/modules/webmidi/...
Source/modules/webmidi/MIDIPort.cpp:163: MIDIPort* self = this;
On 2015/03/31 01:26:10, tkent wrote:
> Ah, MIDIPort is already on Oilpan.  Then you need to do nothing.  Oilpan
ensures
> |this| is not collected while the stack or a CPU register contains |this|.

But I think we should protect the wrapper in order to retain event listeners.
That means We need to make MIDIPort inherit ActiveDOMObject. Please add a TODO.

[Takashi Toyoshima, 2015-03-31 03:05:33]

I left a TODO in MIDIPort.idl.

[Takashi Toyoshima, 2015-03-31 03:05:39]

The CQ bit was checked by toyoshim@chromium.org

[Takashi Toyoshima, 2015-03-31 03:05:40]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org
Link to the patchset: https://codereview.chromium.org/1043863002/#ps120001
(title: "TODO: ActiveDOMObject")

[commit-bot: I haz the power, 2015-03-31 03:06:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1043863002/120001

[commit-bot: I haz the power, 2015-03-31 05:22:32]

Committed patchset #7 (id:120001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=192802