Move AW renderer compositor context to gpu thread

Move AW renderer compositor context to gpu thread

Use TexSubImage2D instead of TexImage2D to avoid orphaning
EGLImage
Create the renderer compositor context on the gpu thread.

BUG=448168, 259924
Committed: https://crrev.com/a4baed454be2bb165cb12eb7df432e9138e746e6
Cr-Commit-Position: refs/heads/master@{#313846}

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

[boliu, 2015-01-14 23:59:00]

boliu@chromium.org changed reviewers:
+ epenner@chromium.org, sievers@chromium.org

[boliu, 2015-01-14 23:59:01]

PTAL :)

[epenner, 2015-01-15 00:28:23]

LGTM (optional nit)

https://codereview.chromium.org/769703005/diff/60001/gpu/command_buffer/servi...
File gpu/command_buffer/service/async_pixel_transfer_manager_android.cc (right):

https://codereview.chromium.org/769703005/diff/60001/gpu/command_buffer/servi...
gpu/command_buffer/service/async_pixel_transfer_manager_android.cc:88: bool
threaded_texture_mailboxes = TransferWithThreadedTextureMailboxes();
Nit.

Mildly prefer reversing the bool to be "use_teximage2d_over_texsubimage2d", and
then having an additional variable here (to be self documenting):
bool use_teximage2d_over_texsubimage2d = !threaded_texture_mailboxes;

[epenner, 2015-01-15 00:29:34]

> LGTM (optional nit)

Correction: LGTM for async transfers.

[no sievers, 2015-01-15 20:26:05]

https://codereview.chromium.org/769703005/diff/60001/content/browser/android/...
File content/browser/android/in_process/synchronous_compositor_factory_impl.cc
(right):

https://codereview.chromium.org/769703005/diff/60001/content/browser/android/...
content/browser/android/in_process/synchronous_compositor_factory_impl.cc:303:
service_, gpu::GLInProcessContextSharedMemoryLimits(), false));
Can you put a comment that this needs to be treated as 'onscreen' because it
runs in the shared onscreen context (due to SurfaceTexture limitations when it
comes to share groups and such)?

https://codereview.chromium.org/769703005/diff/60001/gpu/command_buffer/servi...
File gpu/command_buffer/service/async_pixel_transfer_manager_android.cc (right):

https://codereview.chromium.org/769703005/diff/60001/gpu/command_buffer/servi...
gpu/command_buffer/service/async_pixel_transfer_manager_android.cc:71:
cl->HasSwitch(switches::kEnableAsyncPixelWithThreadedTextureMailboxes);
Do we really need the other command line
(kEnableAsyncPixelWithThreadedTextureMailboxes)?

https://codereview.chromium.org/769703005/diff/60001/gpu/command_buffer/servi...
gpu/command_buffer/service/async_pixel_transfer_manager_android.cc:102: }
Can you put a comment that we do this because of how threaded mailboxes use
EGLimage and that causes problems with the orphaning on every upload if we used
texImage2D?

https://codereview.chromium.org/769703005/diff/60001/gpu/command_buffer/servi...
File gpu/command_buffer/service/mailbox_manager_sync.cc (right):

https://codereview.chromium.org/769703005/diff/60001/gpu/command_buffer/servi...
gpu/command_buffer/service/mailbox_manager_sync.cc:52: texture->pool() ==
GL_TEXTURE_POOL_MANAGED_CHROMIUM) {
Can we just remove the workaround entirely?
We shouldn't hit the problem if we always disable the EGL image uploader when
MailboxManagerSync is in use.

[boliu, 2015-01-21 02:08:01]

https://codereview.chromium.org/769703005/diff/60001/content/browser/android/...
File content/browser/android/in_process/synchronous_compositor_factory_impl.cc
(right):

https://codereview.chromium.org/769703005/diff/60001/content/browser/android/...
content/browser/android/in_process/synchronous_compositor_factory_impl.cc:303:
service_, gpu::GLInProcessContextSharedMemoryLimits(), false));
On 2015/01/15 20:26:05, sievers wrote:
> Can you put a comment that this needs to be treated as 'onscreen' because it
> runs in the shared onscreen context (due to SurfaceTexture limitations when it
> comes to share groups and such)?

I put something generic. Can you look again?

What is the share group limitation? I thought the issue is SurfaceTexture uses
eglImages underneath so can't be shared across threads with mailboxes.

https://codereview.chromium.org/769703005/diff/60001/gpu/command_buffer/servi...
File gpu/command_buffer/service/async_pixel_transfer_manager_android.cc (right):

https://codereview.chromium.org/769703005/diff/60001/gpu/command_buffer/servi...
gpu/command_buffer/service/async_pixel_transfer_manager_android.cc:71:
cl->HasSwitch(switches::kEnableAsyncPixelWithThreadedTextureMailboxes);
On 2015/01/15 20:26:05, sievers wrote:
> Do we really need the other command line
> (kEnableAsyncPixelWithThreadedTextureMailboxes)?
> 

I wanted the new one as a fallback in case anything in the new paths breaks..

But I guess there is plenty of time until m42 branch. Just need to do a bit more
exhaustive testing before landing this. Removing the switch.

