cc: Refactor TileTaskWorkerPool.

cc: Refactor TileTaskWorkerPool.

This patch moves tile task management from all TileTaskWorkerPool
classes to separate class making them thinner.

New class TileTaskManager now handles running tasks using task graph
runner set. This now puts the logic of raster buffer provider and
running task in different classes. This patch also renames all
TileTaskWorkerPools to RasterBufferProviders to depict what they
really do. New test classes FakeRasterBufferProviderImpl,
FakeTileTaskManagerImpl are added to cut the duplicated code.

Fixed duplicate nodes getting added in task graph in raster buffer
provider perftests.

BUG=599863
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

[prashant.n, 2016-04-22 14:50:51]

Description was changed from

==========
cc: Move tile task management its own class.

This patch moves tile task management from TileTaskWorkerPool classes
to its own class. The new class TileTaskManager now handles running
tasks using task graph runner set. This now puts the logic of
raster buffer provider and running task in different classes.

BUG=599863
==========

to

==========
cc: Move tile task management its own class.

This patch moves tile task management from TileTaskWorkerPool classes
to its own class. The new class TileTaskManager now handles running
tasks using task graph runner set. This now puts the logic of
raster buffer provider and running task in different classes.

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

[prashant.n, 2016-04-22 14:51:45]

Description was changed from

==========
cc: Move tile task management its own class.

This patch moves tile task management from TileTaskWorkerPool classes
to its own class. The new class TileTaskManager now handles running
tasks using task graph runner set. This now puts the logic of
raster buffer provider and running task in different classes.

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

to

==========
cc: Move tile task management to its own class.

This patch moves tile task management from TileTaskWorkerPool classes
to its own class. The new class TileTaskManager now handles running
tasks using task graph runner set. This now puts the logic of
raster buffer provider and running task in different classes.

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

[prashant.n, 2016-04-22 14:54:40]

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

[prashant.n, 2016-04-22 14:54:40]

Wip patch to separate tile task management from TTWP. Tests are yet to fixed. Do
not review yet.

In separate CL, I'll rename TTWP to RBP (raster buffer provider).

[prashant.n, 2016-04-25 03:34:12]

ptal.

[prashant.n, 2016-04-25 03:42:58]

Description was changed from

==========
cc: Move tile task management to its own class.

This patch moves tile task management from TileTaskWorkerPool classes
to its own class. The new class TileTaskManager now handles running
tasks using task graph runner set. This now puts the logic of
raster buffer provider and running task in different classes.

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

to

==========
cc: Move tile task management to its own class.

This patch moves tile task management from all TileTaskWorkerPool
classes to separate class making them thinner. The new class
TileTaskManager now handles running tasks using task graph runner set.
This now puts the logic of raster buffer provider and running task in
different classes.

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

[prashant.n, 2016-04-25 18:34:13]

On 2016/04/25 03:34:12, prashant.n wrote:
> ptal.

Before raster buffer provider patch, this patch needs to be reviewed.

[ericrk, 2016-04-25 20:56:08]

Overall looks like this cuts out a nice amount of duplicate code!

A few initial comments - still thinking over the general structure of the
refactor.

https://codereview.chromium.org/1910213005/diff/80001/cc/tiles/tile_manager_p...
File cc/tiles/tile_manager_perftest.cc (right):

