aw: Fallback delegated composite

aw: Fallback delegated composite

If BrowserViewRenderer already has hardware acceleration
enabled, then fallback tick can produce a real instead of a
wasted 1x1 software frame. And to avoid the bug, we can force
this frame through the pipeline ignoring stalls.

BUG=424002
Committed: https://crrev.com/96ba1a63f02a9c8842d985712fa440f9a28bcf20
Cr-Commit-Position: refs/heads/master@{#299824}

[boliu, 2014-10-15 17:23:35]

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

[boliu, 2014-10-15 17:23:36]

Still testing, but I want to submit this before the big refactors so it's easier
to merge

[boliu, 2014-10-15 17:43:33]

On 2014/10/15 17:23:36, boliu wrote:
> Still testing, but I want to submit this before the big refactors so it's
easier
> to merge

Has a bug that parent is not properly releasing resources back to child

[boliu, 2014-10-15 18:01:26]

https://codereview.chromium.org/659743002/diff/20001/android_webview/browser/...
File android_webview/browser/hardware_renderer.cc (right):

https://codereview.chromium.org/659743002/diff/20001/android_webview/browser/...
android_webview/browser/hardware_renderer.cc:211: SetFrameData();
Hmm, my guess is it doesn't like having 2 frames shoved into the layer at once??
That would really suck and make this more complicated...

[hush (inactive), 2014-10-15 18:17:31]

https://codereview.chromium.org/659743002/diff/20001/android_webview/browser/...
File android_webview/browser/browser_view_renderer.cc (left):

https://codereview.chromium.org/659743002/diff/20001/android_webview/browser/...
android_webview/browser/browser_view_renderer.cc:16: #include
"cc/output/compositor_frame.h"
why is this removed? we still use cc::compositorFrame right?

https://codereview.chromium.org/659743002/diff/20001/android_webview/browser/...
File android_webview/browser/browser_view_renderer.cc (right):

https://codereview.chromium.org/659743002/diff/20001/android_webview/browser/...
android_webview/browser/browser_view_renderer.cc:722:
ReturnUnusedResource(shared_renderer_state_->PassCompositorFrame());
there is also a frame in HardwareRenderer.commited_frame_ that could contain
some resources. I think you need to return unused resources too, if you are not
going to SetFrameData twice in HardwareRenderer::DrawGL

https://codereview.chromium.org/659743002/diff/20001/android_webview/browser/...
File android_webview/browser/hardware_renderer.cc (right):

https://codereview.chromium.org/659743002/diff/20001/android_webview/browser/...
android_webview/browser/hardware_renderer.cc:150: committed_frame_ =
shared_renderer_state_->PassCompositorFrame();
since ForceCommmit() stays true until the next onDraw produces a frame to
SharedRendererState, commitFrame() could be called multiple times before
DrawGL() right? So committed_frame_ produced by the fallback tick may not be
consumed by DrawGL.

https://codereview.chromium.org/659743002/diff/20001/android_webview/browser/...
android_webview/browser/hardware_renderer.cc:211: SetFrameData();
On 2014/10/15 18:01:25, boliu wrote:
> Hmm, my guess is it doesn't like having 2 frames shoved into the layer at
once??
> That would really suck and make this more complicated...

you committed frame data twice. I don't know what that does.
Supposedly your fallback tick's frame data is most recent, and you can just drop
the previous frames' data.

https://codereview.chromium.org/659743002/diff/20001/android_webview/browser/...
File android_webview/browser/shared_renderer_state.cc (right):

https://codereview.chromium.org/659743002/diff/20001/android_webview/browser/...
android_webview/browser/shared_renderer_state.cc:153: force_commit_ =
force_commit;
that means there is a period of time that force_commit stays true after the HW
accelerated fallback tick's frame is committed to HardwareRenderer, but the next
onDraw hasn't put any frames to SharedRendererState yet..


why doesn't force_commit go together with compositorFrame, in a struct or
something?

[boliu, 2014-10-15 19:37:52]

https://codereview.chromium.org/659743002/diff/20001/android_webview/browser/...
File android_webview/browser/browser_view_renderer.cc (left):

https://codereview.chromium.org/659743002/diff/20001/android_webview/browser/...
android_webview/browser/browser_view_renderer.cc:16: #include
"cc/output/compositor_frame.h"
On 2014/10/15 18:17:30, hush wrote:
> why is this removed? we still use cc::compositorFrame right?

Added back.

https://codereview.chromium.org/659743002/diff/20001/android_webview/browser/...
File android_webview/browser/browser_view_renderer.cc (right):

https://codereview.chromium.org/659743002/diff/20001/android_webview/browser/...
android_webview/browser/browser_view_renderer.cc:722:
ReturnUnusedResource(shared_renderer_state_->PassCompositorFrame());
On 2014/10/15 18:17:30, hush wrote:
> there is also a frame in HardwareRenderer.commited_frame_ that could contain
> some resources. I think you need to return unused resources too, if you are
not
> going to SetFrameData twice in HardwareRenderer::DrawGL

I am calling SetFrameData twice in HWR::DrawGL :)