https://codereview.chromium.org/769703005/diff/60001/gpu/command_buffer/servi...
gpu/command_buffer/service/async_pixel_transfer_manager_android.cc:88: bool
threaded_texture_mailboxes = TransferWithThreadedTextureMailboxes();
On 2015/01/15 00:28:23, epenner wrote:
> Nit.
> 
> Mildly prefer reversing the bool to be "use_teximage2d_over_texsubimage2d",
and
> then having an additional variable here (to be self documenting):
> bool use_teximage2d_over_texsubimage2d = !threaded_texture_mailboxes;

Done.

https://codereview.chromium.org/769703005/diff/60001/gpu/command_buffer/servi...
gpu/command_buffer/service/async_pixel_transfer_manager_android.cc:102: }
On 2015/01/15 20:26:05, sievers wrote:
> Can you put a comment that we do this because of how threaded mailboxes use
> EGLimage and that causes problems with the orphaning on every upload if we
used
> texImage2D?

Done.

[no sievers, 2015-01-23 21:48:35]

lgtm

https://codereview.chromium.org/769703005/diff/60001/content/browser/android/...
File content/browser/android/in_process/synchronous_compositor_factory_impl.cc
(right):

https://codereview.chromium.org/769703005/diff/60001/content/browser/android/...
content/browser/android/in_process/synchronous_compositor_factory_impl.cc:303:
service_, gpu::GLInProcessContextSharedMemoryLimits(), false));
On 2015/01/21 02:08:01, boliu wrote:
> On 2015/01/15 20:26:05, sievers wrote:
> > Can you put a comment that this needs to be treated as 'onscreen' because it
> > runs in the shared onscreen context (due to SurfaceTexture limitations when
it
> > comes to share groups and such)?
> 
> I put something generic. Can you look again?
> 
> What is the share group limitation? I thought the issue is SurfaceTexture uses
> eglImages underneath so can't be shared across threads with mailboxes.

Yes you cannot create another texture from a SurfaceTexture. You can only have
one and the API is a bit weird. For example, you can detach/attach it to a
different context, but then it will delete your texture (although the caller
created it). Also, it doesn't know about GL share groups but does a pointer
comparison (!) in many APIs for the context it's used with.

[boliu, 2015-01-23 21:58:40]

Holding off on landing this until I figure out the webgl flicker problem. With
this I can sometimes see corrupt tiles on webgl pages.

And probably needs more general testing.

On 2015/01/23 21:48:35, sievers wrote:
> lgtm
> 
>
https://codereview.chromium.org/769703005/diff/60001/content/browser/android/...
> File content/browser/android/in_process/synchronous_compositor_factory_impl.cc
> (right):
> 
>
https://codereview.chromium.org/769703005/diff/60001/content/browser/android/...
> content/browser/android/in_process/synchronous_compositor_factory_impl.cc:303:
> service_, gpu::GLInProcessContextSharedMemoryLimits(), false));
> On 2015/01/21 02:08:01, boliu wrote:
> > On 2015/01/15 20:26:05, sievers wrote:
> > > Can you put a comment that this needs to be treated as 'onscreen' because
it
> > > runs in the shared onscreen context (due to SurfaceTexture limitations
when
> it
> > > comes to share groups and such)?
> > 
> > I put something generic. Can you look again?
> > 
> > What is the share group limitation? I thought the issue is SurfaceTexture
uses
> > eglImages underneath so can't be shared across threads with mailboxes.
> 
> Yes you cannot create another texture from a SurfaceTexture. You can only have
> one and the API is a bit weird. For example, you can detach/attach it to a
> different context, but then it will delete your texture (although the caller
> created it). Also, it doesn't know about GL share groups but does a pointer
> comparison (!) in many APIs for the context it's used with.

Ok. The generic thing sounds fine then :p

[boliu, 2015-01-29 23:53:12]

Fix for crbug.com/453199 landed. This going in!

[boliu, 2015-01-29 23:53:35]

The CQ bit was checked by boliu@chromium.org

[commit-bot: I haz the power, 2015-01-29 23:54:05]

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

[commit-bot: I haz the power, 2015-01-30 01:01:26]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-01-30 01:02:17]

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

[boliu, 2015-01-30 18:23:44]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/888903002/ by boliu@chromium.org.

The reason for reverting is: Suspect causing
AwContentsClientFullScreenTest#testPowerSaveBlockerIsTransferredToFullscreen to
time out.

[boliu, 2015-02-05 01:19:42]

On 2015/01/30 18:23:44, boliu wrote:
> A revert of this CL (patchset #6 id:100001) has been created in
> https://codereview.chromium.org/888903002/ by mailto:boliu@chromium.org.
> 
> The reason for reverting is: Suspect causing
> AwContentsClientFullScreenTest#testPowerSaveBlockerIsTransferredToFullscreen
to
> time out.

The failure is a driver bug and worked around. Details in crbug.com/453857

This patch going back in!

[boliu, 2015-02-05 01:19:58]

The CQ bit was checked by boliu@chromium.org

[commit-bot: I haz the power, 2015-02-05 01:21:16]

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

[commit-bot: I haz the power, 2015-02-05 01:25:58]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-02-05 01:28:02]

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