Fulfill or reject MIDIAccessPromise from MIDIAccessor

Fulfill or reject MIDIAccessPromise from MIDIAccessor

This asks the browser for permission to access MIDI hardware, including whether SYSEX
access is being requested. The browser will either block access, in which case the
promise's failure callback will be invoked, or the browser will add the available
input and output ports to the MIDIAccess object, and then invoke the success callback
with the MIDIAccess as an argument according to the Web MIDI spec.

BUG=163795
TEST=webmidi/requestmidiaccess.html (and tested manually with real MIDI devices)
R=toyoshim@chromium.org

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

[abarth-chromium, 2013-06-24 20:50:43]

https://codereview.chromium.org/17619003/diff/1/Source/modules/webmidi/MIDIAc...
File Source/modules/webmidi/MIDIAccessPromise.h (right):

https://codereview.chromium.org/17619003/diff/1/Source/modules/webmidi/MIDIAc...
Source/modules/webmidi/MIDIAccessPromise.h:51: class MIDIAccessPromise : public
RefCounted<MIDIAccessPromise>, public ScriptWrappable, public ActiveDOMObject {
What is a MIDIAccessPromise?  We haven't implemented promises yet...

[abarth-chromium, 2013-06-24 20:51:29]

> TEST=(tested manually with real MIDI devices)

^^^ We need automated tests for this feature.  Testing with real MIDI devices
isn't maintainable.

[Chris Rogers, 2013-06-24 21:00:32]

For a test, I can add a mock implementation of WebMIDIAccessor in
test_webkit_platform_support.cc, and then can write a simple test requesting
access.  Would this be in line with your thinking?

https://codereview.chromium.org/17619003/diff/1/Source/modules/webmidi/MIDIAc...
File Source/modules/webmidi/MIDIAccessPromise.h (right):

https://codereview.chromium.org/17619003/diff/1/Source/modules/webmidi/MIDIAc...
Source/modules/webmidi/MIDIAccessPromise.h:51: class MIDIAccessPromise : public
RefCounted<MIDIAccessPromise>, public ScriptWrappable, public ActiveDOMObject {
On 2013/06/24 20:50:43, abarth wrote:
> What is a MIDIAccessPromise?  We haven't implemented promises yet...

Takashi has implemented the part of promises that will be the normal use case
with the Web MIDI API as described in the spec example, which is the then()
method.  So although it's true that the more advanced uses for promises are not
yet implemented, this gives us a basic implementation which should be forward
compatible with the full impl once we have "for-real" promises.

[abarth-chromium, 2013-06-25 04:43:34]

On 2013/06/24 21:00:32, Chris Rogers wrote:
> For a test, I can add a mock implementation of WebMIDIAccessor in
> test_webkit_platform_support.cc, and then can write a simple test requesting
> access.  Would this be in line with your thinking?

That sounds great.

>
https://codereview.chromium.org/17619003/diff/1/Source/modules/webmidi/MIDIAc...
> File Source/modules/webmidi/MIDIAccessPromise.h (right):
> 
>
https://codereview.chromium.org/17619003/diff/1/Source/modules/webmidi/MIDIAc...
> Source/modules/webmidi/MIDIAccessPromise.h:51: class MIDIAccessPromise :
public
> RefCounted<MIDIAccessPromise>, public ScriptWrappable, public ActiveDOMObject
{
> On 2013/06/24 20:50:43, abarth wrote:
> > What is a MIDIAccessPromise?  We haven't implemented promises yet...
> 
> Takashi has implemented the part of promises that will be the normal use case
> with the Web MIDI API as described in the spec example, which is the then()
> method.  So although it's true that the more advanced uses for promises are
not
> yet implemented, this gives us a basic implementation which should be forward
> compatible with the full impl once we have "for-real" promises.

Makes sense.  Thanks.

[Takashi Toyoshima, 2013-06-25 07:48:57]

Thank you Chris,
One confirmation on DOMError type.

https://chromiumcodereview.appspot.com/17619003/diff/1/Source/modules/webmidi...
File Source/modules/webmidi/MIDIAccess.cpp (right):

https://chromiumcodereview.appspot.com/17619003/diff/1/Source/modules/webmidi...
Source/modules/webmidi/MIDIAccess.cpp:93: RefPtr<DOMError> error =
DOMError::create("MIDI access blocked");
Should be "SecurityError".

http://webaudio.github.io/web-midi-api/#requestmidiaccess
4.1.1 Methods, step 9.

https://chromiumcodereview.appspot.com/17619003/diff/1/Source/modules/webmidi...
File Source/modules/webmidi/MIDIAccessPromise.h (right):

https://chromiumcodereview.appspot.com/17619003/diff/1/Source/modules/webmidi...
Source/modules/webmidi/MIDIAccessPromise.h:51: class MIDIAccessPromise : public
RefCounted<MIDIAccessPromise>, public ScriptWrappable, public ActiveDOMObject {
Yes, we have a comment on that at MIDIAccessPromise.idl as Once Promise is
implemented, remove this interface and use it.

[Chris Rogers, 2013-06-26 01:47:55]

PTAL: I've added a basic layout test

https://codereview.chromium.org/17619003/diff/1/Source/modules/webmidi/MIDIAc...
File Source/modules/webmidi/MIDIAccess.cpp (right):

https://codereview.chromium.org/17619003/diff/1/Source/modules/webmidi/MIDIAc...
Source/modules/webmidi/MIDIAccess.cpp:93: RefPtr<DOMError> error =
DOMError::create("MIDI access blocked");
On 2013/06/25 07:48:58, Takashi Toyoshima (chromium) wrote:
> Should be "SecurityError".
> 
> http://webaudio.github.io/web-midi-api/#requestmidiaccess
> 4.1.1 Methods, step 9.

Done.

[Takashi Toyoshima, 2013-06-26 03:53:13]

We already have a simple test for midi access at
LayoutTests/webmidi/request-midi-access.html
I think your test includes the old one. Should we merge it?

Other things lgtm.

[abarth-chromium, 2013-06-26 19:47:20]

Thanks for the test.

[Chris Rogers, 2013-06-27 00:02:36]

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