Oilpan: Move MediaStreamSource, MediaStreamComponent and MediaStreamDescriptor to oilpan's heap

Oilpan: Move MediaStreamSource, MediaStreamComponent and MediaStreamDescriptor
to oilpan's heap

- MediaStreamSource, MediaStreamComponent and MediaStreamDescriptor must be moved
to oilpan's heap in one go because their lifetime are tightly coupled.

- Removed m_descriptor->setClient(0) from MediaStream's destructor. This is OK
because MediaStreamDescriptor has a strong Member back to the Client
(i.e., MediaStream).

- Removed m_component->source()->removeObserver(this) from MediaStreamTrack's
destructor. This is OK because this CL made the observers weak
(i.e., MediaStreamSource::m_observers is a hash set of weak members to
MediaStreamTrack objects).

- Removed WebMediaStreamTrack::ExtraData::m_owner because it's unused.

- Introduced MediaStreamComponentDisposer to delay the destruction of
MediaStreamComponent::m_extraData. The ExtraData is exposed to the web
and some Chromium objects inherit from the ExtraData. If we clear the
m_extraData in the MediaStreamComponent's destructor, those Chromium-side
objects are also destructed in the MediaStreamComponent's destructor.
This is problematic because the destructors of the Chromium-side classes
can touch other on-heap objects in the Blink side. To avoid the issue,
we need to delay the destruction of the ExtraData to thread-specific
weak processing. The disposer pattern actually makes the deletion of
the extra data happen earlier and not later. The disposer makes sure
that the extra data is destructed in weak processing which is run before
sweeping and therefore all the objects are still alive and can be touched.


- The same problem arises for MediaStreamDescriptor and MediaStreamSource.
This CL adds MediaStreamDescriptorDisposer and MediaStreamSourceDisposer.

- This CL depends on https://codereview.chromium.org/543603003/ and https://codereview.chromium.org/549153002.

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

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

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

[haraken, 2014-09-09 04:29:19]

haraken@chromium.org changed reviewers:
+ oilpan-reviews@chromium.org, tkent@chromium.org

[haraken, 2014-09-09 04:29:50]

I think this CL is now ready for full review. I verified that Chromium-side
tests pass with this CL. PTAL.

[Mads Ager (chromium), 2014-09-09 14:14:26]

ager@chromium.org changed reviewers:
+ ager@chromium.org

[Mads Ager (chromium), 2014-09-09 14:14:27]

LGTM

Update the description: The disposer pattern actually makes the deletion of the
extra data happen earlier and not later. The disposer makes sure that the extra
data is destructed in weak processing which is run before sweeping and therefore
all the objects are still alive and can be touched.

Maybe add comments in the code about why the disposer is needed.

https://codereview.chromium.org/552653005/diff/1/Source/web/WebHeap.cpp
File Source/web/WebHeap.cpp (right):

https://codereview.chromium.org/552653005/diff/1/Source/web/WebHeap.cpp#newco...
Source/web/WebHeap.cpp:48: void WebHeap::collectGarbage()
You are landing these changes in a separate change, right?

[haraken, 2014-09-10 03:27:46]

On 2014/09/09 14:14:27, Mads Ager (chromium) wrote:
> LGTM
> 
> Update the description: The disposer pattern actually makes the deletion of
the
> extra data happen earlier and not later. The disposer makes sure that the
extra
> data is destructed in weak processing which is run before sweeping and
therefore
> all the objects are still alive and can be touched.
> 
> Maybe add comments in the code about why the disposer is needed.
> 
> https://codereview.chromium.org/552653005/diff/1/Source/web/WebHeap.cpp
> File Source/web/WebHeap.cpp (right):
> 
>
https://codereview.chromium.org/552653005/diff/1/Source/web/WebHeap.cpp#newco...
> Source/web/WebHeap.cpp:48: void WebHeap::collectGarbage()
> You are landing these changes in a separate change, right?

Done.

tkent-san: Would you take a look at public/ ?

[tkent, 2014-09-10 04:25:19]

lgtm.

Please wrap the CL description in 72 columns.  You should have seen an indicator
for it when you ran "git cl upload".

[haraken, 2014-09-10 04:27:16]

> Please wrap the CL description in 72 columns.  You should have seen an
indicator
> for it when you ran "git cl upload".

Done. (I'm writing a CL description in a browser...)

The chromium-side CL was landed, so let me land this.

[haraken, 2014-09-10 04:27:20]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-09-10 04:28:15]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/haraken@chromium.org/552653005/20001

[commit-bot: I haz the power, 2014-09-10 05:49:05]

Committed patchset #2 (id:20001) as 181702

[haraken, 2014-09-22 05:42:09]

Now that the chromium side fix has been landed
(https://codereview.chromium.org/566793002/), I think it's safe to reland this
CL.

[haraken, 2014-09-22 05:42:15]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-09-22 05:42:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/552653005/40001

[commit-bot: I haz the power, 2014-09-22 07:17:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-22 07:17:22]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/28138)

[haraken, 2014-09-22 08:19:55]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-09-22 08:20:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/552653005/40001

[commit-bot: I haz the power, 2014-09-22 08:56:20]

Committed patchset #3 (id:40001) as 182399

[haraken, 2014-09-29 08:46:49]

The Chromium-side fix was landed (https://codereview.chromium.org/596923003/),
so I think it's safe to land this CL.

[haraken, 2014-09-29 08:46:53]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-09-29 08:48:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/552653005/60001

[commit-bot: I haz the power, 2014-09-29 09:52:22]

Committed patchset #4 (id:60001) as 182826

[haraken, 2014-09-30 01:09:28]

On 2014/09/29 09:52:22, I haz the power (commit-bot) wrote:
> Committed patchset #4 (id:60001) as 182826

Looks like I finally succeeded in landing this CL without causing any memory
leakage in chromium-side unittests :)