cc: Use ResourceUtil::UncheckedWidthInBytes() in CopyOnWorkerThread().

cc: Use ResourceUtil::UncheckedWidthInBytes() in CopyOnWorkerThread().

Use unchecked version for computing bytes_per_row in
OneCopyTileTaskWorkerPool::CopyOnWorkerThread() function to check
overflow or underflow conditions in debug builds.

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/00f3200485a68c3757e686d7d6ba164aca2cdeb6
Cr-Commit-Position: refs/heads/master@{#388034}

[prashant.n, 2016-04-13 11:46:55]

Description was changed from

==========
cc: Use ResourceUtil::UncheckedWidthInBytes() in CopyOnWorkerThread().

Use unchecked version for computing bytes_per_row in
OneCopyTileTaskWorkerPool::CopyOnWorkerThread() function to check
overflow or underflow conditions in debug builds.

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

to

==========
cc: Use ResourceUtil::UncheckedWidthInBytes() in CopyOnWorkerThread().

Use unchecked version for computing bytes_per_row in
OneCopyTileTaskWorkerPool::CopyOnWorkerThread() function to check
overflow or underflow conditions in debug builds.

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

[prashant.n, 2016-04-13 11:51:53]

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

[prashant.n, 2016-04-13 11:51:54]

ptal

[danakj, 2016-04-13 18:38:59]

https://codereview.chromium.org/1879153003/diff/1/cc/raster/one_copy_tile_tas...
File cc/raster/one_copy_tile_task_worker_pool.cc (right):

https://codereview.chromium.org/1879153003/diff/1/cc/raster/one_copy_tile_tas...
cc/raster/one_copy_tile_task_worker_pool.cc:328: int bytes_per_row =
ResourceUtil::UncheckedWidthInBytes<int>(
If it can overflow/underflow then you want to use Checked math. Unchecked is for
when you can prove to yourself that it can't.

[prashant.n, 2016-04-14 03:00:51]

On 2016/04/13 18:38:59, danakj wrote:
>
https://codereview.chromium.org/1879153003/diff/1/cc/raster/one_copy_tile_tas...
> File cc/raster/one_copy_tile_task_worker_pool.cc (right):
> 
>
https://codereview.chromium.org/1879153003/diff/1/cc/raster/one_copy_tile_tas...
> cc/raster/one_copy_tile_task_worker_pool.cc:328: int bytes_per_row =
> ResourceUtil::UncheckedWidthInBytes<int>(
> If it can overflow/underflow then you want to use Checked math. Unchecked is
for
> when you can prove to yourself that it can't.

Yes. Unchecked is for we can safely use without checking overflow/underflow. But
debug builds check it.
Here resource size is always defined by tile size, so I thought of using
unchecked version.

[prashant.n, 2016-04-14 03:03:33]

> Yes. Unchecked is for we can safely use without checking overflow/underflow.
But
> debug builds check it.
I meant the verification of fits in bytes or not.

[prashant.n, 2016-04-15 07:04:47]

ptal - patch #3.

[prashant.n, 2016-04-17 18:30:27]

ping

[reveman, 2016-04-17 20:22:59]

lgtm when danakj is happy with it

https://codereview.chromium.org/1879153003/diff/1/cc/raster/one_copy_tile_tas...
File cc/raster/one_copy_tile_task_worker_pool.cc (right):

https://codereview.chromium.org/1879153003/diff/1/cc/raster/one_copy_tile_tas...
cc/raster/one_copy_tile_task_worker_pool.cc:328: int bytes_per_row =
ResourceUtil::UncheckedWidthInBytes<int>(
On 2016/04/13 at 18:38:59, danakj wrote:
> If it can overflow/underflow then you want to use Checked math. Unchecked is
for when you can prove to yourself that it can't.

I don't think we want unnecessary conditionals to be executed here to check if
this overflow/underflow unless we're in a debug build. There's no reason this
should overflow/underflow unless there's a bug in our code. Unchecked seems
appropriate.

[prashant.n, 2016-04-18 05:06:13]

prashant.n@samsung.com changed reviewers:
+ danakj@chromium.org

[prashant.n, 2016-04-18 05:06:14]

danakj@, can you pls. put your opinion.

[danakj, 2016-04-18 19:47:53]

https://codereview.chromium.org/1879153003/diff/1/cc/raster/one_copy_tile_tas...
File cc/raster/one_copy_tile_task_worker_pool.cc (right):

https://codereview.chromium.org/1879153003/diff/1/cc/raster/one_copy_tile_tas...
cc/raster/one_copy_tile_task_worker_pool.cc:328: int bytes_per_row =
ResourceUtil::UncheckedWidthInBytes<int>(
On 2016/04/17 20:22:59, reveman wrote:
> On 2016/04/13 at 18:38:59, danakj wrote:
> > If it can overflow/underflow then you want to use Checked math. Unchecked is
> for when you can prove to yourself that it can't.
> 
> I don't think we want unnecessary conditionals to be executed here to check if
> this overflow/underflow unless we're in a debug build. There's no reason this
> should overflow/underflow unless there's a bug in our code. Unchecked seems
> appropriate.

ok sg thanks, just wanted to be sure. LGTM then

[prashant.n, 2016-04-18 20:16:46]

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

[commit-bot: I haz the power, 2016-04-18 20:17:30]

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

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

Committed patchset #1 (id:1)

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

Description was changed from

==========
cc: Use ResourceUtil::UncheckedWidthInBytes() in CopyOnWorkerThread().

Use unchecked version for computing bytes_per_row in
OneCopyTileTaskWorkerPool::CopyOnWorkerThread() function to check
overflow or underflow conditions in debug builds.

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

to

==========
cc: Use ResourceUtil::UncheckedWidthInBytes() in CopyOnWorkerThread().

Use unchecked version for computing bytes_per_row in
OneCopyTileTaskWorkerPool::CopyOnWorkerThread() function to check
overflow or underflow conditions in debug builds.

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/00f3200485a68c3757e686d7d6ba164aca2cdeb6
Cr-Commit-Position: refs/heads/master@{#388034}
==========

[commit-bot: I haz the power, 2016-04-18 21:31:05]

Patchset 1 (id:??) landed as
https://crrev.com/00f3200485a68c3757e686d7d6ba164aca2cdeb6
Cr-Commit-Position: refs/heads/master@{#388034}