RELAND: CONTENT: Optimize SurfaceFlinger overlays - Step 1

RELAND: CONTENT: Optimize SurfaceFlinger overlays - Step 1

Currently Chrome has two fullscreen overlays. There is a
hidden optimization where Android detects transparent areas
on top of SurfaceViews and removes them from SurfaceFlinger
compositing. In order to get the most from this
optimization, we need to set all views in the viewport to
be either:
a.) Not visible (+layout to trigger it)
b.) setWillNotDraw(true) with no visible back/foreground.

ContentViews in general don't draw, so we remove
setWillNotDraw(false). For WebView we do need this now
though, so move setWillNotDraw(false) into AwContents.

BUG=342541
Committed: https://crrev.com/ac4ad5a52fca8fc4b7ec24d32df9efee93a4fa51
Cr-Commit-Position: refs/heads/master@{#301940}

[epenner, 2014-10-25 01:53:00]

epenner@chromium.org changed reviewers:
+ boliu@chromium.org, mkosiba@chromium.org, tedchoc@chromium.org

[epenner, 2014-10-25 01:54:14]

Ptal. This android_webview_apk works fine with this now.

[mkosiba (inactive), 2014-10-27 11:48:25]

https://codereview.chromium.org/675313002/diff/1/android_webview/java/src/org...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/675313002/diff/1/android_webview/java/src/org...
android_webview/java/src/org/chromium/android_webview/AwContents.java:635:
mContainerView.setWillNotDraw(false);
you probably want to put this in the AwContents() constructor above.

[boliu, 2014-10-27 15:19:39]

https://codereview.chromium.org/675313002/diff/1/android_webview/java/src/org...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/675313002/diff/1/android_webview/java/src/org...
android_webview/java/src/org/chromium/android_webview/AwContents.java:635:
mContainerView.setWillNotDraw(false);
On 2014/10/27 11:48:24, mkosiba wrote:
> you probably want to put this in the AwContents() constructor above.

Hmm, need to be careful with this. What if the container view changes (pop ups,
full screen video)?

Is it possible to put setWillNotDraw(true) to somewhere under chrome/ instead of
in content/? Then it won't affect webview.

[mkosiba (inactive), 2014-10-27 16:13:58]

On 2014/10/27 15:19:39, boliu wrote:
>
https://codereview.chromium.org/675313002/diff/1/android_webview/java/src/org...
> File android_webview/java/src/org/chromium/android_webview/AwContents.java
> (right):
> 
>
https://codereview.chromium.org/675313002/diff/1/android_webview/java/src/org...
> android_webview/java/src/org/chromium/android_webview/AwContents.java:635:
> mContainerView.setWillNotDraw(false);
> On 2014/10/27 11:48:24, mkosiba wrote:
> > you probably want to put this in the AwContents() constructor above.
> 
> Hmm, need to be careful with this. What if the container view changes (pop
ups,
> full screen video)?

there are 2 places in AwContents where we set mContainerView:
 1) the constructor,
 2) the setContainerView method.

> 
> Is it possible to put setWillNotDraw(true) to somewhere under chrome/ instead
of
> in content/? Then it won't affect webview.

what? but we want this. Otherwise the WebView stops drawing. The change goes
from
[Chrome on Android AND WebView call setWillNotDraw(true)] to [only WebView calls
setWillNotDraw(true)].

[boliu, 2014-10-27 16:26:03]

On 2014/10/27 16:13:58, mkosiba wrote:
> On 2014/10/27 15:19:39, boliu wrote:
> >
>
https://codereview.chromium.org/675313002/diff/1/android_webview/java/src/org...
> > File android_webview/java/src/org/chromium/android_webview/AwContents.java
> > (right):
> > 
> >
>
https://codereview.chromium.org/675313002/diff/1/android_webview/java/src/org...
> > android_webview/java/src/org/chromium/android_webview/AwContents.java:635:
> > mContainerView.setWillNotDraw(false);
> > On 2014/10/27 11:48:24, mkosiba wrote:
> > > you probably want to put this in the AwContents() constructor above.
> > 
> > Hmm, need to be careful with this. What if the container view changes (pop
> ups,
> > full screen video)?
> 
> there are 2 places in AwContents where we set mContainerView:
>  1) the constructor,
>  2) the setContainerView method.

