Remove android webview rendering cruft

Remove android webview rendering cruft

Remove pipelining and blocking the pipeline in different stages. The
underlying issue it was trying to workaround is fixed in Android, and it
involves a lot of complexity and was a source of bugs.

Also remove special handling when viewport is empty, as this is handled
correctly now.

Add back the IsEmpty corner case workaround by passing some frame
metadata with ChildFrame.

BUG=

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

[boliu, 2015-03-12 00:38:41]

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

[boliu, 2015-03-12 00:38:41]

Uploading for closer inspection. I just mostly blindly tried to apply
https://codereview.chromium.org/914103004/, and surprisingly compiled on first
try.

Haven't gone through all the details yet.

[hush (inactive), 2015-03-13 18:45:17]

https://codereview.chromium.org/1001643004/diff/1/android_webview/browser/bro...
File android_webview/browser/browser_view_renderer.cc (right):

https://codereview.chromium.org/1001643004/diff/1/android_webview/browser/bro...
android_webview/browser/browser_view_renderer.cc:261:
ReturnUnusedResource(shared_renderer_state_.PassUncommittedFrameOnUI());
so this is the case where the kModeDraw does not come fast enough and child
compositor has produced more frames than parent compositor could consume, right?

https://codereview.chromium.org/1001643004/diff/1/android_webview/browser/har...
File android_webview/browser/hardware_renderer.cc (right):

https://codereview.chromium.org/1001643004/diff/1/android_webview/browser/har...
android_webview/browser/hardware_renderer.cc:147: // Happens with empty global
visible rect.
Remove this comment here? because an empty globall visible rect does not mean
the produced frame is empty.

https://codereview.chromium.org/1001643004/diff/1/android_webview/browser/sha...
File android_webview/browser/shared_renderer_state.h (left):

https://codereview.chromium.org/1001643004/diff/1/android_webview/browser/sha...
android_webview/browser/shared_renderer_state.h:50: void
SetForceInvalidateOnNextDrawGLOnUI(
We've talked about this:
We need to keep this otherwise we break
https://codereview.chromium.org/603633003/

https://codereview.chromium.org/1001643004/diff/1/android_webview/browser/sha...
android_webview/browser/shared_renderer_state.h:111: bool
needs_force_invalidate_on_next_draw_gl_;
need to keep it

[boliu, 2015-03-18 02:02:23]

Ok, so as discussed, instead of bringing back
SetForceInvalidateOnNextDrawGLOnUI, I'm doing a different fix for that bug that
became available after this refactor :D

https://codereview.chromium.org/1001643004/diff/1/android_webview/browser/bro...
File android_webview/browser/browser_view_renderer.cc (right):

https://codereview.chromium.org/1001643004/diff/1/android_webview/browser/bro...
android_webview/browser/browser_view_renderer.cc:261:
ReturnUnusedResource(shared_renderer_state_.PassUncommittedFrameOnUI());
On 2015/03/13 18:45:17, hush wrote:
> so this is the case where the kModeDraw does not come fast enough and child
> compositor has produced more frames than parent compositor could consume,
right?

This is copied from fallback tick run. So if there are 2 consecutive fallback
ticks, then we need to return the previous frame. I'll add a comment.

https://codereview.chromium.org/1001643004/diff/1/android_webview/browser/har...
File android_webview/browser/hardware_renderer.cc (right):

https://codereview.chromium.org/1001643004/diff/1/android_webview/browser/har...
android_webview/browser/hardware_renderer.cc:147: // Happens with empty global
visible rect.
On 2015/03/13 18:45:17, hush wrote:
> Remove this comment here? because an empty globall visible rect does not mean
> the produced frame is empty.

Removed comment, but still need that check. We want to move in a direction where
child compositor is free to not swap if it doesn't want to.

[hush (inactive), 2015-03-18 22:29:27]

https://codereview.chromium.org/1001643004/diff/60001/android_webview/browser...
File android_webview/browser/parent_compositor_draw_constraints.cc (right):

https://codereview.chromium.org/1001643004/diff/60001/android_webview/browser...
android_webview/browser/parent_compositor_draw_constraints.cc:22: bool
ParentCompositorDrawConstraints::NeedUpdate(
I'm kind of confused by the logic of NeedUpdate.
In the case of animating in and out of the screen, the last constraint before
going offscreen and the first constraint after going onscreen could be the same.
And in this case, the hardware renderer should force the draw constraints to
SRS.
But NeedUpdate will return false if the constraints are the same?

https://codereview.chromium.org/1001643004/diff/60001/android_webview/browser...
android_webview/browser/parent_compositor_draw_constraints.cc:29: // Only care
about surface size when we are not a layer and not in pre-raster
I think the comment is "only care about surface size when we are on a layer and
not in pre-raster mode."

https://codereview.chromium.org/1001643004/diff/60001/android_webview/browser...
android_webview/browser/parent_compositor_draw_constraints.cc:30: // is turned
on.
remove "is turned on"

[boliu, 2015-03-18 22:34:08]

https://codereview.chromium.org/1001643004/diff/60001/android_webview/browser...
File android_webview/browser/parent_compositor_draw_constraints.cc (right):

https://codereview.chromium.org/1001643004/diff/60001/android_webview/browser...
android_webview/browser/parent_compositor_draw_constraints.cc:22: bool
ParentCompositorDrawConstraints::NeedUpdate(
On 2015/03/18 22:29:27, hush wrote:
> I'm kind of confused by the logic of NeedUpdate.
> In the case of animating in and out of the screen, the last constraint before
> going offscreen and the first constraint after going onscreen could be the
same.
> And in this case, the hardware renderer should force the draw constraints to
> SRS.
> But NeedUpdate will return false if the constraints are the same?

You are right.

To get the same effect, we need to pass the draw viewport here, and check if
it's empty here.. (or maybe just compare the size to surface_rect.size())

[boliu, 2015-03-18 23:32:52]

Ok, IsEmpty check added back..

[boliu, 2015-03-18 23:33:52]

Argg, PS5 contains some unrelated changes.. one sec

[boliu, 2015-03-18 23:36:38]

Ok, now diff of PS6 vs PS4 makes sense now.

[hush (inactive), 2015-03-18 23:55:09]

lgtm with minor comments

https://codereview.chromium.org/1001643004/diff/100001/android_webview/browse...
File android_webview/browser/child_frame.h (right):

https://codereview.chromium.org/1001643004/diff/100001/android_webview/browse...
android_webview/browser/child_frame.h:29: const gfx::Rect
viewport_for_tile_priority;
can you change it to viewport_rect_for_tile_priority for the sake of
consistency?

[boliu, 2015-03-18 23:59:28]

renamed the var and clang-formatted the patch

[boliu, 2015-03-19 00:01:52]

The CQ bit was checked by boliu@chromium.org

[boliu, 2015-03-19 00:01:52]

The patchset sent to the CQ was uploaded after l-g-t-m from hush@chromium.org
Link to the patchset: https://codereview.chromium.org/1001643004/#ps120001
(title: "rename var")

[commit-bot: I haz the power, 2015-03-19 00:02:23]

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

[commit-bot: I haz the power, 2015-03-19 00:29:38]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-03-19 00:33:31]

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