Web MIDI: implement MIDIErrorCallback

Web MIDI: implement MIDIErrorCallback

This change implements MIDIErrorCallback.

BUG=163795
TEST=none
R=crogers@google.com

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

[Takashi Toyoshima, 2013-04-23 06:58:42]

Hi Chris,

This is the second interface.
Can you take a look this?

[Chris Rogers, 2013-04-23 20:35:23]

toyoshim: lgtm with nits

cwilso: Web MIDI spec should probably be updated to define ErrorCallback as
MIDIErrorCallback like Takashi has chosen

https://codereview.chromium.org/14423002/diff/1/Source/modules/webmidi/MIDIEr...
File Source/modules/webmidi/MIDIErrorCallback.cpp (right):

https://codereview.chromium.org/14423002/diff/1/Source/modules/webmidi/MIDIEr...
Source/modules/webmidi/MIDIErrorCallback.cpp:44: static
PassOwnPtr<DispatchCallbackTask> create(PassRefPtr<MIDIErrorCallback> callback,
PassRefPtr<DOMError> data)
"data" should be named "error"

https://codereview.chromium.org/14423002/diff/1/Source/modules/webmidi/MIDIEr...
Source/modules/webmidi/MIDIErrorCallback.cpp:62: RefPtr<DOMError> m_data;
should be named "m_error"

https://codereview.chromium.org/14423002/diff/1/Source/modules/webmidi/MIDIEr...
Source/modules/webmidi/MIDIErrorCallback.cpp:67: void
MIDIErrorCallback::scheduleCallback(ScriptExecutionContext* context,
PassRefPtr<DOMError> data)
"data" should be named "error"

https://codereview.chromium.org/14423002/diff/1/Source/modules/webmidi/MIDIEr...
File Source/modules/webmidi/MIDIErrorCallback.h (right):

https://codereview.chromium.org/14423002/diff/1/Source/modules/webmidi/MIDIEr...
Source/modules/webmidi/MIDIErrorCallback.h:41: class SecurityExecutionContext;
seems like SecurityExecutionContext is not needed - can be removed

[Takashi Toyoshima, 2013-04-24 01:31:51]

https://codereview.chromium.org/14423002/diff/1/Source/modules/webmidi/MIDIEr...
File Source/modules/webmidi/MIDIErrorCallback.cpp (right):

https://codereview.chromium.org/14423002/diff/1/Source/modules/webmidi/MIDIEr...
Source/modules/webmidi/MIDIErrorCallback.cpp:44: static
PassOwnPtr<DispatchCallbackTask> create(PassRefPtr<MIDIErrorCallback> callback,
PassRefPtr<DOMError> data)
On 2013/04/23 20:35:23, Chris Rogers wrote:
> "data" should be named "error"

Done.

https://codereview.chromium.org/14423002/diff/1/Source/modules/webmidi/MIDIEr...
Source/modules/webmidi/MIDIErrorCallback.cpp:62: RefPtr<DOMError> m_data;
On 2013/04/23 20:35:23, Chris Rogers wrote:
> should be named "m_error"

Done.

https://codereview.chromium.org/14423002/diff/1/Source/modules/webmidi/MIDIEr...
Source/modules/webmidi/MIDIErrorCallback.cpp:67: void
MIDIErrorCallback::scheduleCallback(ScriptExecutionContext* context,
PassRefPtr<DOMError> data)
On 2013/04/23 20:35:23, Chris Rogers wrote:
> "data" should be named "error"

Done.

https://codereview.chromium.org/14423002/diff/1/Source/modules/webmidi/MIDIEr...
File Source/modules/webmidi/MIDIErrorCallback.h (right):

https://codereview.chromium.org/14423002/diff/1/Source/modules/webmidi/MIDIEr...
Source/modules/webmidi/MIDIErrorCallback.h:41: class SecurityExecutionContext;
On 2013/04/23 20:35:23, Chris Rogers wrote:
> seems like SecurityExecutionContext is not needed - can be removed

Done.

[commit-bot: I haz the power, 2013-04-24 01:39:46]

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-04-24 01:52:02]

+tkent for modules/modules.gypi OWNERS review.
Hum... I feel modules/modules.gypi should have per-file OWNERS entry.

[Chris Rogers, 2013-04-24 01:58:33]

