Remove ContextClient from NavigatorMIDI

Remove ContextClient from NavigatorMIDI

It's not needed. ContextClient is used to detect if a frame is detached,
but we can check it with scriptState->contextIsValid().

BUG=610176
Review-Url: https://codereview.chromium.org/2614943003
Cr-Commit-Position: refs/heads/master@{#441924}
Committed: https://chromium.googlesource.com/chromium/src/+/5d044bb4fef05b493693b2d548352bc025f08d46

[haraken, 2017-01-06 01:11:19]

haraken@chromium.org changed reviewers:
+ sigbjornf@opera.com

[haraken, 2017-01-06 01:11:19]

PTAL

https://codereview.chromium.org/2614943003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webmidi/NavigatorWebMIDI.cpp (right):

https://codereview.chromium.org/2614943003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webmidi/NavigatorWebMIDI.cpp:75: if
(!scriptState->contextIsValid()) {

This is "mostly" equivalent to the previous check (except a very subtle
scenario).

[sof, 2017-01-06 07:33:33]

https://codereview.chromium.org/2614943003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webmidi/NavigatorWebMIDI.cpp (right):

https://codereview.chromium.org/2614943003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webmidi/NavigatorWebMIDI.cpp:75: if
(!scriptState->contextIsValid()) {
On 2017/01/06 01:11:19, haraken wrote:
> 
> This is "mostly" equivalent to the previous check (except a very subtle
> scenario).

If |scriptState| represents the script state of the caller (with its document
etc), but what was here previously was that of the frame&document of the object,
it seems like it differs quite a bit. Is that the case?

[haraken, 2017-01-06 08:13:35]

https://codereview.chromium.org/2614943003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webmidi/NavigatorWebMIDI.cpp (right):

https://codereview.chromium.org/2614943003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webmidi/NavigatorWebMIDI.cpp:75: if
(!scriptState->contextIsValid()) {
On 2017/01/06 07:33:33, sof wrote:
> On 2017/01/06 01:11:19, haraken wrote:
> > 
> > This is "mostly" equivalent to the previous check (except a very subtle
> > scenario).
> 
> If |scriptState| represents the script state of the caller (with its document
> etc), but what was here previously was that of the frame&document of the
object,
> it seems like it differs quite a bit. Is that the case?

I think the two frames should match. The user script is calling
navigator.requestMIDIAccess. Then in the C++ callback for requestMIDIAccess,
ScriptState::current() returns the ScriptState of the navigator object. So they
should match (in theory).

Also below we're calling MIDIAccessInitializer::start(scriptState, options),
which indicates that it's better to validate ScriptState rather than frame() or
document().

[sof, 2017-01-06 10:21:20]

lgtm

https://codereview.chromium.org/2614943003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webmidi/NavigatorWebMIDI.cpp (right):

https://codereview.chromium.org/2614943003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webmidi/NavigatorWebMIDI.cpp:75: if
(!scriptState->contextIsValid()) {
On 2017/01/06 08:13:35, haraken wrote:
> On 2017/01/06 07:33:33, sof wrote:
> > On 2017/01/06 01:11:19, haraken wrote:
> > > 
> > > This is "mostly" equivalent to the previous check (except a very subtle
> > > scenario).
> > 
> > If |scriptState| represents the script state of the caller (with its
document
> > etc), but what was here previously was that of the frame&document of the
> object,
> > it seems like it differs quite a bit. Is that the case?
> 
> I think the two frames should match. The user script is calling
> navigator.requestMIDIAccess. Then in the C++ callback for requestMIDIAccess,
> ScriptState::current() returns the ScriptState of the navigator object. So
they
> should match (in theory).
> 
> Also below we're calling MIDIAccessInitializer::start(scriptState, options),
> which indicates that it's better to validate ScriptState rather than frame()
or
> document().
> 

agreed, thanks for clarifying.

[haraken, 2017-01-06 10:54:50]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2017-01-06 10:55:01]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-06 12:18:23]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1483700090771250, "parent_rev":
"c1751e0b4be0106ef84dcde9ab48816bfa74a95a", "commit_rev":
"5d044bb4fef05b493693b2d548352bc025f08d46"}

[commit-bot: I haz the power, 2017-01-06 12:18:50]

Description was changed from

==========
Remove ContextClient from NavigatorMIDI

It's not needed. ContextClient is used to detect if a frame is detached,
but we can check it with scriptState->contextIsValid().

BUG=610176
==========

to

==========
Remove ContextClient from NavigatorMIDI

It's not needed. ContextClient is used to detect if a frame is detached,
but we can check it with scriptState->contextIsValid().

BUG=610176

Review-Url: https://codereview.chromium.org/2614943003
Cr-Commit-Position: refs/heads/master@{#441924}
Committed:
https://chromium.googlesource.com/chromium/src/+/5d044bb4fef05b493693b2d54835...
==========

[commit-bot: I haz the power, 2017-01-06 12:18:51]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/5d044bb4fef05b493693b2d54835...