cc: Change max_tiles_for_interest_rect to be screen pixels padding.

cc: Change max_tiles_for_interest_rect to be screen pixels padding.

This patch changes the interest rect calculation from using number
of tiles to screen space distance (3k as the initial setting). The
problem with using the number of tiles is that on devices like mac
book pro, we end up having very little prepainting area in software
and way too large of a prepainting area with ganesh, since the sizes
of tiles differ drastically.

BUG=483520
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/6c0087ba54947efa6dd3c9e8b7dd40ea644d9947
Cr-Commit-Position: refs/heads/master@{#339704}

[vmpstr, 2015-07-13 21:48:20]

vmpstr@chromium.org changed reviewers:
+ enne@chromium.org, jdduke@chromium.org, weiliangc@chromium.org

[vmpstr, 2015-07-13 21:48:20]

Please take a look. jdduke@, this is the device space approach. I'm asking for a
review just to prepare this for landing, but let's first agree on whether this
is acceptable or not on the bug :)

[weiliangc, 2015-07-15 19:56:10]

LGTM w/ some question on unittest numbers.

https://codereview.chromium.org/1236533004/diff/80001/cc/layers/picture_layer...
File cc/layers/picture_layer_impl_unittest.cc (right):

https://codereview.chromium.org/1236533004/diff/80001/cc/layers/picture_layer...
cc/layers/picture_layer_impl_unittest.cc:4755: gfx::Rect(0, 5000, 100, 100),  //
viewport_rect_for_tile_priority
Not sure why are we changing to 5k?

https://codereview.chromium.org/1236533004/diff/80001/cc/tiles/picture_layer_...
File cc/tiles/picture_layer_tiling_unittest.cc (left):

https://codereview.chromium.org/1236533004/diff/80001/cc/tiles/picture_layer_...
cc/tiles/picture_layer_tiling_unittest.cc:553:
settings.max_tiles_for_interest_area = 10000;
Why can we rm this here? Does it have no effects in the first place?

https://codereview.chromium.org/1236533004/diff/80001/cc/tiles/tile_manager_u...
File cc/tiles/tile_manager_unittest.cc (left):

https://codereview.chromium.org/1236533004/diff/80001/cc/tiles/tile_manager_u...
cc/tiles/tile_manager_unittest.cc:1164: settings.max_tiles_for_interest_area =
10000;
Like before is this removed because our default |tiling_interest_area_padding|
is big enough?

[vmpstr, 2015-07-16 00:31:03]

https://codereview.chromium.org/1236533004/diff/80001/cc/layers/picture_layer...
File cc/layers/picture_layer_impl_unittest.cc (right):

https://codereview.chromium.org/1236533004/diff/80001/cc/layers/picture_layer...
cc/layers/picture_layer_impl_unittest.cc:4755: gfx::Rect(0, 5000, 100, 100),  //
viewport_rect_for_tile_priority
On 2015/07/15 19:56:09, weiliangc wrote:
> Not sure why are we changing to 5k?

So we set the tile size to be 100, 100 and so in order to ensure that the layer
is offscreen it was enough to set the viewport to be at 0, 1000. However, by
using 3k as the padding, this is no longer sufficient. So this just moves it
"far enough away" to not be in the interest rect, which is the intent of this
test.

https://codereview.chromium.org/1236533004/diff/80001/cc/tiles/picture_layer_...
File cc/tiles/picture_layer_tiling_unittest.cc (left):

https://codereview.chromium.org/1236533004/diff/80001/cc/tiles/picture_layer_...
cc/tiles/picture_layer_tiling_unittest.cc:553:
settings.max_tiles_for_interest_area = 10000;
On 2015/07/15 19:56:09, weiliangc wrote:
> Why can we rm this here? Does it have no effects in the first place?

It's a similar reason as the previous comment. Before, we didn't have enough to
get all of the times for the interest rect. Now, the padding is large enough to
get the tiles, so this isn't required (ie the default value is good enough).

https://codereview.chromium.org/1236533004/diff/80001/cc/tiles/tile_manager_u...
File cc/tiles/tile_manager_unittest.cc (left):