https://codereview.chromium.org/659743002/diff/20001/android_webview/browser/...
File android_webview/browser/hardware_renderer.cc (right):

https://codereview.chromium.org/659743002/diff/20001/android_webview/browser/...
android_webview/browser/hardware_renderer.cc:150: committed_frame_ =
shared_renderer_state_->PassCompositorFrame();
On 2014/10/15 18:17:30, hush wrote:
> since ForceCommmit() stays true until the next onDraw produces a frame to
> SharedRendererState, commitFrame() could be called multiple times before
> DrawGL() right? So committed_frame_ produced by the fallback tick may not be
> consumed by DrawGL.

As discussed, fixed. Just don't have the new condition as it's not needed.

https://codereview.chromium.org/659743002/diff/20001/android_webview/browser/...
File android_webview/browser/shared_renderer_state.cc (right):

https://codereview.chromium.org/659743002/diff/20001/android_webview/browser/...
android_webview/browser/shared_renderer_state.cc:153: force_commit_ =
force_commit;
On 2014/10/15 18:17:31, hush wrote:
> that means there is a period of time that force_commit stays true after the HW
> accelerated fallback tick's frame is committed to HardwareRenderer, but the
next
> onDraw hasn't put any frames to SharedRendererState yet..
> 
> 
> why doesn't force_commit go together with compositorFrame, in a struct or
> something?

Tried that, but it's really weird to ask something about a frame without taking
ownership of it, so I thought this was cleaner.

[hush (inactive), 2014-10-15 21:10:37]

lgtm

https://codereview.chromium.org/659743002/diff/40001/android_webview/browser/...
File android_webview/browser/shared_renderer_state.h (right):

https://codereview.chromium.org/659743002/diff/40001/android_webview/browser/...
android_webview/browser/shared_renderer_state.h:44: bool ForceCommit();
add a const in the end?

[boliu, 2014-10-16 01:20:11]

https://codereview.chromium.org/659743002/diff/40001/android_webview/browser/...
File android_webview/browser/shared_renderer_state.h (right):

https://codereview.chromium.org/659743002/diff/40001/android_webview/browser/...
android_webview/browser/shared_renderer_state.h:44: bool ForceCommit();
On 2014/10/15 21:10:37, hush wrote:
> add a const in the end?

Done.

[boliu, 2014-10-16 01:20:23]

The CQ bit was checked by boliu@chromium.org

[commit-bot: I haz the power, 2014-10-16 01:23:20]

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

[commit-bot: I haz the power, 2014-10-16 02:30:03]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-10-16 02:31:07]

Patchset 4 (id:??) landed as
https://crrev.com/96ba1a63f02a9c8842d985712fa440f9a28bcf20
Cr-Commit-Position: refs/heads/master@{#299824}