Oilpan: Expose Heap::collectGarbage to the web layer

Oilpan: Expose Heap::collectGarbage to the web layer

Heap::collectGarbage is needed in media_stream_impl_unittest.

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

[haraken, 2014-09-04 12:09:07]

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

[haraken, 2014-09-04 12:09:08]

PTAL

[haraken, 2014-09-04 12:10:12]

More detailed explanation for why we need WebHeap::collectGarbage is written
here: https://codereview.chromium.org/509933002/

[Mads Ager (chromium), 2014-09-04 12:13:55]

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

[Mads Ager (chromium), 2014-09-04 12:13:55]

LGTM

https://codereview.chromium.org/543603003/diff/1/public/web/WebHeap.h
File public/web/WebHeap.h (right):

https://codereview.chromium.org/543603003/diff/1/public/web/WebHeap.h#newcode51
public/web/WebHeap.h:51: static void collectGarbage();
Could you add a comment that these are only exposed for testing purposes and
should not be used outside of tests?

[haraken, 2014-09-04 12:15:51]

https://codereview.chromium.org/543603003/diff/1/public/web/WebHeap.h
File public/web/WebHeap.h (right):

https://codereview.chromium.org/543603003/diff/1/public/web/WebHeap.h#newcode51
public/web/WebHeap.h:51: static void collectGarbage();
On 2014/09/04 12:13:55, Mads Ager (chromium) wrote:
> Could you add a comment that these are only exposed for testing purposes and
> should not be used outside of tests?

Done.

[jochen (gone - plz use gerrit), 2014-09-04 13:33:46]

https://codereview.chromium.org/543603003/diff/20001/public/web/WebHeap.h
File public/web/WebHeap.h (right):

https://codereview.chromium.org/543603003/diff/20001/public/web/WebHeap.h#new...
public/web/WebHeap.h:53: static void collectGarbage();
those APIs should all end in ForTests for ForTesting.

[haraken, 2014-09-04 13:44:00]

On 2014/09/04 13:33:46, jochen wrote:
> https://codereview.chromium.org/543603003/diff/20001/public/web/WebHeap.h
> File public/web/WebHeap.h (right):
> 
>
https://codereview.chromium.org/543603003/diff/20001/public/web/WebHeap.h#new...
> public/web/WebHeap.h:53: static void collectGarbage();
> those APIs should all end in ForTests for ForTesting.

Thanks for review.

I noticed that chromium-side unittests about MediaStream fail with various
reasons if I move MediaStreamSource to oilpan. I want to first make sure that
exposing the WebHeap::collectGarbageForTesting fix all the failures, so please
hold on review :)

[haraken, 2014-09-08 13:48:32]

On 2014/09/04 13:44:00, haraken wrote:
> On 2014/09/04 13:33:46, jochen wrote:
> > https://codereview.chromium.org/543603003/diff/20001/public/web/WebHeap.h
> > File public/web/WebHeap.h (right):
> > 
> >
>
https://codereview.chromium.org/543603003/diff/20001/public/web/WebHeap.h#new...
> > public/web/WebHeap.h:53: static void collectGarbage();
> > those APIs should all end in ForTests for ForTesting.
> 
> Thanks for review.
> 
> I noticed that chromium-side unittests about MediaStream fail with various
> reasons if I move MediaStreamSource to oilpan. I want to first make sure that
> exposing the WebHeap::collectGarbageForTesting fix all the failures, so please
> hold on review :)

OK, I verified that WebHeap::collectGarbage APIs solve the crashes in
MediaStream-related content_unittests.

Chromium-side CL: https://codereview.chromium.org/549153002
Blink-side CL: https://codereview.chromium.org/509933002/

So, jochen@, would you approve this CL to land? I renamed
WebHeap::collectGarbage to WebHeap::collectGarbageForTesting.

[haraken, 2014-09-09 04:31:39]

jochen@: Would you take a look at this one?

[tkent, 2014-09-09 06:45:03]

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

[tkent, 2014-09-09 06:45:04]

https://codereview.chromium.org/543603003/diff/60001/public/web/WebHeap.h
File public/web/WebHeap.h (right):

https://codereview.chromium.org/543603003/diff/60001/public/web/WebHeap.h#new...
public/web/WebHeap.h:53: static void collectGarbageForTesting();
Needs BLINK_EXPORT

https://codereview.chromium.org/543603003/diff/60001/public/web/WebHeap.h#new...
public/web/WebHeap.h:54: static void collectAllGarbageForTesting();
Ditto.

[haraken, 2014-09-09 06:46:17]

https://codereview.chromium.org/543603003/diff/60001/public/web/WebHeap.h
File public/web/WebHeap.h (right):

https://codereview.chromium.org/543603003/diff/60001/public/web/WebHeap.h#new...
public/web/WebHeap.h:53: static void collectGarbageForTesting();
On 2014/09/09 06:45:04, tkent (overloaded) wrote:
> Needs BLINK_EXPORT

Done.

[tkent, 2014-09-09 06:48:11]

lgtm

[tkent, 2014-09-09 06:48:15]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2014-09-09 06:48:23]

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

[commit-bot: I haz the power, 2014-09-09 07:48:58]

Committed patchset #5 (id:80001) as 181615

[jochen (gone - plz use gerrit), 2014-09-09 08:13:46]

lgtm, thx