https://codereview.chromium.org/1236533004/diff/80001/cc/tiles/tile_manager_u...
cc/tiles/tile_manager_unittest.cc:1164: settings.max_tiles_for_interest_area =
10000;
On 2015/07/15 19:56:09, weiliangc wrote:
> Like before is this removed because our default |tiling_interest_area_padding|
> is big enough?

Yup.

[weiliangc, 2015-07-16 19:19:23]

On 2015/07/16 00:31:03, vmpstr wrote:
>
https://codereview.chromium.org/1236533004/diff/80001/cc/layers/picture_layer...
> File cc/layers/picture_layer_impl_unittest.cc (right):
> 
>
https://codereview.chromium.org/1236533004/diff/80001/cc/layers/picture_layer...
> cc/layers/picture_layer_impl_unittest.cc:4755: gfx::Rect(0, 5000, 100, 100), 
//
> viewport_rect_for_tile_priority
> On 2015/07/15 19:56:09, weiliangc wrote:
> > Not sure why are we changing to 5k?
> 
> So we set the tile size to be 100, 100 and so in order to ensure that the
layer
> is offscreen it was enough to set the viewport to be at 0, 1000. However, by
> using 3k as the padding, this is no longer sufficient. So this just moves it
> "far enough away" to not be in the interest rect, which is the intent of this
> test.
> 
>
https://codereview.chromium.org/1236533004/diff/80001/cc/tiles/picture_layer_...
> File cc/tiles/picture_layer_tiling_unittest.cc (left):
> 
>
https://codereview.chromium.org/1236533004/diff/80001/cc/tiles/picture_layer_...
> cc/tiles/picture_layer_tiling_unittest.cc:553:
> settings.max_tiles_for_interest_area = 10000;
> On 2015/07/15 19:56:09, weiliangc wrote:
> > Why can we rm this here? Does it have no effects in the first place?
> 
> It's a similar reason as the previous comment. Before, we didn't have enough
to
> get all of the times for the interest rect. Now, the padding is large enough
to
> get the tiles, so this isn't required (ie the default value is good enough).
> 
>
https://codereview.chromium.org/1236533004/diff/80001/cc/tiles/tile_manager_u...
> File cc/tiles/tile_manager_unittest.cc (left):
> 
>
https://codereview.chromium.org/1236533004/diff/80001/cc/tiles/tile_manager_u...
> cc/tiles/tile_manager_unittest.cc:1164: settings.max_tiles_for_interest_area =
> 10000;
> On 2015/07/15 19:56:09, weiliangc wrote:
> > Like before is this removed because our default
|tiling_interest_area_padding|
> > is big enough?
> 
> Yup.

Thanks for explaining! :)

[vmpstr, 2015-07-17 17:34:04]

+rsesek for crash_keys
+piman for content/ and chrome_restart_request

Please take a look.

[vmpstr, 2015-07-17 17:43:34]

vmpstr@chromium.org changed reviewers:
+ piman@chromium.org, rsesek@chromium.org

[vmpstr, 2015-07-17 17:43:35]

actually +rsesek and +piman. Please take a look.

[Robert Sesek, 2015-07-17 17:53:38]

crash_keys.cc LGTM

[vmpstr, 2015-07-20 18:47:39]

ping, piman@ could you take a look at content/ and chrome/browser/? thanks!

[piman, 2015-07-20 22:00:24]

lgtm

[vmpstr, 2015-07-21 17:33:29]

The CQ bit was checked by vmpstr@chromium.org

[vmpstr, 2015-07-21 17:33:30]

The patchset sent to the CQ was uploaded after l-g-t-m from rsesek@chromium.org,
weiliangc@chromium.org, piman@chromium.org
Link to the patchset: https://codereview.chromium.org/1236533004/#ps100001
(title: "rebase")

[commit-bot: I haz the power, 2015-07-21 17:34:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1236533004/100001

[commit-bot: I haz the power, 2015-07-21 18:32:49]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-07-21 18:34:06]

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