Re-land: cc: Cleanup internal::WorkerPoolTaskClient interface.

Re-land: cc: Cleanup internal::WorkerPoolTaskClient interface.

::CompleteOnOriginThread is now called when RasterWorkerPool has
completed all necessary initialization of resource, including
texture uploads. This improves the WorkerPoolTask/Client interface
significantly and will allow better resource recycling once
::RunReplyOnOriginThread is removed.

BUG=269841
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=252874

[vmpstr, 2014-02-19 00:49:31]

https://codereview.chromium.org/170783007/diff/1/cc/resources/pixel_buffer_ra...
File cc/resources/pixel_buffer_raster_worker_pool.cc (right):

https://codereview.chromium.org/170783007/diff/1/cc/resources/pixel_buffer_ra...
cc/resources/pixel_buffer_raster_worker_pool.cc:133:
new_raster_task_states[task].type = RasterTaskState::UNSCHEDULED;
You either need to set resource to NULL here or have a constructor for
RasterTaskState (I would prefer the latter).

https://codereview.chromium.org/170783007/diff/1/cc/resources/pixel_buffer_ra...
cc/resources/pixel_buffer_raster_worker_pool.cc:630: if (state_it !=
raster_task_states_.end()) {
Can you put the smaller block in if? 

if (state_it == raster_task_state_.end()) {
  // handle image decode
  continue;
}

....

https://codereview.chromium.org/170783007/diff/1/cc/resources/pixel_buffer_ra...
cc/resources/pixel_buffer_raster_worker_pool.cc:640: if (!content_has_changed) {
Can you put a comment here explaining what has happened if this is true (ie,
it's solid color).

https://codereview.chromium.org/170783007/diff/1/cc/resources/resource_provid...
File cc/resources/resource_provider.cc (right):

https://codereview.chromium.org/170783007/diff/1/cc/resources/resource_provid...
cc/resources/resource_provider.cc:508: raster_bitmap_generation_id_ !=
raster_bitmap_.getGenerationID();
Is this like a good check? According to skia header files, I can buy that this
works, but have you verified it in most cases?

Also, can you put a comment here explaining what's happening?

[reveman, 2014-02-19 01:41:12]

PTAL

https://codereview.chromium.org/170783007/diff/1/cc/resources/pixel_buffer_ra...
File cc/resources/pixel_buffer_raster_worker_pool.cc (right):

https://codereview.chromium.org/170783007/diff/1/cc/resources/pixel_buffer_ra...
cc/resources/pixel_buffer_raster_worker_pool.cc:133:
new_raster_task_states[task].type = RasterTaskState::UNSCHEDULED;
On 2014/02/19 00:49:31, vmpstr wrote:
> You either need to set resource to NULL here or have a constructor for
> RasterTaskState (I would prefer the latter).

added an explicit default ctor.

https://codereview.chromium.org/170783007/diff/1/cc/resources/pixel_buffer_ra...
cc/resources/pixel_buffer_raster_worker_pool.cc:630: if (state_it !=
raster_task_states_.end()) {
On 2014/02/19 00:49:31, vmpstr wrote:
> Can you put the smaller block in if? 
> 
> if (state_it == raster_task_state_.end()) {
>   // handle image decode
>   continue;
> }
> 
> ....

Done.

https://codereview.chromium.org/170783007/diff/1/cc/resources/pixel_buffer_ra...
cc/resources/pixel_buffer_raster_worker_pool.cc:640: if (!content_has_changed) {
On 2014/02/19 00:49:31, vmpstr wrote:
> Can you put a comment here explaining what has happened if this is true (ie,
> it's solid color).

Done.

https://codereview.chromium.org/170783007/diff/1/cc/resources/resource_provid...
File cc/resources/resource_provider.cc (right):

https://codereview.chromium.org/170783007/diff/1/cc/resources/resource_provid...
cc/resources/resource_provider.cc:508: raster_bitmap_generation_id_ !=
raster_bitmap_.getGenerationID();
On 2014/02/19 00:49:31, vmpstr wrote:
> Is this like a good check? According to skia header files, I can buy that this
> works, but have you verified it in most cases?
> 
> Also, can you put a comment here explaining what's happening?

Done.

[alokp, 2014-02-19 19:12:12]

lgtm

https://codereview.chromium.org/170783007/diff/80001/cc/resources/resource_pr...
File cc/resources/resource_provider.h (right):

https://codereview.chromium.org/170783007/diff/80001/cc/resources/resource_pr...
cc/resources/resource_provider.h:454: bool UnlockForWrite();
Add a comment what the bool means.

[reveman, 2014-02-19 22:53:08]

https://codereview.chromium.org/170783007/diff/80001/cc/resources/resource_pr...
File cc/resources/resource_provider.h (right):

https://codereview.chromium.org/170783007/diff/80001/cc/resources/resource_pr...
cc/resources/resource_provider.h:454: bool UnlockForWrite();
On 2014/02/19 19:12:12, Alok Priyadarshi wrote:
> Add a comment what the bool means.

Done.

[vmpstr, 2014-02-19 22:59:34]

lgtm

[reveman, 2014-02-19 23:08:50]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2014-02-19 23:19:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/reveman@chromium.org/170783007/130001

[commit-bot: I haz the power, 2014-02-20 01:11:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/reveman@chromium.org/170783007/130001

[commit-bot: I haz the power, 2014-02-20 04:56:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/reveman@chromium.org/170783007/130001

[commit-bot: I haz the power, 2014-02-20 08:55:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/reveman@chromium.org/170783007/130001

[commit-bot: I haz the power, 2014-02-20 12:23:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/reveman@chromium.org/170783007/130001

[commit-bot: I haz the power, 2014-02-21 00:25:57]

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

[commit-bot: I haz the power, 2014-02-21 00:25:57]

The commit queue went berserk retrying too often for a
seemingly flaky test on builder android_dbg:
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...

[reveman, 2014-02-21 00:34:03]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2014-02-21 00:37:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/reveman@chromium.org/170783007/130001

[commit-bot: I haz the power, 2014-02-21 09:06:23]

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

[commit-bot: I haz the power, 2014-02-21 09:06:24]

The commit queue went berserk retrying too often for a
seemingly flaky test on builder android_dbg:
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...

[reveman, 2014-02-21 15:10:00]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2014-02-21 15:12:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/reveman@chromium.org/170783007/130001

[commit-bot: I haz the power, 2014-02-21 18:23:11]

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

[commit-bot: I haz the power, 2014-02-21 18:23:12]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[commit-bot: I haz the power, 2014-02-21 18:23:12]

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

[commit-bot: I haz the power, 2014-02-21 18:23:12]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[Sergey Berezin, 2014-02-21 19:34:10]

The CQ bit was checked by sergeyberezin@chromium.org

[commit-bot: I haz the power, 2014-02-21 19:36:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/reveman@chromium.org/170783007/130001

[commit-bot: I haz the power, 2014-02-22 01:23:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/reveman@chromium.org/170783007/130001

[reveman, 2014-02-22 02:12:37]

The CQ bit was unchecked by reveman@chromium.org

[reveman, 2014-02-22 02:20:26]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2014-02-22 02:24:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/reveman@chromium.org/170783007/980001

[commit-bot: I haz the power, 2014-02-22 04:02:51]

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

[commit-bot: I haz the power, 2014-02-22 04:02:52]

Retried try job too often on win_rel for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[reveman, 2014-02-22 04:28:52]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2014-02-22 04:29:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/reveman@chromium.org/170783007/980001

[commit-bot: I haz the power, 2014-02-22 08:42:46]

Change committed as 252790

[jdduke (slow), 2014-02-22 15:15:32]

On 2014/02/22 08:42:46, I haz the power (commit-bot) wrote:
> Change committed as 252790

This appears to be tripping a DCHECK on the Android dbg builder in
PixelBufferRasterWorkerPool::ScheduleMoreTasks()+416 (see
http://build.chromium.org/p/chromium.linux/builders/Android%20Tests%20%28dbg%...).
 reveman@: How do you want to move forward? Should we revert to re-open the
tree?

[pkotwicz, 2014-02-22 18:48:08]

A revert of this CL has been created in
https://codereview.chromium.org/176403002/ by pkotwicz@chromium.org.

The reason for reverting is: Speculative revert. Suspecting that this CL may be
causing a DCHECK to fire on Android in
PixelBufferRasterWorkerPool::ScheduleMoreTasks(

http://build.chromium.org/p/chromium.linux/builders/Android%20Tests%20%28dbg%....

[reveman, 2014-02-22 22:55:20]

On 2014/02/22 18:48:08, pkotwicz wrote:
> A revert of this CL has been created in
> https://codereview.chromium.org/176403002/ by mailto:pkotwicz@chromium.org.
> 
> The reason for reverting is: Speculative revert. Suspecting that this CL may
be
> causing a DCHECK to fire on Android in
> PixelBufferRasterWorkerPool::ScheduleMoreTasks(
> 
>
http://build.chromium.org/p/chromium.linux/builders/Android%2520Tests%2520%25....

Thanks for reverting. I found the problem and will try to re-land this change.

[reveman, 2014-02-22 22:55:31]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2014-02-22 22:55:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/reveman@chromium.org/170783007/1340001

[commit-bot: I haz the power, 2014-02-23 08:00:14]

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

[commit-bot: I haz the power, 2014-02-23 08:00:15]

The commit queue went berserk retrying too often for a
seemingly flaky test on builder android_dbg:
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...

[reveman, 2014-02-23 22:59:41]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2014-02-23 22:59:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/reveman@chromium.org/170783007/1340001

[commit-bot: I haz the power, 2014-02-24 02:20:22]

Change committed as 252874