cc: Fix UAF in g_raster_required_for_activation_delay

cc: Fix UAF in g_raster_required_for_activation_delay

g_raster_required_for_activation_delay is used after free. See bug for
details.

Fix by putting the delay object in RasterTaskGraphRunner which itself is
a leaky lazy instance.

BUG=402653
Committed: https://crrev.com/2b2b17a7bd1bbe3f5e08b3284da5c66c3e186b5d
Cr-Commit-Position: refs/heads/master@{#292083}

[boliu, 2014-08-26 19:14:02]

boliu@chromium.org changed reviewers:
+ reveman@chromium.org

[boliu, 2014-08-26 19:14:02]

The "I give up because I don't know the root cause" solution. I've updated the
bug with what I think is happening.

[reveman, 2014-08-27 00:34:37]

https://codereview.chromium.org/498553005/diff/1/cc/resources/raster_worker_p...
File cc/resources/raster_worker_pool.cc (right):

https://codereview.chromium.org/498553005/diff/1/cc/resources/raster_worker_p...
cc/resources/raster_worker_pool.cc:54: base::debug::TraceEventSyntheticDelay*
trace_event_synthetic_delay_;
I think |synthetic_delay_| is verbose enough.

https://codereview.chromium.org/498553005/diff/1/cc/resources/raster_worker_p...
cc/resources/raster_worker_pool.cc:128: base::debug::TraceEventSyntheticDelay*
delay_;
How about just using g_task_graph_runner.Get().synthetic_delay() above instead?
Technically a bit more efficient as we don't have to access
g_task_graph_runner.Get() each time we create one of these tasks and that's good
as we'll create many more of these tasks than we run.

[reveman, 2014-08-27 00:35:24]

On 2014/08/26 19:14:02, boliu wrote:
> The "I give up because I don't know the root cause" solution. I've updated the
> bug with what I think is happening.

Ack. I don't mind as I think this patch is a nice cleanup.

[boliu, 2014-08-27 01:14:49]

https://codereview.chromium.org/498553005/diff/1/cc/resources/raster_worker_p...
File cc/resources/raster_worker_pool.cc (right):

https://codereview.chromium.org/498553005/diff/1/cc/resources/raster_worker_p...
cc/resources/raster_worker_pool.cc:54: base::debug::TraceEventSyntheticDelay*
trace_event_synthetic_delay_;
On 2014/08/27 00:34:37, reveman wrote:
> I think |synthetic_delay_| is verbose enough.

Done.

https://codereview.chromium.org/498553005/diff/1/cc/resources/raster_worker_p...
cc/resources/raster_worker_pool.cc:128: base::debug::TraceEventSyntheticDelay*
delay_;
On 2014/08/27 00:34:37, reveman wrote:
> How about just using g_task_graph_runner.Get().synthetic_delay() above
instead?
> Technically a bit more efficient as we don't have to access
> g_task_graph_runner.Get() each time we create one of these tasks and that's
good
> as we'll create many more of these tasks than we run.

Done.

https://codereview.chromium.org/498553005/diff/20001/cc/resources/raster_work...
File cc/resources/raster_worker_pool.cc (right):

https://codereview.chromium.org/498553005/diff/20001/cc/resources/raster_work...
cc/resources/raster_worker_pool.cc:42: base::debug::TraceEventSyntheticDelay*
synthetic_delay() {
Removed const here. Learned today that returning a non-const member from a const
method is against the style guide.

Source: https://codereview.chromium.org/502453003#msg12

[reveman, 2014-08-27 01:16:15]

lgtm

[boliu, 2014-08-27 01:29:04]

The CQ bit was checked by boliu@chromium.org

[commit-bot: I haz the power, 2014-08-27 01:30:50]

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

[commit-bot: I haz the power, 2014-08-27 02:43:19]

Committed patchset #2 (20001) as c5dad70410179aa99ec144a368be1886ff4122a0

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

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