cc: Refactor OneCopyTileTaskWorkerPool.

cc: Refactor OneCopyTileTaskWorkerPool.

Move the staging buffer pool related functionalities out to its own
file. To isolate the usage of |lock_| from OneCopyTileTaskWorkerPool,
the PlaybackAndCopyOnWorkerThread() is splitted into few other
functions.

  PlaybackAndCopyOnWorkerThread() {
    StagingBufferPool::AcquireStagingBuffer(); // with |lock_|
    PlaybackToStagingBuffer(); // without |lock_|
    CopyOnWorkerThread(); // without |lock_|, with context lock
    StagingBufferPool::ReleaseStagingBuffer(); // with |lock_|
  }

BUG=599863
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/a9dd9effc06a05b417255fe97afd92c3f32d963b
Cr-Commit-Position: refs/heads/master@{#386170}

[prashant.n, 2016-04-05 12:51:14]

Description was changed from

==========
wip raster buffer refactor

BUG=
==========

to

==========
wip raster buffer refactor

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[prashant.n, 2016-04-05 12:53:43]

Description was changed from

==========
wip raster buffer refactor

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
wip raster buffer refactor

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[prashant.n, 2016-04-05 12:53:43]

prashant.n@samsung.com changed reviewers:
+ ericrk@chromium.org, reveman@chromium.org, vmpstr@chromium.org

[prashant.n, 2016-04-05 12:55:30]

Kindly give opinion on this.

[prashant.n, 2016-04-06 06:27:26]

Description was changed from

==========
wip raster buffer refactor

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc: Refactor OneCopyTileTaskWorkerPool.

Move the staging buffer pool related functionality out to its own file.
To isolate the usage of |lock_| from OneCopyTileTaskWorkerPool, the
PlaybackAndCopyOnWorkerThread() is splitted into few other functions.

  PlaybackAndCopyOnWorkerThread() {
    StagingBufferPool::AcquireStagingBuffer(); // with |lock_|
    PlaybackToStagingBuffer(); // without |lock_|
    CopyOnWorkerThread(); // without |lock_|
    StagingBufferPool::ReleaseStagingBuffer(); // with |lock_|
  }

BUG=599863
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[prashant.n, 2016-04-06 06:31:00]

ptal

[prashant.n, 2016-04-06 10:46:48]

Description was changed from

==========
cc: Refactor OneCopyTileTaskWorkerPool.

Move the staging buffer pool related functionality out to its own file.
To isolate the usage of |lock_| from OneCopyTileTaskWorkerPool, the
PlaybackAndCopyOnWorkerThread() is splitted into few other functions.

  PlaybackAndCopyOnWorkerThread() {
    StagingBufferPool::AcquireStagingBuffer(); // with |lock_|
    PlaybackToStagingBuffer(); // without |lock_|
    CopyOnWorkerThread(); // without |lock_|
    StagingBufferPool::ReleaseStagingBuffer(); // with |lock_|
  }

BUG=599863
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc: Refactor OneCopyTileTaskWorkerPool.

Move the staging buffer pool related functionalities out to its own
file. To isolate the usage of |lock_| from OneCopyTileTaskWorkerPool,
the PlaybackAndCopyOnWorkerThread() is splitted into few other
functions.

  PlaybackAndCopyOnWorkerThread() {
    StagingBufferPool::AcquireStagingBuffer(); // with |lock_|
    PlaybackToStagingBuffer(); // without |lock_|
    CopyOnWorkerThread(); // without |lock_|
    StagingBufferPool::ReleaseStagingBuffer(); // with |lock_|
  }

BUG=599863
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[vmpstr, 2016-04-06 17:53:01]

This looks fine to me, but one of reveman or ericrk should take a look as well.

https://codereview.chromium.org/1861623003/diff/20001/cc/raster/one_copy_tile...
File cc/raster/one_copy_tile_task_worker_pool.h (right):

https://codereview.chromium.org/1861623003/diff/20001/cc/raster/one_copy_tile...
cc/raster/one_copy_tile_task_worker_pool.h:62: uint64_t previous_content_id,
Thanks!

https://codereview.chromium.org/1861623003/diff/20001/cc/raster/one_copy_tile...
cc/raster/one_copy_tile_task_worker_pool.h:89: uint64_t resource_content_id,
Let's keep the order consistent: previous, then new.

[ericrk, 2016-04-07 19:53:48]

lgtm

Seems reasonable to me. Just one nit.

https://codereview.chromium.org/1861623003/diff/20001/cc/raster/one_copy_tile...
File cc/raster/one_copy_tile_task_worker_pool.cc (right):

https://codereview.chromium.org/1861623003/diff/20001/cc/raster/one_copy_tile...
cc/raster/one_copy_tile_task_worker_pool.cc:353: // Increment
|bytes_scheduled_since_last_flush_| by the amount of memory
This used to be guarded by lock_ - the usage is still safe, as we take the
context provider lock, but this usage is now guarded by a new lock. Can you add
comments to the header to indicate that the context lock must be held before
accessing this variable.

[prashant.n, 2016-04-08 16:33:16]

https://codereview.chromium.org/1861623003/diff/20001/cc/raster/one_copy_tile...
File cc/raster/one_copy_tile_task_worker_pool.cc (right):

https://codereview.chromium.org/1861623003/diff/20001/cc/raster/one_copy_tile...
cc/raster/one_copy_tile_task_worker_pool.cc:353: // Increment
|bytes_scheduled_since_last_flush_| by the amount of memory
On 2016/04/07 19:53:47, ericrk wrote:
> This used to be guarded by lock_ - the usage is still safe, as we take the
> context provider lock, 

Due to this I removed dependency of lock.

but this usage is now guarded by a new lock. Can you add
> comments to the header to indicate that the context lock must be held before
> accessing this variable.

Yes. I'll add.

[prashant.n, 2016-04-08 16:58:54]

https://codereview.chromium.org/1861623003/diff/20001/cc/raster/staging_buffe...
File cc/raster/staging_buffer_pool.cc (right):

https://codereview.chromium.org/1861623003/diff/20001/cc/raster/staging_buffe...
cc/raster/staging_buffer_pool.cc:5: #include "cc/raster/staging_buffer_pool.h"
Should this be in cc/resources OR
cc/raster is okay?

[prashant.n, 2016-04-08 17:40:02]

Description was changed from

==========
cc: Refactor OneCopyTileTaskWorkerPool.

Move the staging buffer pool related functionalities out to its own
file. To isolate the usage of |lock_| from OneCopyTileTaskWorkerPool,
the PlaybackAndCopyOnWorkerThread() is splitted into few other
functions.

  PlaybackAndCopyOnWorkerThread() {
    StagingBufferPool::AcquireStagingBuffer(); // with |lock_|
    PlaybackToStagingBuffer(); // without |lock_|
    CopyOnWorkerThread(); // without |lock_|
    StagingBufferPool::ReleaseStagingBuffer(); // with |lock_|
  }

BUG=599863
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc: Refactor OneCopyTileTaskWorkerPool.

Move the staging buffer pool related functionalities out to its own
file. To isolate the usage of |lock_| from OneCopyTileTaskWorkerPool,
the PlaybackAndCopyOnWorkerThread() is splitted into few other
functions.

  PlaybackAndCopyOnWorkerThread() {
    StagingBufferPool::AcquireStagingBuffer(); // with |lock_|
    PlaybackToStagingBuffer(); // without |lock_|
    CopyOnWorkerThread(); // without |lock_|, with context lock
    StagingBufferPool::ReleaseStagingBuffer(); // with |lock_|
  }

BUG=599863
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[prashant.n, 2016-04-08 17:47:53]

The CQ bit was checked by prashant.n@samsung.com

[prashant.n, 2016-04-08 17:47:54]

The patchset sent to the CQ was uploaded after l-g-t-m from ericrk@chromium.org
Link to the patchset: https://codereview.chromium.org/1861623003/#ps60001
(title: "nit")

[commit-bot: I haz the power, 2016-04-08 17:48:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1861623003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1861623003/60001

[commit-bot: I haz the power, 2016-04-08 17:51:11]

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

[commit-bot: I haz the power, 2016-04-08 17:51:12]

Try jobs failed on following builders:
  ios_dbg_simulator_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_gn...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[prashant.n, 2016-04-08 18:24:55]

The CQ bit was checked by prashant.n@samsung.com

[prashant.n, 2016-04-08 18:24:55]

The patchset sent to the CQ was uploaded after l-g-t-m from ericrk@chromium.org
Link to the patchset: https://codereview.chromium.org/1861623003/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2016-04-08 18:25:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1861623003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1861623003/80001

[commit-bot: I haz the power, 2016-04-08 19:57:39]

Description was changed from

==========
cc: Refactor OneCopyTileTaskWorkerPool.

Move the staging buffer pool related functionalities out to its own
file. To isolate the usage of |lock_| from OneCopyTileTaskWorkerPool,
the PlaybackAndCopyOnWorkerThread() is splitted into few other
functions.

  PlaybackAndCopyOnWorkerThread() {
    StagingBufferPool::AcquireStagingBuffer(); // with |lock_|
    PlaybackToStagingBuffer(); // without |lock_|
    CopyOnWorkerThread(); // without |lock_|, with context lock
    StagingBufferPool::ReleaseStagingBuffer(); // with |lock_|
  }

BUG=599863
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc: Refactor OneCopyTileTaskWorkerPool.

Move the staging buffer pool related functionalities out to its own
file. To isolate the usage of |lock_| from OneCopyTileTaskWorkerPool,
the PlaybackAndCopyOnWorkerThread() is splitted into few other
functions.

  PlaybackAndCopyOnWorkerThread() {
    StagingBufferPool::AcquireStagingBuffer(); // with |lock_|
    PlaybackToStagingBuffer(); // without |lock_|
    CopyOnWorkerThread(); // without |lock_|, with context lock
    StagingBufferPool::ReleaseStagingBuffer(); // with |lock_|
  }

BUG=599863
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[commit-bot: I haz the power, 2016-04-08 19:57:40]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-04-08 20:01:48]

Description was changed from

==========
cc: Refactor OneCopyTileTaskWorkerPool.

Move the staging buffer pool related functionalities out to its own
file. To isolate the usage of |lock_| from OneCopyTileTaskWorkerPool,
the PlaybackAndCopyOnWorkerThread() is splitted into few other
functions.

  PlaybackAndCopyOnWorkerThread() {
    StagingBufferPool::AcquireStagingBuffer(); // with |lock_|
    PlaybackToStagingBuffer(); // without |lock_|
    CopyOnWorkerThread(); // without |lock_|, with context lock
    StagingBufferPool::ReleaseStagingBuffer(); // with |lock_|
  }

BUG=599863
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc: Refactor OneCopyTileTaskWorkerPool.

Move the staging buffer pool related functionalities out to its own
file. To isolate the usage of |lock_| from OneCopyTileTaskWorkerPool,
the PlaybackAndCopyOnWorkerThread() is splitted into few other
functions.

  PlaybackAndCopyOnWorkerThread() {
    StagingBufferPool::AcquireStagingBuffer(); // with |lock_|
    PlaybackToStagingBuffer(); // without |lock_|
    CopyOnWorkerThread(); // without |lock_|, with context lock
    StagingBufferPool::ReleaseStagingBuffer(); // with |lock_|
  }

BUG=599863
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/a9dd9effc06a05b417255fe97afd92c3f32d963b
Cr-Commit-Position: refs/heads/master@{#386170}
==========

[commit-bot: I haz the power, 2016-04-08 20:01:50]

Patchset 5 (id:??) landed as
https://crrev.com/a9dd9effc06a05b417255fe97afd92c3f32d963b
Cr-Commit-Position: refs/heads/master@{#386170}