Insist on a Blink GC on RenderViewImpl unit test teardowns.

Insist on a Blink GC on RenderViewImpl unit test teardowns.

Without it, flaky LSan reports are a risk, depending on GC timing and
on tests allowing posted GC tasks to be processed before many of them
abruptly clear posted messages as part of their operation.

R=haraken
TBR=jochen
BUG=

Committed: https://crrev.com/ca438fc0e1e6731a6bebee4e298f8e5dd7da44fa
Cr-Commit-Position: refs/heads/master@{#371169}

[sof, 2016-01-23 21:27:32]

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

[sof, 2016-01-23 21:27:33]

please take a look.

addressing the LSan failure reported via
https://codereview.chromium.org/1611303006/#msg3 , which shows up with Oilpan
always-enabled.

It is problematic that these unit tests clear out messages from the event loop
as they run, causing posted GCs not to run.

[sof, 2016-01-23 21:30:51]

Test results (with Oilpan enabled):
https://codereview.chromium.org/1517193002/#ps220001 (linux_chromium_asan_rel_ng
, please ignore the chromeos ASan build as it is generally failing atm)

[haraken, 2016-01-24 09:27:56]

LGTM

Maybe we can TBR jochen and land this. (Given the timeline, we want to get to a
state where we can enable Oilpan on ToT by the end of Mon JST.)

https://codereview.chromium.org/1621823004/diff/1/content/renderer/render_vie...
File content/renderer/render_view_browsertest.cc (right):

https://codereview.chromium.org/1621823004/diff/1/content/renderer/render_vie...
content/renderer/render_view_browsertest.cc:179:
WebHeap::collectGarbageForTesting();

collectAllGarbageForTesting would be better.

[sof, 2016-01-24 12:33:30]

Description was changed from

==========
Insist on a Blink GC on RenderViewImpl unit test teardowns.

Without it, flaky LSan reports are a risk, depending on GC timing and
on tests allowing posted GC tasks to be processed before many of them
abruptly clear posted messages as part of their operation.

R=
BUG=
==========

to

==========
Insist on a Blink GC on RenderViewImpl unit test teardowns.

Without it, flaky LSan reports are a risk, depending on GC timing and
on tests allowing posted GC tasks to be processed before many of them
abruptly clear posted messages as part of their operation.

R=haraken
TBR=jochen
BUG=
==========

[sof, 2016-01-24 12:35:18]

ok, given the previous https://codereview.chromium.org/1495923002 a TBR wouldn't
be taking unbounded amounts of liberties (I hope.) Will followup on feedback, of
course.

https://codereview.chromium.org/1621823004/diff/1/content/renderer/render_vie...
File content/renderer/render_view_browsertest.cc (right):

https://codereview.chromium.org/1621823004/diff/1/content/renderer/render_vie...
content/renderer/render_view_browsertest.cc:179:
WebHeap::collectGarbageForTesting();
On 2016/01/24 09:27:56, haraken wrote:
> 
> collectAllGarbageForTesting would be better.

That's what I thought would be tidier too, but forcing a (conservative) GC is
vital here (and sufficient for these tests.)

*AllGarbageForTesting()'s posting of tasks to run precise GCs along with
processing pending messages afterwards isn't a strong enough guarantee -- you
can still observe flaky leak reports when doing that.

So I will keep it like this.

[sof, 2016-01-24 13:14:49]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2016-01-24 13:15:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1621823004/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1621823004/1

[commit-bot: I haz the power, 2016-01-24 13:21:30]

Description was changed from

==========
Insist on a Blink GC on RenderViewImpl unit test teardowns.

Without it, flaky LSan reports are a risk, depending on GC timing and
on tests allowing posted GC tasks to be processed before many of them
abruptly clear posted messages as part of their operation.

R=haraken
TBR=jochen
BUG=
==========

to

==========
Insist on a Blink GC on RenderViewImpl unit test teardowns.

Without it, flaky LSan reports are a risk, depending on GC timing and
on tests allowing posted GC tasks to be processed before many of them
abruptly clear posted messages as part of their operation.

R=haraken
TBR=jochen
BUG=
==========

[commit-bot: I haz the power, 2016-01-24 13:21:31]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-01-24 13:22:10]

Description was changed from

==========
Insist on a Blink GC on RenderViewImpl unit test teardowns.

Without it, flaky LSan reports are a risk, depending on GC timing and
on tests allowing posted GC tasks to be processed before many of them
abruptly clear posted messages as part of their operation.

R=haraken
TBR=jochen
BUG=
==========

to

==========
Insist on a Blink GC on RenderViewImpl unit test teardowns.

Without it, flaky LSan reports are a risk, depending on GC timing and
on tests allowing posted GC tasks to be processed before many of them
abruptly clear posted messages as part of their operation.

R=haraken
TBR=jochen
BUG=

Committed: https://crrev.com/ca438fc0e1e6731a6bebee4e298f8e5dd7da44fa
Cr-Commit-Position: refs/heads/master@{#371169}
==========

[commit-bot: I haz the power, 2016-01-24 13:22:11]

Patchset 1 (id:??) landed as
https://crrev.com/ca438fc0e1e6731a6bebee4e298f8e5dd7da44fa
Cr-Commit-Position: refs/heads/master@{#371169}

[sof, 2016-01-28 12:39:39]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/1649753002/ by sigbjornf@opera.com.

The reason for reverting is: Rolling this one out, going with
https://codereview.chromium.org/1645923002/ 's approach instead..