https://codereview.chromium.org/1910213005/diff/80001/cc/tiles/tile_manager_p...
cc/tiles/tile_manager_perftest.cc:63: class FakeTileTaskManager : public
TileTaskManager {
This looks like the same code from fake_tile_manager.cc... can we make a
fake_tile_task_manager.h/cc and share this?

https://codereview.chromium.org/1910213005/diff/80001/cc/tiles/tile_task_mana...
File cc/tiles/tile_task_manager.cc (right):

https://codereview.chromium.org/1910213005/diff/80001/cc/tiles/tile_task_mana...
cc/tiles/tile_task_manager.cc:36: TRACE_EVENT0("cc",
"TileTaskManager::ScheduleTasksOnOriginThread");
nit: trace event should read "TileTaskManager::ScheduleTasks"

https://codereview.chromium.org/1910213005/diff/80001/cc/tiles/tile_task_mana...
File cc/tiles/tile_task_manager.h (right):

https://codereview.chromium.org/1910213005/diff/80001/cc/tiles/tile_task_mana...
cc/tiles/tile_task_manager.h:10: #include "cc/playback/raster_source.h"
do we need raster_source.h or raster_buffer.h?

https://codereview.chromium.org/1910213005/diff/80001/cc/tiles/tile_task_mana...
cc/tiles/tile_task_manager.h:17: class SequencedTaskRunner;
nite: remove this forward decl. - it isn't used in this file.

https://codereview.chromium.org/1910213005/diff/80001/cc/tiles/tile_task_mana...
cc/tiles/tile_task_manager.h:25: class CC_EXPORT TileTaskManager {
It's a bit weird that this class provides full implementations for all
functions, and then the testing subclasses override all these implementations
with their own implementations, modifying the base class's member variables
directly. This feels more to me like a few independent implementations of an
interface.

Can we make this more of an interface (without an actual implementation), then
create a TileTaskManagerImpl which provides the real implementation for Chrome?
Tests can then provide their own implementations of the interface.

[prashant.n, 2016-04-26 03:18:08]

As modifying this patch will need rebase for my next patch. I'll merge those two
patches together as they are related.

https://codereview.chromium.org/1910213005/diff/80001/cc/tiles/tile_task_mana...
File cc/tiles/tile_task_manager.h (right):

https://codereview.chromium.org/1910213005/diff/80001/cc/tiles/tile_task_mana...
cc/tiles/tile_task_manager.h:25: class CC_EXPORT TileTaskManager {
On 2016/04/25 20:56:08, ericrk wrote:
> It's a bit weird that this class provides full implementations for all
> functions, and then the testing subclasses override all these implementations
> with their own implementations, modifying the base class's member variables
> directly. This feels more to me like a few independent implementations of an
> interface.
> 
> Can we make this more of an interface (without an actual implementation), then
> create a TileTaskManagerImpl which provides the real implementation for
Chrome?
> Tests can then provide their own implementations of the interface.

Hmm. There are 3 functions for which testing classes have their own behavior,
rest would be same, so added virtual functions only than having interface. -

ScheduleTasks()
CheckForCompletedTasks()
Shutdown()

I'll add the interface.

[prashant.n, 2016-04-26 14:23:25]

Description was changed from

==========
cc: Move tile task management to its own class.

This patch moves tile task management from all TileTaskWorkerPool
classes to separate class making them thinner. The new class
TileTaskManager now handles running tasks using task graph runner set.
This now puts the logic of raster buffer provider and running task in
different classes.

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

to

==========
cc: Refactor TileTaskWorkerPool.

This patch moves tile task management from all TileTaskWorkerPool
classes to separate class making them thinner. 

New class TileTaskManager now handles running tasks using task graph
runner set. This now puts the logic of raster buffer provider and
running task in different classes. This patch also renames all
TileTaskWorkerPools to RasterBufferProviders to depict what they
really do.

New test classes FakeRasterBufferProviderImpl, FakeTileTaskManagerImpl
are added to cut the duplicated code.

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

[prashant.n, 2016-04-26 14:23:59]

ptal.

[prashant.n, 2016-04-26 14:26:14]

The CQ bit was checked by prashant.n@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-04-26 14:26:27]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1910213005/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1910213005/140001

[commit-bot: I haz the power, 2016-04-26 14:28:46]

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

[commit-bot: I haz the power, 2016-04-26 14:28:47]

Dry run: 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...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[prashant.n, 2016-04-27 05:30:24]

The CQ bit was checked by prashant.n@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-04-27 05:30:33]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1910213005/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1910213005/180001

[prashant.n, 2016-04-27 05:30:40]

ptal.

[commit-bot: I haz the power, 2016-04-27 06:06:47]

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

[commit-bot: I haz the power, 2016-04-27 06:06:48]

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

[prashant.n, 2016-04-29 06:53:56]

ptal.

[ericrk, 2016-04-29 07:24:38]

Very nice change - any time we delete 3x more lines than we add, it's a good
thing! Also, thanks for factoring out the FakeTileTaskManager.

LGTM

https://codereview.chromium.org/1910213005/diff/140001/cc/raster/bitmap_raste...
File cc/raster/bitmap_raster_buffer_provider.cc (right):

https://codereview.chromium.org/1910213005/diff/140001/cc/raster/bitmap_raste...
cc/raster/bitmap_raster_buffer_provider.cc:96: void
BitmapRasterBufferProvider::BarrierToSyncResources() {}
Please add a comment explaining that we do not need to sync resources, as this
provider does not use GL.

[prashant.n, 2016-04-29 10:05:03]

Description was changed from

==========
cc: Refactor TileTaskWorkerPool.

This patch moves tile task management from all TileTaskWorkerPool
classes to separate class making them thinner. 

New class TileTaskManager now handles running tasks using task graph
runner set. This now puts the logic of raster buffer provider and
running task in different classes. This patch also renames all
TileTaskWorkerPools to RasterBufferProviders to depict what they
really do.

New test classes FakeRasterBufferProviderImpl, FakeTileTaskManagerImpl
are added to cut the duplicated code.

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

to

==========
cc: Refactor TileTaskWorkerPool.

This patch moves tile task management from all TileTaskWorkerPool
classes to separate class making them thinner. 

New class TileTaskManager now handles running tasks using task graph
runner set. This now puts the logic of raster buffer provider and
running task in different classes. This patch also renames all
TileTaskWorkerPools to RasterBufferProviders to depict what they
really do. New test classes FakeRasterBufferProviderImpl,
FakeTileTaskManagerImpl are added to cut the duplicated code.

Fixed duplicate nodes getting added in task graph in raster buffer
provider perftests.

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

[prashant.n, 2016-04-29 10:06:41]

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

[prashant.n, 2016-04-29 10:06:42]

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/1910213005/#ps260001
(title: "feedback")

[commit-bot: I haz the power, 2016-04-29 10:06:48]

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

[commit-bot: I haz the power, 2016-04-29 11:26:17]

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

[commit-bot: I haz the power, 2016-04-29 11:26:18]

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

[prashant.n, 2016-04-29 12:46:17]

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

[commit-bot: I haz the power, 2016-04-29 12:46:23]

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

[commit-bot: I haz the power, 2016-04-29 12:51:37]

Description was changed from

==========
cc: Refactor TileTaskWorkerPool.

This patch moves tile task management from all TileTaskWorkerPool
classes to separate class making them thinner. 

New class TileTaskManager now handles running tasks using task graph
runner set. This now puts the logic of raster buffer provider and
running task in different classes. This patch also renames all
TileTaskWorkerPools to RasterBufferProviders to depict what they
really do. New test classes FakeRasterBufferProviderImpl,
FakeTileTaskManagerImpl are added to cut the duplicated code.

Fixed duplicate nodes getting added in task graph in raster buffer
provider perftests.

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

to

==========
cc: Refactor TileTaskWorkerPool.

This patch moves tile task management from all TileTaskWorkerPool
classes to separate class making them thinner. 

New class TileTaskManager now handles running tasks using task graph
runner set. This now puts the logic of raster buffer provider and
running task in different classes. This patch also renames all
TileTaskWorkerPools to RasterBufferProviders to depict what they
really do. New test classes FakeRasterBufferProviderImpl,
FakeTileTaskManagerImpl are added to cut the duplicated code.

Fixed duplicate nodes getting added in task graph in raster buffer
provider perftests.

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

[commit-bot: I haz the power, 2016-04-29 12:51:38]

Committed patchset #14 (id:260001)

[commit-bot: I haz the power, 2016-04-30 17:25:24]

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

[commit-bot: I haz the power, 2016-04-30 18:00:21]

Description was changed from

==========
cc: Refactor TileTaskWorkerPool.

This patch moves tile task management from all TileTaskWorkerPool
classes to separate class making them thinner. 

New class TileTaskManager now handles running tasks using task graph
runner set. This now puts the logic of raster buffer provider and
running task in different classes. This patch also renames all
TileTaskWorkerPools to RasterBufferProviders to depict what they
really do. New test classes FakeRasterBufferProviderImpl,
FakeTileTaskManagerImpl are added to cut the duplicated code.

Fixed duplicate nodes getting added in task graph in raster buffer
provider perftests.

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

to

==========
cc: Refactor TileTaskWorkerPool.

This patch moves tile task management from all TileTaskWorkerPool
classes to separate class making them thinner.

New class TileTaskManager now handles running tasks using task graph
runner set. This now puts the logic of raster buffer provider and
running task in different classes. This patch also renames all
TileTaskWorkerPools to RasterBufferProviders to depict what they
really do. New test classes FakeRasterBufferProviderImpl,
FakeTileTaskManagerImpl are added to cut the duplicated code.

Fixed duplicate nodes getting added in task graph in raster buffer
provider perftests.

BUG=599863
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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