cc: Avoid expensive RenderingStats collection.

cc: Avoid expensive RenderingStats collection.

When --enable-gpu-benchmarking is not present, don't do expensive
steps in the benchmark collection (meaning don't call
base::TimeTicks::Now()).

BUG=170735
NOTRY=true

Depends on: https://bugs.webkit.org/show_bug.cgi?id=108358

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=180224

[danakj, 2013-01-30 08:00:19]

This is a minimal CL that disables expensive calls to Now() when not recording
rendering stats.

I could, instead, pass in a RenderingStats* object to TileManager, and have
everything record directly to a RenderingStats* from LayerTreeHostImpl. But
that's a larger refactor and thus more risky. WDYT?

[nduca, 2013-01-30 08:08:20]

LGTM

[danakj, 2013-01-30 19:09:51]

Updated to plumb the setting the WebLayerTreeViewSettings as cc/ can't see
content/ flags. Depends on webkit bug
https://bugs.webkit.org/show_bug.cgi?id=108358 now.

[danakj, 2013-01-30 19:10:35]

+jamesr for webkit/
+piman for content/

[jamesr, 2013-01-30 19:13:27]

https://codereview.chromium.org/12095053/diff/8001/webkit/compositor_bindings...
File webkit/compositor_bindings/web_layer_tree_view_impl.cc (right):

https://codereview.chromium.org/12095053/diff/8001/webkit/compositor_bindings...
webkit/compositor_bindings/web_layer_tree_view_impl.cc:56:
settings.recordRenderingStats = webSettings.recordRenderingStats;
this code can see command line switches - why can't you hoist the command line
flag in question to somewhere this can see?

[danakj, 2013-01-30 19:15:08]

https://codereview.chromium.org/12095053/diff/8001/webkit/compositor_bindings...
File webkit/compositor_bindings/web_layer_tree_view_impl.cc (right):

https://codereview.chromium.org/12095053/diff/8001/webkit/compositor_bindings...
webkit/compositor_bindings/web_layer_tree_view_impl.cc:56:
settings.recordRenderingStats = webSettings.recordRenderingStats;
On 2013/01/30 19:13:27, jamesr wrote:
> this code can see command line switches - why can't you hoist the command line
> flag in question to somewhere this can see?

Hm, ya I can move it from content/ to cc/. That doesn't seem as appropriate as
the flag is not a cc-only flag though? Or is there somewhere else you're
thinking?

[piman, 2013-01-30 22:04:48]

lgtm

[vmpstr, 2013-01-31 22:29:19]

https://codereview.chromium.org/12095053/diff/8001/cc/picture_pile_impl.cc
File cc/picture_pile_impl.cc (right):

https://codereview.chromium.org/12095053/diff/8001/cc/picture_pile_impl.cc#ne...
cc/picture_pile_impl.cc:117: stats->totalPixelsRasterized +=
Should this be conditional as well?

[danakj, 2013-02-01 00:46:11]

https://codereview.chromium.org/12095053/diff/8001/cc/picture_pile_impl.cc
File cc/picture_pile_impl.cc (right):

https://codereview.chromium.org/12095053/diff/8001/cc/picture_pile_impl.cc#ne...
cc/picture_pile_impl.cc:117: stats->totalPixelsRasterized +=
On 2013/01/31 22:29:19, vmpstr1 wrote:
> Should this be conditional as well?

Branching is more expensive than adding. So I'd say no.

[nduca, 2013-02-01 01:48:53]

https://codereview.chromium.org/12095053/diff/8001/cc/picture_pile_impl.cc
File cc/picture_pile_impl.cc (right):

https://codereview.chromium.org/12095053/diff/8001/cc/picture_pile_impl.cc#ne...
cc/picture_pile_impl.cc:117: stats->totalPixelsRasterized +=
@danakj: stats could be null, right?

[danakj, 2013-02-01 01:51:22]

https://codereview.chromium.org/12095053/diff/8001/cc/picture_pile_impl.cc
File cc/picture_pile_impl.cc (right):

https://codereview.chromium.org/12095053/diff/8001/cc/picture_pile_impl.cc#ne...
cc/picture_pile_impl.cc:117: stats->totalPixelsRasterized +=
On 2013/02/01 01:48:53, nduca wrote:
> @danakj: stats could be null, right?

Yaa, they pinged me offline about it too.

Rebasing this patch is insane, so going to need a re-review after.

[nduca, 2013-02-01 01:53:48]

kk i'll be around

[danakj, 2013-02-01 02:26:29]

PTAL

Please don't be frightened, almost all of this patch is s/&/*/ so that we don't
have to pass around bools so much and just rely on the stats being present or
not.

[nduca, 2013-02-01 02:39:56]

lgtm

[jamesr, 2013-02-01 02:41:56]

webkit/ lgtm

[commit-bot: I haz the power, 2013-02-01 03:03:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/12095053/22001

[commit-bot: I haz the power, 2013-02-01 18:49:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/12095053/22001

[commit-bot: I haz the power, 2013-02-01 23:16:39]

Retried try job too often on win_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-02-02 00:00:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/12095053/22001

[danakj, 2013-02-02 01:02:05]

NOTRY=true to get past win_rel browser tests. All other bots were green for a
run.

[commit-bot: I haz the power, 2013-02-02 01:04:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/12095053/22001

[commit-bot: I haz the power, 2013-02-02 01:10:32]

Change committed as 180224