Web MIDI: MIDIPort doesn't have to be ActiveDOMObject

Web MIDI: MIDIPort doesn't have to be ActiveDOMObject

MIDIAccess is the only class which should be ActiveDOMObject.
MIDIPort, MIDIInput, and MIDIOutput does't have to be.

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

[Takashi Toyoshima, 2013-09-12 09:44:57]

Hi Kouhei,

As you suggested, I make MIDIPort and so on not to be ActiveDOMObject.
Please take a look.

[kouhei (in TOK), 2013-09-13 01:50:27]

The CL seems to be missing MIDIOutput.idl?

[Takashi Toyoshima, 2014-02-06 13:06:57]

Hi, I'd revisit this CL.
Can you take a look?

[Takashi Toyoshima, 2014-02-10 05:32:20]

ping

[kouhei (in TOK), 2014-02-10 05:36:24]

On 2014/02/10 05:32:20, Takashi Toyoshima (chromium) wrote:
> ping

Sorry for the delay. The mail has somehow slipped out. Started reviewing now.

[kouhei (in TOK), 2014-02-10 06:07:31]

https://codereview.chromium.org/23609033/diff/159001/Source/modules/webmidi/M...
File Source/modules/webmidi/MIDIPort.h (right):

https://codereview.chromium.org/23609033/diff/159001/Source/modules/webmidi/M...
Source/modules/webmidi/MIDIPort.h:77: MIDIAccess* m_access;
Is this raw-ptr to avoid refcycles? Please comment if so.

[Takashi Toyoshima, 2014-02-10 06:29:02]

Good point. Also I have a related question about SetWrapperReferenceTo()
annotation because I don't have enough knowledge how this annotation works.
Now, we annotate it in MIDIInput.idl and MIDIOutput.idl. Can we move them to
MIDIPort.idl? If it works, it's easy to understand why we can use raw pointer
safely in MIDIPort.cpp. Of course, I'll add some comment there.

[kouhei (in TOK), 2014-02-10 06:35:21]

On 2014/02/10 06:29:02, Takashi Toyoshima (chromium) wrote:
> Good point. Also I have a related question about SetWrapperReferenceTo()
> annotation because I don't have enough knowledge how this annotation works.
> Now, we annotate it in MIDIInput.idl and MIDIOutput.idl. Can we move them to
> MIDIPort.idl? If it works, it's easy to understand why we can use raw pointer
> safely in MIDIPort.cpp. Of course, I'll add some comment there.

A good(?) news is that I am going to migrate webmidi to oilpan real soon.
(should be mid-Feb), so we should be able to get rid of these refcycle hacks.

[Takashi Toyoshima, 2014-02-14 07:41:51]

That's really good news.
Anyway, I updated my PS5 to 6. I moved SetWrapperReferenceTo() annotation to
MIDIPort.idl, and added comment on raw pointer.
PTAL.

[kouhei (in TOK), 2014-02-14 08:00:26]

On 2014/02/14 07:41:51, Takashi Toyoshima (chromium) wrote:
> That's really good news.
> Anyway, I updated my PS5 to 6. I moved SetWrapperReferenceTo() annotation to
> MIDIPort.idl, and added comment on raw pointer.
> PTAL.
Thanks for adding comments.

I'm not sure if we are allowed to SetWrapperReferenceTo on parent class to be
inherited to child classes. Would you check on generated code and make sure its
working as intended?

[Takashi Toyoshima, 2014-02-14 10:02:07]

I checked generated codes.
MIDIInput and MIDIOutput define WrapperTypeInfo which does not contain
visitDOMWrapper calling setWrapperReference.
So, IIUC, it does not work correctly.

I reverted idl changes and uploaded new CL as PS7.
PTAL.

[kouhei (in TOK), 2014-02-14 10:04:07]

lgtm

[commit-bot: I haz the power, 2014-02-14 10:04:15]

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

[commit-bot: I haz the power, 2014-02-14 12:09:30]

Change committed as 167183