Refactor ResourcePool::AcquireResource() and add test.

Refactor ResourcePool::AcquireResource() and add test.

This CL has no intended functional effect.

Separate AcquireResource() into the functions ReuseResource() and
CreateResource() and write a test.

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

Committed: https://crrev.com/0bada44ad4dc13728a59e89a596dacb763bbb268
Cr-Commit-Position: refs/heads/master@{#411998}

[erikchen, 2016-08-12 20:58:56]

Description was changed from

==========
Refactor ResourcePool::AcquireResource().

This CL has no intended functional effect.

Separate AcquireResource() into the functions ReuseResource() and
CreateResource() in preparation for resource reuse with looser size constraints.

BUG=581526
==========

to

==========
Refactor ResourcePool::AcquireResource().

This CL has no intended functional effect.

Separate AcquireResource() into the functions ReuseResource() and
CreateResource() in preparation for resource reuse with looser size constraints.

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

[erikchen, 2016-08-12 21:02:49]

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

[erikchen, 2016-08-12 21:02:59]

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

[erikchen, 2016-08-12 21:03:00]

ccameron: Please review.

[commit-bot: I haz the power, 2016-08-12 21:03:18]

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-12 21:05:31]

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

[commit-bot: I haz the power, 2016-08-12 21:05:32]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[ccameron, 2016-08-12 21:22:39]

Could you send out a patch with the full proposed change (in particular, how
GLRenderer is planning to use this).

I feel odd about this interface -- I may prefer something closer to just a "do
some sort of flush when you can't recreate or whatever" flag to ResourcePool.

[erikchen, 2016-08-12 22:14:20]

On 2016/08/12 21:22:39, ccameron wrote:
> Could you send out a patch with the full proposed change (in particular, how
> GLRenderer is planning to use this).
> 
> I feel odd about this interface -- I may prefer something closer to just a "do
> some sort of flush when you can't recreate or whatever" flag to ResourcePool.

GLRenderer changes: https://codereview.chromium.org/2245813002/

Depend CLs: This one and https://codereview.chromium.org/2240993002/.

[erikchen, 2016-08-12 22:14:26]

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

[commit-bot: I haz the power, 2016-08-12 22:14:53]

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-12 23:12:48]

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

[commit-bot: I haz the power, 2016-08-12 23:12:49]

Dry run: This issue passed the CQ dry run.

[ccameron, 2016-08-15 08:25:00]

On 2016/08/12 22:14:20, erikchen wrote:
> On 2016/08/12 21:22:39, ccameron wrote:
> > Could you send out a patch with the full proposed change (in particular, how
> > GLRenderer is planning to use this).
> > 
> > I feel odd about this interface -- I may prefer something closer to just a
"do
> > some sort of flush when you can't recreate or whatever" flag to
ResourcePool.
> 
> GLRenderer changes: https://codereview.chromium.org/2245813002/
> 
> Depend CLs: This one and https://codereview.chromium.org/2240993002/.