That sgtm

> 
> > 
> > Is it possible to put setWillNotDraw(true) to somewhere under chrome/
instead
> of
> > in content/? Then it won't affect webview.
> 
> what? but we want this. Otherwise the WebView stops drawing. The change goes
> from
> [Chrome on Android AND WebView call setWillNotDraw(true)] to [only WebView
calls
> setWillNotDraw(true)].

Scratch that. Bad idea on my part. Didn't understand the change.

[Ted C, 2014-10-29 00:05:52]

On 2014/10/27 16:26:03, boliu wrote:
> On 2014/10/27 16:13:58, mkosiba wrote:
> > On 2014/10/27 15:19:39, boliu wrote:
> > >
> >
>
https://codereview.chromium.org/675313002/diff/1/android_webview/java/src/org...
> > > File android_webview/java/src/org/chromium/android_webview/AwContents.java
> > > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/675313002/diff/1/android_webview/java/src/org...
> > > android_webview/java/src/org/chromium/android_webview/AwContents.java:635:
> > > mContainerView.setWillNotDraw(false);
> > > On 2014/10/27 11:48:24, mkosiba wrote:
> > > > you probably want to put this in the AwContents() constructor above.
> > > 
> > > Hmm, need to be careful with this. What if the container view changes (pop
> > ups,
> > > full screen video)?
> > 
> > there are 2 places in AwContents where we set mContainerView:
> >  1) the constructor,
> >  2) the setContainerView method.
> 
> That sgtm
> 
> > 
> > > 
> > > Is it possible to put setWillNotDraw(true) to somewhere under chrome/
> instead
> > of
> > > in content/? Then it won't affect webview.
> > 
> > what? but we want this. Otherwise the WebView stops drawing. The change goes
> > from
> > [Chrome on Android AND WebView call setWillNotDraw(true)] to [only WebView
> calls
> > setWillNotDraw(true)].
> 
> Scratch that. Bad idea on my part. Didn't understand the change.

still lgtm for the content/chrome side of the world

[epennerAtGoogle, 2014-10-29 20:35:34]

epenner@google.com changed reviewers:
+ epenner@google.com

[epennerAtGoogle, 2014-10-29 20:35:35]

Addressed feedback. Ptal.

Bo, can I get an android_webview OWNERS lgtm?

https://codereview.chromium.org/675313002/diff/1/android_webview/java/src/org...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/675313002/diff/1/android_webview/java/src/org...
android_webview/java/src/org/chromium/android_webview/AwContents.java:635:
mContainerView.setWillNotDraw(false);
On 2014/10/27 11:48:24, mkosiba wrote:
> you probably want to put this in the AwContents() constructor above.

Ahh, that's nicer. I was thinking in terms of the old
contentViewCore.initialize where it happened before. I've changed it such that
the setWillNotDraw is always immediately following "mContainerView =", in order
to make it even more clear.

[epennerAtGoogle, 2014-10-29 20:37:08]

On 2014/10/29 20:35:35, epennerAtGoogle wrote:
> Addressed feedback. Ptal.
> 
> Bo, can I get an android_webview OWNERS lgtm?

Or Martin of course (if you are still there in that time-zone ;) )

[boliu, 2014-10-29 20:39:54]

lgtm

[epennerAtGoogle, 2014-10-29 20:45:28]

The CQ bit was checked by epenner@google.com

[commit-bot: I haz the power, 2014-10-29 20:46:22]

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

[commit-bot: I haz the power, 2014-10-29 21:43:06]

Committed patchset #4 (id:50003)

[commit-bot: I haz the power, 2014-10-29 21:44:08]

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