cc: Save correct frame begin time to FrameRateCounter

cc: Save correct frame begin time to FrameRateCounter

This change passes the frame begin time from RenderWidget::DoDeferredUpdate()
in single threaded compositing and the vsync tick time from cc::FrameRateController
in threaded compositing into the FrameRateCounter.

BUG=


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

[jamesr, 2013-03-13 01:34:18]

Nat should look at cc/
content/ and webkit/ lgtm.
You'll also need an OWNER for ui/compositor/ once Nat is happy (I'd suggest
piman@)

[nduca, 2013-03-13 05:53:28]

this looks really good

some nits, wdyt?

https://codereview.chromium.org/12804006/diff/1/cc/layer_tree_host_impl.h
File cc/layer_tree_host_impl.h (right):

https://codereview.chromium.org/12804006/diff/1/cc/layer_tree_host_impl.h#new...
cc/layer_tree_host_impl.h:113: virtual void animate(base::TimeTicks
monotonicTime, base::Time wallClockTime);
should we clarify these to be frameBeginTime as well? They actually are supposed
to be, I think.

https://codereview.chromium.org/12804006/diff/1/cc/scheduler.cc
File cc/scheduler.cc (right):

https://codereview.chromium.org/12804006/diff/1/cc/scheduler.cc#newcode153
cc/scheduler.cc:153: base::TimeTicks Scheduler::lastDrawTime()
Any reason not to call this lastVSyncTickTime or something like that?

https://codereview.chromium.org/12804006/diff/1/cc/thread_proxy.cc
File cc/thread_proxy.cc (right):

https://codereview.chromium.org/12804006/diff/1/cc/thread_proxy.cc#newcode848
cc/thread_proxy.cc:848: bool drawFrame = m_layerTreeHostImpl->canDraw() &&
(m_layerTreeHostImpl->prepareToDraw(frame) || forcedDraw);
i think we should pass the lastVsynctime to animate as well...


can you also fix the routing in threadproxy for beginFrameAndCommit's timestamp?
that should also be lastVSyncTime instead of now...

[egraether, 2013-03-13 22:26:01]

I did the one renaming, but your other comments look more like separate patches
to me. Also I have never worked in these code paths before.

https://codereview.chromium.org/12804006/diff/1/cc/layer_tree_host_impl.h
File cc/layer_tree_host_impl.h (right):

https://codereview.chromium.org/12804006/diff/1/cc/layer_tree_host_impl.h#new...
cc/layer_tree_host_impl.h:113: virtual void animate(base::TimeTicks
monotonicTime, base::Time wallClockTime);
On 2013/03/13 05:53:28, nduca wrote:
> should we clarify these to be frameBeginTime as well? They actually are
supposed
> to be, I think.

This naming goes through some files and I'm not sure why this name was chosen.
If you want me to change this, I would rather do it in a separate patch.

https://codereview.chromium.org/12804006/diff/1/cc/scheduler.cc
File cc/scheduler.cc (right):

https://codereview.chromium.org/12804006/diff/1/cc/scheduler.cc#newcode153
cc/scheduler.cc:153: base::TimeTicks Scheduler::lastDrawTime()
On 2013/03/13 05:53:28, nduca wrote:
> Any reason not to call this lastVSyncTickTime or something like that?

Done.

https://codereview.chromium.org/12804006/diff/1/cc/thread_proxy.cc
File cc/thread_proxy.cc (right):

https://codereview.chromium.org/12804006/diff/1/cc/thread_proxy.cc#newcode829
cc/thread_proxy.cc:829: base::TimeTicks monotonicTime = base::TimeTicks::Now();
There's another time here that could be replaced by lastVSyncTime, but I'm not
sure where this is going either.

https://codereview.chromium.org/12804006/diff/1/cc/thread_proxy.cc#newcode848
cc/thread_proxy.cc:848: bool drawFrame = m_layerTreeHostImpl->canDraw() &&
(m_layerTreeHostImpl->prepareToDraw(frame) || forcedDraw);
On 2013/03/13 05:53:28, nduca wrote:
> i think we should pass the lastVsynctime to animate as well...
> 
> 
> can you also fix the routing in threadproxy for beginFrameAndCommit's
timestamp?
> that should also be lastVSyncTime instead of now...

I think you are right, but I'm not sure what side effects this could cause.
Maybe it's better to do this in another patch.

[nduca, 2013-03-13 22:44:05]

okay. lgtm.

[piman, 2013-03-13 22:50:51]

lgtm

[commit-bot: I haz the power, 2013-03-14 20:27:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/egraether@chromium.org/12804006/25001

[commit-bot: I haz the power, 2013-03-15 12:31:24]

Failed to apply patch for cc/test/layer_tree_test_common.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file cc/test/layer_tree_test_common.h
  Hunk #1 FAILED at 166.
  1 out of 1 hunk FAILED -- saving rejects to file
cc/test/layer_tree_test_common.h.rej

Patch:       cc/test/layer_tree_test_common.h
Index: cc/test/layer_tree_test_common.h
diff --git a/cc/test/layer_tree_test_common.h b/cc/test/layer_tree_test_common.h
index
ac9534e3d1eaa0397e58f7bf0aebc83a1e556f6f..dcbea2e1eba661deb7eeb8a2538ae55f72b6485d
100644
--- a/cc/test/layer_tree_test_common.h
+++ b/cc/test/layer_tree_test_common.h
@@ -166,7 +166,7 @@ public:
     virtual void BeginCommit() OVERRIDE;
     virtual void CommitComplete() OVERRIDE;
     virtual bool PrepareToDraw(FrameData* frame) OVERRIDE;
-    virtual void DrawLayers(FrameData* frame) OVERRIDE;
+    virtual void DrawLayers(FrameData* frame, base::TimeTicks frameBeginTime)
OVERRIDE;
     virtual bool ActivatePendingTreeIfNeeded() OVERRIDE;
     virtual bool InitializeRenderer(scoped_ptr<OutputSurface> outputSurface)
OVERRIDE;
     virtual void SetVisible(bool visible) OVERRIDE;

[commit-bot: I haz the power, 2013-03-15 16:12:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/egraether@chromium.org/12804006/39001

[commit-bot: I haz the power, 2013-03-15 19:34:55]

Change committed as 188452