Web MIDI: MIDIInput/MIDIOutput should not have reference to MIDIAccess

Web MIDI: MIDIInput/MIDIOutput should not have reference to MIDIAccess

Currently, MIDIOutput has a reference to MIDIAccess introducing a cyclic
reference between them. To avoid this issue, this change adopts a simple idea
to have a raw pointer to MIDIAccess from MIDIOutput and MIDIInput, and
internally has a reference to MIDIAccess in the v8 bindings.
This internal reference make MIDIAccess alive while MIDIOutput and MIDIInput
live. So, MIDIOutput and MIDIInput can assume the raw pointer is always
valid. Also the reference can be GCed in the v8 world.

BUG=163795
TEST=LayoutTests/webmidi

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

[Takashi Toyoshima, 2013-09-09 07:53:17]

Hi Kouhei,

This change will fix the cyclic reference problem you find in the review on
https://chromiumcodereview.appspot.com/23648006/.
Can you take a look? Once this change is landed, I'll update the previous change
to work with this.

Thanks!

[kouhei (in TOK), 2013-09-09 08:09:13]

lgtm.
+haraken about introducing custom bindings.

[kouhei (in TOK), 2013-09-09 08:09:30]

On 2013/09/09 08:09:13, kouhei wrote:
> lgtm.
> +haraken about introducing custom bindings.

[haraken, 2013-09-09 08:19:17]

On 2013/09/09 08:09:30, kouhei wrote:
> On 2013/09/09 08:09:13, kouhei wrote:
> > lgtm.
> > +haraken about introducing custom bindings.

We're doing the same thing for V8TreeWalkerCustom.cpp and
V8NodeIteratorCustom.cpp.

Can we introduce [SetHiddenReferenceTo=xxx] IDL attribute, generate the wrap
method automatically, and thus get rid of all of the custom bindings?

[Takashi Toyoshima, 2013-09-09 10:26:35]

That sounds awesome. I'll use it once SetHiddenRefenrenceTo attribute is ready
to use.
This is not just a refactoring change, but have a dependent change that enable
one of Web MIDI feature.
Can you allow me to land this without waiting for the attribute?

[Takashi Toyoshima, 2013-09-09 10:50:26]

Note that since MIDIInput.idl and MIDIOutput.idl are still compiled by
deprecated perl IDL compiler, introducing new attribute for them are wasteful
for now.

[haraken, 2013-09-09 15:35:18]

Actually I'd want you to fix the code generator before adding the two custom
bindings. The compiler rewrite won't happen soon, so it makes sense to implement
the [SetHiddenValueTo=xxx] IDL attribute to the perl generator (I'm really sorry
that it is taking so much time rewriting the compiler. Now we're planning to
revert back to the Perl generator until we complete the new Python generator in
our local branch).

Also, please add a comment to MIDIOutput.h and MIDIInput.h about how the
lifetime of MIDI objects is managed.

[kouhei (in TOK), 2013-09-09 23:47:15]

On 2013/09/09 15:35:18, haraken wrote:
> Actually I'd want you to fix the code generator before adding the two custom
> bindings. The compiler rewrite won't happen soon, so it makes sense to
implement
> the [SetHiddenValueTo=xxx] IDL attribute to the perl generator (I'm really
sorry
> that it is taking so much time rewriting the compiler. Now we're planning to
> revert back to the Perl generator until we complete the new Python generator
in
> our local branch).
> 
> Also, please add a comment to MIDIOutput.h and MIDIInput.h about how the
> lifetime of MIDI objects is managed.

OK, should I or nbarth take over that task? I'll start on that this morning.

[haraken, 2013-09-10 00:00:44]

> OK, should I or nbarth take over that task? I'll start on that this morning.

If you could, that's awesome!

[haraken, 2013-09-10 08:30:45]

Discussed offline with kouhei. We decided to solve the problem by introducing
[ReachableFrom] and [ReachableTo]. See discussion in
https://codereview.chromium.org/23532053/ . This work will take a couple of
days.

In the Blink community, we're strongly encouraged to fix/refactor underlying
problems first and then implement new features. Unless security fixes, we don't
want to accept short-term hacks.

However, in this particular case, you're just introducing custom bindings and
kouhei already started working on [ReachableFrom] and [ReachableTo], so I'm OK
with landing the CL now. LGTM.

[Takashi Toyoshima, 2013-09-10 08:49:08]

Thanks:)

[commit-bot: I haz the power, 2013-09-10 08:49:35]

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

[commit-bot: I haz the power, 2013-09-10 09:45:32]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_layout...

[commit-bot: I haz the power, 2013-09-10 11:04:48]

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

[commit-bot: I haz the power, 2013-09-10 11:53:25]

Change committed as 157517