aw: Fix FBO restore in webview functor

aw: Fix FBO restore in webview functor

See bug for problem description.

Have AwGLSurface get the current FBO directly from
ScopedAppGLStateRestore. Conceptually ScopedAppGLStateRestore would be
in the ui/gl layer, so having AwGLSurface access it directly isn't
really a problem.

Seems a lot cleaner than dependency injection which involves crossing
many (conceptual) layers anyway. Global is bad, but any time GL executes
without ScopedAppGLStateRestore would indicate bigger bugs, so in
practice should be ok.

BUG=649738
Committed: https://crrev.com/cf4731774e0d1110244be09be8cb456f83f5ce44
Cr-Commit-Position: refs/heads/master@{#420913}

[boliu, 2016-09-23 18:26:40]

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

[boliu, 2016-09-23 18:26:41]

ptal

[Tobias Sargeant, 2016-09-26 09:46:18]

LGTM. Couple of questions below.

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

https://codereview.chromium.org/2360423003/diff/20001/android_webview/browser...
android_webview/browser/hardware_renderer.cc:75: const ScopedAppGLStateRestore&
gl_state) {
If we make ScopedAppGLStateRestore global, maybe we should stop passing it
around?

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

https://codereview.chromium.org/2360423003/diff/20001/android_webview/browser...
android_webview/browser/render_thread_manager.cc:339: DrawGL(&info);
Could this cause DCHECKs now, given that we could potentially create a
ScopedAppGLStateResore on the UI thread while the render thread object was in
scope?

[boliu, 2016-09-26 15:54:52]

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

https://codereview.chromium.org/2360423003/diff/20001/android_webview/browser...
android_webview/browser/hardware_renderer.cc:75: const ScopedAppGLStateRestore&
gl_state) {
On 2016/09/26 09:46:18, Tobias Sargeant wrote:
> If we make ScopedAppGLStateRestore global, maybe we should stop passing it
> around?

Done.

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

https://codereview.chromium.org/2360423003/diff/20001/android_webview/browser...
android_webview/browser/render_thread_manager.cc:339: DrawGL(&info);
On 2016/09/26 09:46:18, Tobias Sargeant wrote:
> Could this cause DCHECKs now, given that we could potentially create a
> ScopedAppGLStateResore on the UI thread while the render thread object was in
> scope?

Probably fine.. This is not supposed to happen in practice so failing DCHECKs is
ok. But it may crash without DCHECKs. If that's a problem in practice, we can
use a threadlocal I guess.

[boliu, 2016-09-26 15:55:04]

The CQ bit was checked by boliu@chromium.org

[boliu, 2016-09-26 15:55:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
tobiasjs@chromium.org
Link to the patchset: https://codereview.chromium.org/2360423003/#ps40001
(title: "review")

[commit-bot: I haz the power, 2016-09-26 15:55:29]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-26 17:08:05]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-09-26 17:09:40]

Description was changed from

==========
aw: Fix FBO restore in webview functor

See bug for problem description.

Have AwGLSurface get the current FBO directly from
ScopedAppGLStateRestore. Conceptually ScopedAppGLStateRestore would be
in the ui/gl layer, so having AwGLSurface access it directly isn't
really a problem.

Seems a lot cleaner than dependency injection which involves crossing
many (conceptual) layers anyway. Global is bad, but any time GL executes
without ScopedAppGLStateRestore would indicate bigger bugs, so in
practice should be ok.

BUG=649738
==========

to

==========
aw: Fix FBO restore in webview functor

See bug for problem description.

Have AwGLSurface get the current FBO directly from
ScopedAppGLStateRestore. Conceptually ScopedAppGLStateRestore would be
in the ui/gl layer, so having AwGLSurface access it directly isn't
really a problem.

Seems a lot cleaner than dependency injection which involves crossing
many (conceptual) layers anyway. Global is bad, but any time GL executes
without ScopedAppGLStateRestore would indicate bigger bugs, so in
practice should be ok.

BUG=649738

Committed: https://crrev.com/cf4731774e0d1110244be09be8cb456f83f5ce44
Cr-Commit-Position: refs/heads/master@{#420913}
==========

[commit-bot: I haz the power, 2016-09-26 17:09:41]

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