Limit the size of latency info storage

Limit the size of latency info storage

When there is no swaping we copy back the
latency infos from the frame back into the
stored latency info list. If this list grows
beyond a particular size we never use it
anyways. So we can just not store if the size
ever grows more than the threshold.

BUG=732979
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2966803002
Cr-Commit-Position: refs/heads/master@{#484609}
Committed: https://chromium.googlesource.com/chromium/src/+/20548e47b8881bb32efa2d9b8db489d7b20eaf39

[Navid Zolghadr, 2017-06-30 15:48:20]

Description was changed from

==========
Limit the size of latency info storage

When there is no swaping we copy back the
latency infos from the frame back into the
stored latency info list. If this list grows
beyond a particular size we never use it
anyways. So we can just not store if the size
ever grows more than the threshold.

BUG=732979
==========

to

==========
Limit the size of latency info storage

When there is no swaping we copy back the
latency infos from the frame back into the
stored latency info list. If this list grows
beyond a particular size we never use it
anyways. So we can just not store if the size
ever grows more than the threshold.

BUG=732979
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[Navid Zolghadr, 2017-06-30 15:49:25]

The CQ bit was checked by nzolghadr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-30 15:49:40]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Navid Zolghadr, 2017-06-30 16:54:16]

nzolghadr@chromium.org changed reviewers:
+ tdresser@chromium.org

[commit-bot: I haz the power, 2017-06-30 16:58:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-30 16:58:48]

Dry run: This issue passed the CQ dry run.

[tdresser, 2017-06-30 17:28:51]

https://codereview.chromium.org/2966803002/diff/1/cc/surfaces/display.cc
File cc/surfaces/display.cc (right):

