cc: Don't upload UI resources twice after eviction

cc: Don't upload UI resources twice after eviction

This can happen if you create a new UI resource (through LTH)
which queues a request while there are any evicted resources
(as tracked by LTHImpl).

RecreateUIResources() will then iterate over all existing client
ids and queue another CREATE request.

Simply check for duplicates before queuing more requests.

The double-upload behavior didn't play well with Android's
Thumbnail class which drops the bitmap after every
UIResourceClient::GetBitmap() call.

BUG=636630
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/8afa2d49f71b2a3ce36709ba69610d7d1bf85262
Cr-Commit-Position: refs/heads/master@{#414785}

[no sievers, 2016-08-13 00:17:39]

Description was changed from

==========
cc: Don't upload UI resources again after eviction

This can happen if you create a new UI resource (through LTH)
which queues a request while there are any evicted resources
(as tracked by LTHImpl).

RecreateUIResources() will then iterate over all existing client
ids and queue another CREATE request.

Simply check for duplicates before queuing more requests.

The double-upload behavior didn't play well with Android's
Thumbnail class which drops the bitmap after every
UIResourceClient::GetBitmap() call.

BUG=636630
==========

to

==========
cc: Don't upload UI resources again after eviction

This can happen if you create a new UI resource (through LTH)
which queues a request while there are any evicted resources
(as tracked by LTHImpl).

RecreateUIResources() will then iterate over all existing client
ids and queue another CREATE request.

Simply check for duplicates before queuing more requests.

The double-upload behavior didn't play well with Android's
Thumbnail class which drops the bitmap after every
UIResourceClient::GetBitmap() call.

BUG=636630
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[no sievers, 2016-08-13 00:17:45]

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

[commit-bot: I haz the power, 2016-08-13 00:18:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[no sievers, 2016-08-13 00:19:42]

Description was changed from

==========
cc: Don't upload UI resources again after eviction

This can happen if you create a new UI resource (through LTH)
which queues a request while there are any evicted resources
(as tracked by LTHImpl).

RecreateUIResources() will then iterate over all existing client
ids and queue another CREATE request.

Simply check for duplicates before queuing more requests.

The double-upload behavior didn't play well with Android's
Thumbnail class which drops the bitmap after every
UIResourceClient::GetBitmap() call.

BUG=636630
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
cc: Don't upload UI resources again after eviction

This can happen if you create a new UI resource (through LTH)
which queues a request while there are any evicted resources
(as tracked by LTHImpl).

RecreateUIResources() will then iterate over all existing client
ids and queue another CREATE request.

Simply check for duplicates before queuing more requests.

The double-upload behavior didn't play well with Android's
Thumbnail class which drops the bitmap after every
UIResourceClient::GetBitmap() call.

BUG=636630
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[no sievers, 2016-08-13 00:19:42]

sievers@chromium.org changed reviewers:
+ danakj@chromium.org

[no sievers, 2016-08-13 00:19:51]

Dana, ptal.

[no sievers, 2016-08-13 00:19:58]

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

[commit-bot: I haz the power, 2016-08-13 00:20:44]

Dry run: 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, 2016-08-13 01:22:34]

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

[commit-bot: I haz the power, 2016-08-13 01:22:35]

Dry run: This issue passed the CQ dry run.

[no sievers, 2016-08-15 17:19:41]

Description was changed from

==========
cc: Don't upload UI resources again after eviction

This can happen if you create a new UI resource (through LTH)
which queues a request while there are any evicted resources
(as tracked by LTHImpl).

RecreateUIResources() will then iterate over all existing client
ids and queue another CREATE request.

Simply check for duplicates before queuing more requests.

The double-upload behavior didn't play well with Android's
Thumbnail class which drops the bitmap after every
UIResourceClient::GetBitmap() call.

BUG=636630
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
cc: Don't upload UI resources twice after eviction

This can happen if you create a new UI resource (through LTH)
which queues a request while there are any evicted resources
(as tracked by LTHImpl).

RecreateUIResources() will then iterate over all existing client
ids and queue another CREATE request.

Simply check for duplicates before queuing more requests.

The double-upload behavior didn't play well with Android's
Thumbnail class which drops the bitmap after every
UIResourceClient::GetBitmap() call.

BUG=636630
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[danakj, 2016-08-15 22:52:11]

Would like the test to be a bit more verbose about all 3 resources. LGTM tho.

https://codereview.chromium.org/2247573002/diff/20001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host.cc (right):

https://codereview.chromium.org/2247573002/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host.cc:1193: if
(std::find_if(ui_resource_request_queue_.begin(),
Can you split this into a few lines cuz its hard to find the == in it all.

auto it = std::find_if(....);
if (it == ..end()) {
}

would be better IMO

https://codereview.chromium.org/2247573002/diff/20001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_unittest_context.cc (right):

https://codereview.chromium.org/2247573002/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_unittest_context.cc:1472: EXPECT_NE(0,
ui_resource_->id());
do this for both?

https://codereview.chromium.org/2247573002/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_unittest_context.cc:1497: EXPECT_EQ(0u,
impl->ResourceIdForUIResource(ui_resource_->id()));
do this for 2 and 3?

https://codereview.chromium.org/2247573002/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_unittest_context.cc:1512: // The resource should have
been created on LTHI after the commit.
Which resource, both right?

https://codereview.chromium.org/2247573002/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_unittest_context.cc:1514: EXPECT_NE(0u,
impl->ResourceIdForUIResource(ui_resource_->id()));
do this for both?

