cc: Make images that can't possibly fit in cache not checker.

cc: Make images that can't possibly fit in cache not checker.

This patch ensures that we don't even try to checker images that are
too big to fit into our cache.

R=khushalsagar@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2844273004
Cr-Commit-Position: refs/heads/master@{#468186}
Committed: https://chromium.googlesource.com/chromium/src/+/13ee97369ce0dea158c1862cdf48056ddac77d5d

[vmpstr, 2017-04-27 23:09:36]

Description was changed from

==========
cc: Make images that can't possibly fit in cache not checker.

This patch ensures that we don't even try to checker images that are
too big to fit into our cache.

R=khushalsagar@chromium.org
==========

to

==========
cc: Make images that can't possibly fit in cache not checker.

This patch ensures that we don't even try to checker images that are
too big to fit into our cache.

R=khushalsagar@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[vmpstr, 2017-04-27 23:10:33]

Please take a look.

[Khushal, 2017-04-28 00:12:53]

Could you add a CheckerImageTrackerTest for this as well.

https://codereview.chromium.org/2844273004/diff/1/cc/tiles/image_controller.cc
File cc/tiles/image_controller.cc (right):

https://codereview.chromium.org/2844273004/diff/1/cc/tiles/image_controller.c...
cc/tiles/image_controller.cc:135: image_cache_max_limit_bytes_ =
cache_->GetMaximumMemoryLimit();
Reset this in !cache above?

https://codereview.chromium.org/2844273004/diff/1/cc/tiles/image_controller.h
File cc/tiles/image_controller.h (right):

https://codereview.chromium.org/2844273004/diff/1/cc/tiles/image_controller.h...
cc/tiles/image_controller.h:60: return image_cache_max_limit_bytes_;
Could this just be cache_ ? cache_->GetMaximumMemoryLimit() : 0.

https://codereview.chromium.org/2844273004/diff/1/cc/tiles/software_image_dec...
File cc/tiles/software_image_decode_cache.h (right):

https://codereview.chromium.org/2844273004/diff/1/cc/tiles/software_image_dec...
cc/tiles/software_image_decode_cache.h:311: // which is const and thread safe.
nit: Can you make |limit_bytes_| in MemoryBudget const too?

[Khushal, 2017-04-28 00:18:46]

https://codereview.chromium.org/2844273004/diff/1/cc/tiles/software_image_dec...
File cc/tiles/software_image_decode_cache.h (right):

https://codereview.chromium.org/2844273004/diff/1/cc/tiles/software_image_dec...
cc/tiles/software_image_decode_cache.h:311: // which is const and thread safe.
On 2017/04/28 00:12:53, Khushal wrote:
> nit: Can you make |limit_bytes_| in MemoryBudget const too?

Note to self: Look for colors other than green.

[vmpstr, 2017-04-28 17:18:31]

Added tests, and fixed other tests.

https://codereview.chromium.org/2844273004/diff/1/cc/tiles/image_controller.cc
File cc/tiles/image_controller.cc (right):

https://codereview.chromium.org/2844273004/diff/1/cc/tiles/image_controller.c...
cc/tiles/image_controller.cc:135: image_cache_max_limit_bytes_ =
cache_->GetMaximumMemoryLimit();
On 2017/04/28 00:12:53, Khushal wrote:
> Reset this in !cache above?

Done.

https://codereview.chromium.org/2844273004/diff/1/cc/tiles/image_controller.h
File cc/tiles/image_controller.h (right):

https://codereview.chromium.org/2844273004/diff/1/cc/tiles/image_controller.h...
cc/tiles/image_controller.h:60: return image_cache_max_limit_bytes_;
On 2017/04/28 00:12:53, Khushal wrote:
> Could this just be cache_ ? cache_->GetMaximumMemoryLimit() : 0.

GetMaximumMemoryLimit is virtual so I just want to avoid repeated calls to it
for the case where this value is expected not to change very frequently at all.

https://codereview.chromium.org/2844273004/diff/1/cc/tiles/software_image_dec...
File cc/tiles/software_image_decode_cache.h (right):

https://codereview.chromium.org/2844273004/diff/1/cc/tiles/software_image_dec...
cc/tiles/software_image_decode_cache.h:311: // which is const and thread safe.
On 2017/04/28 00:18:46, Khushal wrote:
> On 2017/04/28 00:12:53, Khushal wrote:
> > nit: Can you make |limit_bytes_| in MemoryBudget const too?
> 
> Note to self: Look for colors other than green.

Acknowledged.

[vmpstr, 2017-04-28 17:18:41]

(PTAL)

[Khushal, 2017-04-28 20:19:52]

lgtm

[vmpstr, 2017-04-28 23:03:51]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2017-04-28 23:04:40]

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-04-29 00:11:57]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493420631260230,
"parent_rev": "fc4b4d8fbe2657a7d51cef8939e68acc99fae05a", "commit_rev":
"13ee97369ce0dea158c1862cdf48056ddac77d5d"}

[commit-bot: I haz the power, 2017-04-29 00:12:52]

Description was changed from

==========
cc: Make images that can't possibly fit in cache not checker.

This patch ensures that we don't even try to checker images that are
too big to fit into our cache.

R=khushalsagar@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
cc: Make images that can't possibly fit in cache not checker.

This patch ensures that we don't even try to checker images that are
too big to fit into our cache.

R=khushalsagar@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2844273004
Cr-Commit-Position: refs/heads/master@{#468186}
Committed:
https://chromium.googlesource.com/chromium/src/+/13ee97369ce0dea158c1862cdf48...
==========

[commit-bot: I haz the power, 2017-04-29 00:12:53]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/13ee97369ce0dea158c1862cdf48...