Fix latency calculation when multiple rendered buffers are in flight.

Fix latency calculation when multiple rendered buffers are in flight.

This is particularly important on ChromeOS with freon stack.
SwapBuffers ack is sent when the buffers actually swapped, not when the
call to swap them was made. This changed the scheduler behavior and delayed
beginning of the next frame, so to regain it we allowed multiple in flight
frames. The effect this has on latency info is that two frames arriving in
rapid succession would override incorrect latency info making it look like
things ultimately happened earlier than they did.

This patch associates latency info with a particular frame's callback
thus avoiding it being overridden.

BUG=485302
Committed: https://crrev.com/569a70b0d222c25fa312abc21ab11e501489c22e
Cr-Commit-Position: refs/heads/master@{#329936}

[alexst (slow to review), 2015-05-14 18:30:57]

Patchset #1 (id:1) has been deleted

[alexst (slow to review), 2015-05-14 18:32:01]

alexst@chromium.org changed reviewers:
+ brianderson@chromium.org

[brianderson, 2015-05-14 19:06:11]

brianderson@chromium.org changed reviewers:
+ miletus@chromium.org

[brianderson, 2015-05-14 19:06:12]

+miletus: Do we care about the extra vector copies?

https://codereview.chromium.org/1128323006/diff/20001/content/common/gpu/imag...
File content/common/gpu/image_transport_surface.cc (right):

https://codereview.chromium.org/1128323006/diff/20001/content/common/gpu/imag...
content/common/gpu/image_transport_surface.cc:227:
latency_info.swap(latency_info_);
Although this swap avoids one copy, I think there are still two copies after
this: once on Bind and again on SwapBuffersCallback. It would be nice to avoid
those copies since we're trying to keep the overhead of LatencyInfo low.

Since std::move isn't allowed, could we instead do:
std::vector<ui::LatencyInfo>* latency_info_ptr = new
std::vector<ui::LatencyInfo>;
latency_info_ptr->swap(latency_info_);

...Bind(..., Owned(latency_info_ptr));


Or scoped_ptr<vector<...>> and Passed(latency_info)?

[alexst (slow to review), 2015-05-14 19:32:41]

PTAL

https://codereview.chromium.org/1128323006/diff/20001/content/common/gpu/imag...
File content/common/gpu/image_transport_surface.cc (right):

https://codereview.chromium.org/1128323006/diff/20001/content/common/gpu/imag...
content/common/gpu/image_transport_surface.cc:227:
latency_info.swap(latency_info_);
On 2015/05/14 19:06:12, brianderson wrote:
> Although this swap avoids one copy, I think there are still two copies after
> this: once on Bind and again on SwapBuffersCallback. It would be nice to avoid
> those copies since we're trying to keep the overhead of LatencyInfo low.
> 
> Since std::move isn't allowed, could we instead do:
> std::vector<ui::LatencyInfo>* latency_info_ptr = new
> std::vector<ui::LatencyInfo>;
> latency_info_ptr->swap(latency_info_);
> 
> ...Bind(..., Owned(latency_info_ptr));
> 
> 
> Or scoped_ptr<vector<...>> and Passed(latency_info)?

Sounds good, done.

[brianderson, 2015-05-14 19:36:58]

lgtm

[Yufeng Shen (Slow to review), 2015-05-14 20:10:29]

On 2015/05/14 19:36:58, brianderson wrote:
> lgtm

lgtm

[alexst (slow to review), 2015-05-14 20:11:24]

alexst@chromium.org changed reviewers:
+ ccameron@chromium.org

[alexst (slow to review), 2015-05-14 20:11:36]

+ccameron for owners

[ccameron, 2015-05-14 20:27:47]

lgtm

[alexst (slow to review), 2015-05-14 20:29:49]

The CQ bit was checked by alexst@chromium.org

[commit-bot: I haz the power, 2015-05-14 20:30:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1128323006/40001

[commit-bot: I haz the power, 2015-05-14 21:31:32]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2015-05-14 21:33:10]

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