https://codereview.chromium.org/2966803002/diff/1/cc/surfaces/display.cc#newc...
cc/surfaces/display.cc:345: if
(ui::LatencyInfo::Verify(frame.metadata.latency_info,
Should we add a test for this?

[Navid Zolghadr, 2017-06-30 17:45:10]

On 2017/06/30 17:28:51, tdresser wrote:
> https://codereview.chromium.org/2966803002/diff/1/cc/surfaces/display.cc
> File cc/surfaces/display.cc (right):
> 
>
https://codereview.chromium.org/2966803002/diff/1/cc/surfaces/display.cc#newc...
> cc/surfaces/display.cc:345: if
> (ui::LatencyInfo::Verify(frame.metadata.latency_info,
> Should we add a test for this?

Where do you think the test should be? I couldn't find a good place for it.

[tdresser, 2017-06-30 18:00:43]

On 2017/06/30 17:45:10, Navid Zolghadr wrote:
> On 2017/06/30 17:28:51, tdresser wrote:
> > https://codereview.chromium.org/2966803002/diff/1/cc/surfaces/display.cc
> > File cc/surfaces/display.cc (right):
> > 
> >
>
https://codereview.chromium.org/2966803002/diff/1/cc/surfaces/display.cc#newc...
> > cc/surfaces/display.cc:345: if
> > (ui::LatencyInfo::Verify(frame.metadata.latency_info,
> > Should we add a test for this?
> 
> Where do you think the test should be? I couldn't find a good place for it.

Why not display_unittest.cc?

[Navid Zolghadr, 2017-07-04 16:03:55]

The CQ bit was checked by nzolghadr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-04 16:04:13]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Navid Zolghadr, 2017-07-04 16:25:47]

ptal

https://codereview.chromium.org/2966803002/diff/1/cc/surfaces/display.cc
File cc/surfaces/display.cc (right):

https://codereview.chromium.org/2966803002/diff/1/cc/surfaces/display.cc#newc...
cc/surfaces/display.cc:345: if
(ui::LatencyInfo::Verify(frame.metadata.latency_info,
On 2017/06/30 17:28:51, tdresser wrote:
> Should we add a test for this?

I had to add a test API to the Display class. It was already one there. So I
thought that would be fine.

[tdresser, 2017-07-04 16:41:04]

Thanks - LGTM with nits.

https://codereview.chromium.org/2966803002/diff/20001/cc/surfaces/display_uni...
File cc/surfaces/display_unittest.cc (right):

https://codereview.chromium.org/2966803002/diff/20001/cc/surfaces/display_uni...
cc/surfaces/display_unittest.cc:469: // This is the same as
LatencyInfo::kMaxLAtencyInfoNumber.
kMaxLA -> kMaxLa

https://codereview.chromium.org/2966803002/diff/20001/cc/surfaces/display_uni...
cc/surfaces/display_unittest.cc:488:
display_->stored_latency_info_size_for_testing());
I think this would be a bit easier to read if you had two EXPECT statements
inside a conditional.

[Navid Zolghadr, 2017-07-04 17:15:19]

The CQ bit was checked by nzolghadr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-04 17:15:29]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Navid Zolghadr, 2017-07-04 17:16:28]

https://codereview.chromium.org/2966803002/diff/20001/cc/surfaces/display_uni...
File cc/surfaces/display_unittest.cc (right):

https://codereview.chromium.org/2966803002/diff/20001/cc/surfaces/display_uni...
cc/surfaces/display_unittest.cc:469: // This is the same as
LatencyInfo::kMaxLAtencyInfoNumber.
On 2017/07/04 16:41:04, tdresser wrote:
> kMaxLA -> kMaxLa

Done.

https://codereview.chromium.org/2966803002/diff/20001/cc/surfaces/display_uni...
cc/surfaces/display_unittest.cc:488:
display_->stored_latency_info_size_for_testing());
On 2017/07/04 16:41:04, tdresser wrote:
> I think this would be a bit easier to read if you had two EXPECT statements
> inside a conditional.
> 

Done.

[commit-bot: I haz the power, 2017-07-04 18:30:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-04 18:30:06]

Dry run: This issue passed the CQ dry run.

[Navid Zolghadr, 2017-07-04 18:33:39]

nzolghadr@chromium.org changed reviewers:
+ ajuma@chromium.org

[Navid Zolghadr, 2017-07-04 18:33:39]

ajuma@chromium.org: Please review changes in 

cc/surface/*

[ajuma, 2017-07-04 18:57:10]

Description was changed from

==========
Limit the size of latency info storage

When there is no swaping we copy back the
latency infos from the frame back into the
stored latency info list. If this list grows
beyond a particular size we never use it
anyways. So we can just not store if the size
ever grows more than the threshold.

BUG=732979
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Limit the size of latency info storage

When there is no swaping we copy back the
latency infos from the frame back into the
stored latency info list. If this list grows
beyond a particular size we never use it
anyways. So we can just not store if the size
ever grows more than the threshold.

BUG=732979
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[ajuma, 2017-07-04 18:57:11]

ajuma@chromium.org changed reviewers:
+ danakj@chromium.org
- ajuma@chromium.org

[ajuma, 2017-07-04 18:58:53]

On 2017/07/04 18:33:39, Navid Zolghadr wrote:
> mailto:ajuma@chromium.org: Please review changes in 
> 
> cc/surface/*

+danakj

I'm not the best reviewer for cc/surfaces/* changes, so passing this to Dana.

[Navid Zolghadr, 2017-07-06 14:08:49]

On 2017/07/04 18:58:53, ajuma wrote:
> On 2017/07/04 18:33:39, Navid Zolghadr wrote:
> > mailto:ajuma@chromium.org: Please review changes in 
> > 
> > cc/surface/*
> 
> +danakj
> 
> I'm not the best reviewer for cc/surfaces/* changes, so passing this to Dana.

ping

[danakj, 2017-07-06 14:28:11]

LGTM

[Navid Zolghadr, 2017-07-06 14:32:06]

The CQ bit was checked by nzolghadr@chromium.org

[Navid Zolghadr, 2017-07-06 14:32:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
tdresser@chromium.org
Link to the patchset: https://codereview.chromium.org/2966803002/#ps40001
(title: "Separate the checks for clarity")

[commit-bot: I haz the power, 2017-07-06 14:32:19]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-06 15:41:30]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1499351526434040,
"parent_rev": "afef048df9416f31f2d90535c3ca946ac3f43b80", "commit_rev":
"20548e47b8881bb32efa2d9b8db489d7b20eaf39"}

[commit-bot: I haz the power, 2017-07-06 15:41:42]

Description was changed from

==========
Limit the size of latency info storage

When there is no swaping we copy back the
latency infos from the frame back into the
stored latency info list. If this list grows
beyond a particular size we never use it
anyways. So we can just not store if the size
ever grows more than the threshold.

BUG=732979
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Limit the size of latency info storage

When there is no swaping we copy back the
latency infos from the frame back into the
stored latency info list. If this list grows
beyond a particular size we never use it
anyways. So we can just not store if the size
ever grows more than the threshold.

BUG=732979
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2966803002
Cr-Commit-Position: refs/heads/master@{#484609}
Committed:
https://chromium.googlesource.com/chromium/src/+/20548e47b8881bb32efa2d9b8db4...
==========

[commit-bot: I haz the power, 2017-07-06 15:41:43]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/20548e47b8881bb32efa2d9b8db4...