On 2013/04/24 01:52:02, Takashi Toyoshima (chromium) wrote:
> +tkent for modules/modules.gypi OWNERS review.
> Hum... I feel modules/modules.gypi should have per-file OWNERS entry.

Yes, hopefully we can get more people as OWNERS there --- seems like this will
happen a lot

[tkent, 2013-04-24 02:08:43]

https://codereview.chromium.org/14423002/diff/10001/Source/modules/webmidi/MI...
File Source/modules/webmidi/MIDIErrorCallback.cpp (right):

https://codereview.chromium.org/14423002/diff/10001/Source/modules/webmidi/MI...
Source/modules/webmidi/MIDIErrorCallback.cpp:33: #include "MIDIErrorCallback.h"
Please specify "modules/webmidi/MIDIErrorCallback.h"

https://codereview.chromium.org/14423002/diff/10001/Source/modules/webmidi/MI...
Source/modules/webmidi/MIDIErrorCallback.cpp:35: #include "DOMError.h"
Ditto. Use path relative to WebKit/Source/.

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

https://codereview.chromium.org/14423002/diff/10001/Source/modules/webmidi/MI...
Source/modules/webmidi/MIDIErrorCallback.h:34: #include <wtf/PassRefPtr.h>
Please quote by "", instead of <>.

https://codereview.chromium.org/14423002/diff/10001/Source/modules/webmidi/MI...
Source/modules/webmidi/MIDIErrorCallback.h:51: } // namespace
Just "namespace" is confusing.  It should be "namespace WebCore", or we should
remove the comment.

[Takashi Toyoshima, 2013-04-24 02:23:12]

Thank you Kent-san.
I notice that the policy for #include was changed.

https://codereview.chromium.org/14423002/diff/10001/Source/modules/webmidi/MI...
File Source/modules/webmidi/MIDIErrorCallback.cpp (right):

https://codereview.chromium.org/14423002/diff/10001/Source/modules/webmidi/MI...
Source/modules/webmidi/MIDIErrorCallback.cpp:33: #include "MIDIErrorCallback.h"
On 2013/04/24 02:08:43, tkent wrote:
> Please specify "modules/webmidi/MIDIErrorCallback.h"
> 

Done.

https://codereview.chromium.org/14423002/diff/10001/Source/modules/webmidi/MI...
Source/modules/webmidi/MIDIErrorCallback.cpp:35: #include "DOMError.h"
On 2013/04/24 02:08:43, tkent wrote:
> Ditto. Use path relative to WebKit/Source/.

Done.

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

https://codereview.chromium.org/14423002/diff/10001/Source/modules/webmidi/MI...
Source/modules/webmidi/MIDIErrorCallback.h:34: #include <wtf/PassRefPtr.h>
On 2013/04/24 02:08:43, tkent wrote:
> Please quote by "", instead of <>.

Done.

https://codereview.chromium.org/14423002/diff/10001/Source/modules/webmidi/MI...
Source/modules/webmidi/MIDIErrorCallback.h:51: } // namespace
On 2013/04/24 02:08:43, tkent wrote:
> Just "namespace" is confusing.  It should be "namespace WebCore", or we should
> remove the comment.

Done.

[tkent, 2013-04-24 02:25:29]

lgtm

[commit-bot: I haz the power, 2013-04-24 02:26:18]

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

[commit-bot: I haz the power, 2013-04-24 02:32:19]

Presubmit check for 14423002-8003 failed and returned exit status -2001.
The presubmit check was hung. It took 360.0 seconds to execute and the time
limit is 360.0 seconds.

INFO:root:Found 4 file(s).

[commit-bot: I haz the power, 2013-04-24 02:32:56]

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

[commit-bot: I haz the power, 2013-04-24 02:38:57]

Presubmit check for 14423002-8003 failed and returned exit status -2001.
The presubmit check was hung. It took 360.0 seconds to execute and the time
limit is 360.0 seconds.

INFO:root:Found 4 file(s).

[commit-bot: I haz the power, 2013-04-24 04:42:20]

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

[commit-bot: I haz the power, 2013-04-24 04:48:21]

Presubmit check for 14423002-8003 failed and returned exit status -2001.
The presubmit check was hung. It took 360.0 seconds to execute and the time
limit is 360.0 seconds.

INFO:root:Found 4 file(s).

[Takashi Toyoshima, 2013-04-24 05:21:19]

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