https://codereview.chromium.org/2247573002/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_unittest_context.cc:1521: EXPECT_EQ(0u,
impl->ResourceIdForUIResource(ui_resource_->id()));
do this for both?

https://codereview.chromium.org/2247573002/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_unittest_context.cc:1527: // The resource should have
been recreated.
Both resource and resource2 ya?

[no sievers, 2016-08-26 18:38:19]

https://codereview.chromium.org/2247573002/diff/20001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host.cc (right):

https://codereview.chromium.org/2247573002/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host.cc:1193: if
(std::find_if(ui_resource_request_queue_.begin(),
On 2016/08/15 22:52:11, danakj wrote:
> Can you split this into a few lines cuz its hard to find the == in it all.
> 
> auto it = std::find_if(....);
> if (it == ..end()) {
> }
> 
> would be better IMO

Done.

https://codereview.chromium.org/2247573002/diff/20001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_unittest_context.cc (right):

https://codereview.chromium.org/2247573002/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_unittest_context.cc:1472: EXPECT_NE(0,
ui_resource_->id());
On 2016/08/15 22:52:11, danakj wrote:
> do this for both?

Done.

https://codereview.chromium.org/2247573002/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_unittest_context.cc:1497: EXPECT_EQ(0u,
impl->ResourceIdForUIResource(ui_resource_->id()));
On 2016/08/15 22:52:11, danakj wrote:
> do this for 2 and 3?

Done.

https://codereview.chromium.org/2247573002/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_unittest_context.cc:1512: // The resource should have
been created on LTHI after the commit.
On 2016/08/15 22:52:11, danakj wrote:
> Which resource, both right?

Done.

https://codereview.chromium.org/2247573002/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_unittest_context.cc:1514: EXPECT_NE(0u,
impl->ResourceIdForUIResource(ui_resource_->id()));
On 2016/08/15 22:52:11, danakj wrote:
> do this for both?

Done.

https://codereview.chromium.org/2247573002/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_unittest_context.cc:1521: EXPECT_EQ(0u,
impl->ResourceIdForUIResource(ui_resource_->id()));
On 2016/08/15 22:52:11, danakj wrote:
> do this for both?

Done.

https://codereview.chromium.org/2247573002/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_unittest_context.cc:1527: // The resource should have
been recreated.
On 2016/08/15 22:52:11, danakj wrote:
> Both resource and resource2 ya?

Done.

[no sievers, 2016-08-26 18:39:06]

The CQ bit was checked by sievers@chromium.org

[no sievers, 2016-08-26 18:39:07]

The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org
Link to the patchset: https://codereview.chromium.org/2247573002/#ps40001
(title: "address comments")

[commit-bot: I haz the power, 2016-08-26 18:39:36]

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, 2016-08-26 19:45:35]

Description was changed from

==========
cc: Don't upload UI resources twice after eviction

This can happen if you create a new UI resource (through LTH)
which queues a request while there are any evicted resources
(as tracked by LTHImpl).

RecreateUIResources() will then iterate over all existing client
ids and queue another CREATE request.

Simply check for duplicates before queuing more requests.

The double-upload behavior didn't play well with Android's
Thumbnail class which drops the bitmap after every
UIResourceClient::GetBitmap() call.

BUG=636630
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
cc: Don't upload UI resources twice after eviction

This can happen if you create a new UI resource (through LTH)
which queues a request while there are any evicted resources
(as tracked by LTHImpl).

RecreateUIResources() will then iterate over all existing client
ids and queue another CREATE request.

Simply check for duplicates before queuing more requests.

The double-upload behavior didn't play well with Android's
Thumbnail class which drops the bitmap after every
UIResourceClient::GetBitmap() call.

BUG=636630
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[commit-bot: I haz the power, 2016-08-26 19:45:36]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-26 19:47:55]

Description was changed from

==========
cc: Don't upload UI resources twice after eviction

This can happen if you create a new UI resource (through LTH)
which queues a request while there are any evicted resources
(as tracked by LTHImpl).

RecreateUIResources() will then iterate over all existing client
ids and queue another CREATE request.

Simply check for duplicates before queuing more requests.

The double-upload behavior didn't play well with Android's
Thumbnail class which drops the bitmap after every
UIResourceClient::GetBitmap() call.

BUG=636630
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
cc: Don't upload UI resources twice after eviction

This can happen if you create a new UI resource (through LTH)
which queues a request while there are any evicted resources
(as tracked by LTHImpl).

RecreateUIResources() will then iterate over all existing client
ids and queue another CREATE request.

Simply check for duplicates before queuing more requests.

The double-upload behavior didn't play well with Android's
Thumbnail class which drops the bitmap after every
UIResourceClient::GetBitmap() call.

BUG=636630
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/8afa2d49f71b2a3ce36709ba69610d7d1bf85262
Cr-Commit-Position: refs/heads/master@{#414785}
==========

[commit-bot: I haz the power, 2016-08-26 19:47:55]

Patchset 3 (id:??) landed as
https://crrev.com/8afa2d49f71b2a3ce36709ba69610d7d1bf85262
Cr-Commit-Position: refs/heads/master@{#414785}