Cleanup resourceless_software_draw()

Remove resourceless_software_draw() and related unit tests.

WebView passes the correct viewport and transform for tile
priority in both hardware and resourceless software draws. As
a result, PictureLayerImpl does not need to treat
resourceless software draws differently.

BUG=398587
Committed: https://crrev.com/367d7dd63fb48978cf28918397a8269f368e6f27
Cr-Commit-Position: refs/heads/master@{#292723}

[hush (inactive), 2014-08-29 00:29:53]

Patchset #1 (id:1) has been deleted

[hush (inactive), 2014-08-29 00:32:29]

hush@chromium.org changed reviewers:
+ boliu@chromium.org

[hush (inactive), 2014-08-29 00:32:42]

Hi Bo,
how does it look?

[boliu, 2014-08-29 00:36:11]

https://codereview.chromium.org/517893002/diff/20001/content/browser/android/...
File content/browser/android/in_process/synchronous_compositor_output_surface.cc
(right):

https://codereview.chromium.org/517893002/diff/20001/content/browser/android/...
content/browser/android/in_process/synchronous_compositor_output_surface.cc:205:
InvokeComposite(transform, clip, clip, empty, gfx::Transform(), false);
wait wot? Of course you need it, used the cached version.

[hush (inactive), 2014-08-29 00:50:55]

https://codereview.chromium.org/517893002/diff/20001/content/browser/android/...
File content/browser/android/in_process/synchronous_compositor_output_surface.cc
(right):

https://codereview.chromium.org/517893002/diff/20001/content/browser/android/...
content/browser/android/in_process/synchronous_compositor_output_surface.cc:205:
InvokeComposite(transform, clip, clip, empty, gfx::Transform(), false);
Maybe that comment is not clear enough... That comment refers to "empty" rect
passed in for viewport_rect_for_tile_priority, because empty rect is used for
the resourceless software draw.

The cached version is used after the software draw. 
On 2014/08/29 00:36:11, boliu wrote:
> wait wot? Of course you need it, used the cached version.

[boliu, 2014-08-29 00:56:32]

https://codereview.chromium.org/517893002/diff/20001/content/browser/android/...
File content/browser/android/in_process/synchronous_compositor_output_surface.cc
(right):

https://codereview.chromium.org/517893002/diff/20001/content/browser/android/...
content/browser/android/in_process/synchronous_compositor_output_surface.cc:205:
InvokeComposite(transform, clip, clip, empty, gfx::Transform(), false);
On 2014/08/29 00:50:55, hush wrote:
> Maybe that comment is not clear enough... That comment refers to "empty" rect
> passed in for viewport_rect_for_tile_priority, because empty rect is used for
> the resourceless software draw.

It's clear and it's wrong.

The viewport/matrix for tile priority must be correct (ie match the ones used in
hardware draw) at all times. Otherwise you can get widely different tiling
priorities between software and hardware draws, causing tiles to thrash.

This refactor was only possible after we separated out draw viewport and the
tile viewport. cc used to ignore tile viewport/matrix and use the old value if
it's a software draw. Now we have the old value right here, so just pass the old
values in, so no need for cc to special case software draws.

> 
> The cached version is used after the software draw. 
> On 2014/08/29 00:36:11, boliu wrote:
> > wait wot? Of course you need it, used the cached version.
>

[hush (inactive), 2014-08-29 01:29:52]

https://codereview.chromium.org/517893002/diff/20001/content/browser/android/...
File content/browser/android/in_process/synchronous_compositor_output_surface.cc
(right):

https://codereview.chromium.org/517893002/diff/20001/content/browser/android/...
content/browser/android/in_process/synchronous_compositor_output_surface.cc:205:
InvokeComposite(transform, clip, clip, empty, gfx::Transform(), false);
Cool. I see. Uploaded another patch for it.
On 2014/08/29 00:56:32, boliu wrote:
> On 2014/08/29 00:50:55, hush wrote:
> > Maybe that comment is not clear enough... That comment refers to "empty"
rect
> > passed in for viewport_rect_for_tile_priority, because empty rect is used
for
> > the resourceless software draw.
> 
> It's clear and it's wrong.
> 
> The viewport/matrix for tile priority must be correct (ie match the ones used
in
> hardware draw) at all times. Otherwise you can get widely different tiling
> priorities between software and hardware draws, causing tiles to thrash.
> 
> This refactor was only possible after we separated out draw viewport and the
> tile viewport. cc used to ignore tile viewport/matrix and use the old value if
> it's a software draw. Now we have the old value right here, so just pass the
old
> values in, so no need for cc to special case software draws.
> 
> > 
> > The cached version is used after the software draw. 
> > On 2014/08/29 00:36:11, boliu wrote:
> > > wait wot? Of course you need it, used the cached version.
> > 
>

[hush (inactive), 2014-08-29 01:30:15]

I broke:    NoLowResPictureLayerImplTest.InvalidViewportForPrioritizingTiles
PictureLayerImplTest.InvalidViewportForPrioritizingTiles

I will fix the tests in the next patch.

[boliu, 2014-08-29 01:51:27]

On 2014/08/29 01:30:15, hush wrote:
> I broke:    NoLowResPictureLayerImplTest.InvalidViewportForPrioritizingTiles
> PictureLayerImplTest.InvalidViewportForPrioritizingTiles

Maybe you should just remove the test, since the whole point of this is viewport
for prioritizing tiles is never invalid.

> 
> I will fix the tests in the next patch.

[hush (inactive), 2014-08-29 01:59:25]

On 2014/08/29 01:51:27, boliu wrote:
> On 2014/08/29 01:30:15, hush wrote:
> > I broke:    NoLowResPictureLayerImplTest.InvalidViewportForPrioritizingTiles
> > PictureLayerImplTest.InvalidViewportForPrioritizingTiles
> 
> Maybe you should just remove the test, since the whole point of this is
viewport
> for prioritizing tiles is never invalid.
> 
> > 
> > I will fix the tests in the next patch.

I just changed the test to assert PictureLayerImpl does respect the
matrix/viewport from resourceless software draw

[boliu, 2014-08-29 02:01:42]

https://codereview.chromium.org/517893002/diff/60001/content/browser/android/...
File content/browser/android/in_process/synchronous_compositor_output_surface.cc
(left):

https://codereview.chromium.org/517893002/diff/60001/content/browser/android/...
content/browser/android/in_process/synchronous_compositor_output_surface.cc:244:
SetExternalDrawConstraints(cached_hw_transform_,
we still want this to restore draw viewports and whatnot

[hush (inactive), 2014-08-29 02:20:57]

https://codereview.chromium.org/517893002/diff/60001/content/browser/android/...
File content/browser/android/in_process/synchronous_compositor_output_surface.cc
(left):

https://codereview.chromium.org/517893002/diff/60001/content/browser/android/...
content/browser/android/in_process/synchronous_compositor_output_surface.cc:244:
SetExternalDrawConstraints(cached_hw_transform_,
On 2014/08/29 02:01:41, boliu wrote:
> we still want this to restore draw viewports and whatnot

Done.

[boliu, 2014-08-29 02:31:10]

everything lgtm, though I'm not owner of anything :p

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

https://codereview.chromium.org/517893002/diff/80001/cc/layers/picture_layer_...
cc/layers/picture_layer_impl_unittest.cc:464: // respeced.
"respeced" spelling

[hush (inactive), 2014-08-29 03:03:09]

hush@chromium.org changed reviewers:
+ aelias@chromium.org

[hush (inactive), 2014-08-29 03:03:09]

Alex,
PTAL

[boliu, 2014-08-29 03:16:12]

On 2014/08/29 03:03:09, hush wrote:
> Alex,
> PTAL

Alex just went on vacation..

[aelias_OOO_until_Jul13, 2014-08-29 04:41:00]

lgtm except, please update your patch description to describe what you're doing
and why.

[hush (inactive), 2014-08-29 17:08:28]

On 2014/08/29 04:41:00, aelias wrote:
> lgtm except, please update your patch description to describe what you're
doing
> and why.

Thanks!

[hush (inactive), 2014-08-29 17:08:41]

The CQ bit was checked by hush@chromium.org

[commit-bot: I haz the power, 2014-08-29 17:09:09]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hush@chromium.org/517893002/100001

[boliu, 2014-08-29 17:09:10]

On 2014/08/29 17:08:28, hush wrote:
> On 2014/08/29 04:41:00, aelias wrote:
> > lgtm except, please update your patch description to describe what you're
> doing
> > and why.
> 
> Thanks!

Make the first line a short summary, then leave a blank line after

[boliu, 2014-08-29 17:10:05]

The CQ bit was unchecked by boliu@chromium.org

[danakj, 2014-08-29 17:11:30]

danakj@chromium.org changed reviewers:
+ sievers@chromium.org

[danakj, 2014-08-29 17:11:31]

+sievers for content/browser

[hush (inactive), 2014-08-29 17:11:32]

The CQ bit was checked by hush@chromium.org

[commit-bot: I haz the power, 2014-08-29 17:12:14]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hush@chromium.org/517893002/100001

[commit-bot: I haz the power, 2014-08-29 18:16:29]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[commit-bot: I haz the power, 2014-08-29 18:21:41]

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

[commit-bot: I haz the power, 2014-08-29 18:21:42]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[hush (inactive), 2014-08-29 20:41:03]

Daniel,
PTAL content/.
Thanks!

[no sievers, 2014-08-29 23:21:13]

On 2014/08/29 20:41:03, hush wrote:
> Daniel,
> PTAL content/.
> Thanks!

lgtm

[hush (inactive), 2014-08-29 23:32:30]

The CQ bit was checked by hush@chromium.org

[commit-bot: I haz the power, 2014-08-29 23:38:58]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hush@chromium.org/517893002/100001

[commit-bot: I haz the power, 2014-08-29 23:56:51]

Committed patchset #5 (id:100001) as 1e4228f4c32e6a14553861e249d634cc567bad91

[commit-bot: I haz the power, 2014-09-10 03:11:57]

Patchset 5 (id:??) landed as
https://crrev.com/367d7dd63fb48978cf28918397a8269f368e6f27
Cr-Commit-Position: refs/heads/master@{#292723}