I see. So, back in the distant past, we rounded all IOSurface allocations up to
the nearest 64x64 pixels to avoid assorted badness (see
https://src.chromium.org/viewvc/chrome?revision=155928&view=revision).

If we just always rounded the width and height of updated_dst_rect to the next
multiple of 64, we'd automatically get this reuse for free, and we wouldn't have
to worry about changing the ResourcePool interface. Does that sound good?

[erikchen, 2016-08-15 16:49:29]

Description was changed from

==========
Refactor ResourcePool::AcquireResource().

This CL has no intended functional effect.

Separate AcquireResource() into the functions ReuseResource() and
CreateResource() in preparation for resource reuse with looser size constraints.

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

to

==========
Refactor ResourcePool::AcquireResource() and add test.

This CL has no intended functional effect.

Separate AcquireResource() into the functions ReuseResource() and
CreateResource() and write a test.

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

[erikchen, 2016-08-15 16:49:58]

Description was changed from

==========
Refactor ResourcePool::AcquireResource() and add test.

This CL has no intended functional effect.

Separate AcquireResource() into the functions ReuseResource() and
CreateResource() and write a test.

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

to

==========
Refactor ResourcePool::AcquireResource() and add test.

This CL has no intended functional effect.

Separate AcquireResource() into the functions ReuseResource() and
CreateResource() and write a test.

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

[erikchen, 2016-08-15 16:50:09]

Description was changed from

==========
Refactor ResourcePool::AcquireResource() and add test.

This CL has no intended functional effect.

Separate AcquireResource() into the functions ReuseResource() and
CreateResource() and write a test.

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

to

==========
Refactor ResourcePool::AcquireResource() and add test.

This CL has no intended functional effect.

Separate AcquireResource() into the functions ReuseResource() and
CreateResource() and write a test.

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

[erikchen, 2016-08-15 16:50:13]

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

[commit-bot: I haz the power, 2016-08-15 16:50:44]

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

[erikchen, 2016-08-15 16:51:14]

On 2016/08/15 08:25:00, ccameron (slow til Sept 1) wrote:
> On 2016/08/12 22:14:20, erikchen wrote:
> > On 2016/08/12 21:22:39, ccameron wrote:
> > > Could you send out a patch with the full proposed change (in particular,
how
> > > GLRenderer is planning to use this).
> > > 
> > > I feel odd about this interface -- I may prefer something closer to just a
> "do
> > > some sort of flush when you can't recreate or whatever" flag to
> ResourcePool.
> > 
> > GLRenderer changes: https://codereview.chromium.org/2245813002/
> > 
> > Depend CLs: This one and https://codereview.chromium.org/2240993002/.
> 
> I see. So, back in the distant past, we rounded all IOSurface allocations up
to
> the nearest 64x64 pixels to avoid assorted badness (see
> https://src.chromium.org/viewvc/chrome?revision=155928&view=revision).
> 
> If we just always rounded the width and height of updated_dst_rect to the next
> multiple of 64, we'd automatically get this reuse for free, and we wouldn't
have
> to worry about changing the ResourcePool interface. Does that sound good?

Yup, will do. Please take another look at this CL - I made the refactored
methods private, and added a test. Given that this test caught a real bug, I
still think we should land it.

[commit-bot: I haz the power, 2016-08-15 17:21:51]

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

[commit-bot: I haz the power, 2016-08-15 17:21:52]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[erikchen, 2016-08-15 17:25:33]

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

[commit-bot: I haz the power, 2016-08-15 17:25:54]

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-15 18:20:03]

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

[commit-bot: I haz the power, 2016-08-15 18:20:04]

Dry run: This issue passed the CQ dry run.

[ccameron, 2016-08-15 18:27:23]

lgtm

[erikchen, 2016-08-15 18:30:17]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2016-08-15 18:30:46]

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-15 18:38:14]

Description was changed from

==========
Refactor ResourcePool::AcquireResource() and add test.

This CL has no intended functional effect.

Separate AcquireResource() into the functions ReuseResource() and
CreateResource() and write a test.

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

to

==========
Refactor ResourcePool::AcquireResource() and add test.

This CL has no intended functional effect.

Separate AcquireResource() into the functions ReuseResource() and
CreateResource() and write a test.

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

[commit-bot: I haz the power, 2016-08-15 18:38:15]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-08-15 18:41:42]

Description was changed from

==========
Refactor ResourcePool::AcquireResource() and add test.

This CL has no intended functional effect.

Separate AcquireResource() into the functions ReuseResource() and
CreateResource() and write a test.

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

to

==========
Refactor ResourcePool::AcquireResource() and add test.

This CL has no intended functional effect.

Separate AcquireResource() into the functions ReuseResource() and
CreateResource() and write a test.

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

Committed: https://crrev.com/0bada44ad4dc13728a59e89a596dacb763bbb268
Cr-Commit-Position: refs/heads/master@{#411998}
==========

[commit-bot: I haz the power, 2016-08-15 18:41:43]

Patchset 6 (id:??) landed as
https://crrev.com/0bada44ad4dc13728a59e89a596dacb763bbb268
Cr-Commit-Position: refs/heads/master@{#411998}