Clean up task category assignment

Clean up task category assignment

Task category assignment in TileManager is a bit roundabout, and can be
simpler now that all TileTasks have the |supports_concurrent_execution|
method.

This change also fixes an issue where NONCONCURRENT_FOREGROUND tasks
were accidentally downgraded to FOREGROUND.

BUG=605234
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/3ae72aff5123edc8c9042da88a95b895a227297e
Cr-Commit-Position: refs/heads/master@{#391200}

[ericrk, 2016-04-20 19:52:13]

Description was changed from

==========
Clean up task category assignment

Task category assignment in TileManager is a bit roundabout, and can be
simpler now that all TileTasks have the |supports_concurrent_execution|
method.

This change also fixes an issue where NONCONCURRENT_FOREGROUND tasks
were accidentally downgraded to FOREGROUND.

BUG=605234
==========

to

==========
Clean up task category assignment

Task category assignment in TileManager is a bit roundabout, and can be
simpler now that all TileTasks have the |supports_concurrent_execution|
method.

This change also fixes an issue where NONCONCURRENT_FOREGROUND tasks
were accidentally downgraded to FOREGROUND.

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

[ericrk, 2016-04-22 22:12:57]

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

[prashant.n, 2016-04-23 04:55:20]

Change looks good, but naming another variable as  "high_priority" hurts
readability.

https://codereview.chromium.org/1910613002/diff/20001/cc/tiles/tile_manager.cc
File cc/tiles/tile_manager.cc (right):

https://codereview.chromium.org/1910613002/diff/20001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:135: TaskCategory TaskCategoryForTileTask(TileTask*
task, bool high_priority) {
May be replace high_priority with is_foreground or similar.

https://codereview.chromium.org/1910613002/diff/20001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:145: bool IsHighPriorityCategory(uint16_t category) {
Should we use simple function name IsForeground() for better readability?

https://codereview.chromium.org/1910613002/diff/20001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:184: bool high_prioirty,
May be replace high_priority with is_foreground or similar.

https://codereview.chromium.org/1910613002/diff/20001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:203: size_t priority,
Replace this with task_priority.

https://codereview.chromium.org/1910613002/diff/20001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:204: bool high_priority) {
Replace this with tile_priority for better readability.

[vmpstr, 2016-04-25 18:23:34]

rs lgtm if prashant.n is happy.

https://codereview.chromium.org/1910613002/diff/20001/cc/tiles/tile_manager.cc
File cc/tiles/tile_manager.cc (right):

https://codereview.chromium.org/1910613002/diff/20001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:138: }
nit: remove braces please

[ericrk, 2016-05-02 20:02:04]

Patchset #3 (id:40001) has been deleted

[ericrk, 2016-05-02 23:04:38]

Thanks for the feedback. Naming suggestions made sense (we were definitely using
priority too many places) :D.

https://codereview.chromium.org/1910613002/diff/20001/cc/tiles/tile_manager.cc
File cc/tiles/tile_manager.cc (right):

https://codereview.chromium.org/1910613002/diff/20001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:135: TaskCategory TaskCategoryForTileTask(TileTask*
task, bool high_priority) {
On 2016/04/23 04:55:19, prashant.n wrote:
> May be replace high_priority with is_foreground or similar.

Done.

https://codereview.chromium.org/1910613002/diff/20001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:138: }
On 2016/04/25 18:23:34, vmpstr wrote:
> nit: remove braces please

Done.

https://codereview.chromium.org/1910613002/diff/20001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:145: bool IsHighPriorityCategory(uint16_t category) {
On 2016/04/23 04:55:19, prashant.n wrote:
> Should we use simple function name IsForeground() for better readability?

Using IsForegroundCategory - I agree that HighPriority is misleading.

https://codereview.chromium.org/1910613002/diff/20001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:184: bool high_prioirty,
On 2016/04/23 04:55:19, prashant.n wrote:
> May be replace high_priority with is_foreground or similar.

Done.

https://codereview.chromium.org/1910613002/diff/20001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:203: size_t priority,
On 2016/04/23 04:55:19, prashant.n wrote:
> Replace this with task_priority.

Should be less confusing now that we only have one "priority".

https://codereview.chromium.org/1910613002/diff/20001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:204: bool high_priority) {
On 2016/04/23 04:55:19, prashant.n wrote:
> Replace this with tile_priority for better readability.

Done.

[prashant.n, 2016-05-03 02:06:59]

lgtm

It looks very good now, no confusion.

[ericrk, 2016-05-03 10:44:35]

The CQ bit was checked by ericrk@chromium.org

[ericrk, 2016-05-03 10:44:36]

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

[commit-bot: I haz the power, 2016-05-03 10:44:51]

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

[commit-bot: I haz the power, 2016-05-03 11:19:58]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-05-03 11:20:56]

Description was changed from

==========
Clean up task category assignment

Task category assignment in TileManager is a bit roundabout, and can be
simpler now that all TileTasks have the |supports_concurrent_execution|
method.

This change also fixes an issue where NONCONCURRENT_FOREGROUND tasks
were accidentally downgraded to FOREGROUND.

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

to

==========
Clean up task category assignment

Task category assignment in TileManager is a bit roundabout, and can be
simpler now that all TileTasks have the |supports_concurrent_execution|
method.

This change also fixes an issue where NONCONCURRENT_FOREGROUND tasks
were accidentally downgraded to FOREGROUND.

BUG=605234
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/3ae72aff5123edc8c9042da88a95b895a227297e
Cr-Commit-Position: refs/heads/master@{#391200}
==========

[commit-bot: I haz the power, 2016-05-03 11:20:57]

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