New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 602484 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug

Blocked on:
issue 608026

Blocking:
issue 533617



Sign in to add a comment

Uses IOSurfaces to display 3d pepper content on OS X.

Project Member Reported by erikc...@chromium.org, Apr 11 2016

Issue description

Here's how Flash 3d content currently works:

1) PPB_Graphics3D_Proxy requests the creation of a command buffer using PpapiHostMsg_PPBGraphics3D_Create. 

2) ppb_opengles2_shared.cc exposes many gles2 calls to Flash [via the command buffer].

3) When PPB_Graphics3D_Impl receives a PpapiHostMsg_PPBGraphics3D_SwapBuffers message, it calls PepperPluginInstanceImpl::CommitBackingTexture, which grabs the back buffer of command buffer created in (1).

Using a single texture causes synchronization problems, unsurprisingly.
https://bugs.chromium.org/p/chromium/issues/detail?id=350204

danakj@ suggested doing something like canvas2d or webgl.
WebGL uses a queue of textures, which consumes more memory, but minimizes the number of textures allocations.
Canvas2D does a copy on demand, which is more memory friendly, but potentially forces a texture allocation every frame.

My initial inclination is to make a copy of every texture before passing it to cc, but only on Mac [which is how we implemented IOSurface backed Canvas2D]. This doesn't change the behavior on other platforms, and will not cause a performance regression on Mac [since using the non-CoreAnimation path forces a copy anyways].
 
I think this makes sense (having a pool of IOSurfaces that have the content copied from pepper).

Comment 2 by danakj@chromium.org, Apr 12 2016

Cc: piman@chromium.org

Comment 3 by piman@chromium.org, Apr 13 2016

It actually uses 2 textures, but there's some magic where they get renamed under the hood (GLES2DecoderImpl::UpdateParentTextureInfo). I don't love the later part, we would certainly benefit from being explicit there - as well as allow >2 textures.

I don't think you necessarily have to force a copy. You could allocate them as IOSurface - at least provided you can use those as render targets. The code to allocate storage is in BackTexture::Create in gpu/command_buffer/service/gles2_cmd_decoder.cc, and could be specialized on Mac (by using a GLImageIOSurface rather than a regular GL texture).

I'm not sure what it would give you otherwise, if you already have a copy?
Here's my plan:

-----------------
Phase 1: Fix the mechanism by which textures get passed from pepper to the compositor [all OSes].

Add two new async IPC messages:
GpuCommandBufferMsg_TakeFrontBuffer(gpu::Mailbox)
GpuCommandBufferMsg_ReturnFrontBuffer(gpu::Mailbox)

The former behaves likes GpuCommandBufferMsg_ProduceFrontBuffer, except it requires that the command buffer stop using that particular texture until it has been returned.

The command buffer implementation will keep a queue of textures that can be bound to the backbuffer instead of just 2. [Or, for simplicity, a queue of Backbuffers.]

------------------
Phase 2: Modify the command buffer to use IOSurfaces for the backbuffer texture [Mac only]. 

[Maybe this is 100% of the time, maybe this is only on demand.]

Pretty straight forward, hopefully. Update BackTexture::Create to allocate storage with IOSurfaces. Update all direct references to GL_TEXTURE_2D with a dynamic target. Hope that we don't run into any of the limitations of IOSurfaces.


Comment 5 by piman@chromium.org, Apr 20 2016

To clarify, for phase1, you'd do TakeFrontBuffer right after a SwapBuffers, and that means the next Swap can't reuse that texture for the back buffer, unless ReturnFrontBuffer was called, am I understanding correctly?

For phase2, I believe we currently always create a 1x1 backbuffer for $reasons, even for non-pepper contexts. Not sure if you want an IOSurface for those (overhead?). Otherwise it should be simple to add a creation-time attrib.
> To clarify, for phase1, you'd do TakeFrontBuffer right after a SwapBuffers, and that means the next Swap can't reuse that texture for the back buffer, unless ReturnFrontBuffer was called, am I understanding correctly?

Correct.

> For phase2, I believe we currently always create a 1x1 backbuffer for $reasons, even for non-pepper contexts. Not sure if you want an IOSurface for those (overhead?). Otherwise it should be simple to add a creation-time attrib.

Hm, okay. I guess we should probably avoid IOSurfaces unless we need them, since they appear much more finicky.
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 29 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ed5a6bb9f4321670482acc32a7cb4246a1e22f25

commit ed5a6bb9f4321670482acc32a7cb4246a1e22f25
Author: erikchen <erikchen@chromium.org>
Date: Fri Apr 29 01:54:39 2016

Pepper takes ownership of a mailbox before passing it to the texture layer.

I replaced the IPC message GpuCommandBufferMsg_ProduceFrontBuffer with
GpuCommandBufferMsg_TakeFrontBuffer and GpuCommandBufferMsg_ReturnFrontBuffer.
TakeFrontBuffer gives ownership of the front buffer to the client. When the
client returns it with ReturnFrontBuffer, the command buffer may choose to reuse
it.

This means that pepper no longer needs to use
SetTextureMailboxWithoutReleaseCallback.

BUG= 350204 ,  602484 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/1912833002
Cr-Commit-Position: refs/heads/master@{#390570}

[modify] https://crrev.com/ed5a6bb9f4321670482acc32a7cb4246a1e22f25/cc/layers/texture_layer.cc
[modify] https://crrev.com/ed5a6bb9f4321670482acc32a7cb4246a1e22f25/cc/layers/texture_layer.h
[modify] https://crrev.com/ed5a6bb9f4321670482acc32a7cb4246a1e22f25/cc/layers/texture_layer_unittest.cc
[modify] https://crrev.com/ed5a6bb9f4321670482acc32a7cb4246a1e22f25/components/mus/gles2/command_buffer_impl.cc
[modify] https://crrev.com/ed5a6bb9f4321670482acc32a7cb4246a1e22f25/components/mus/gles2/command_buffer_impl.h
[modify] https://crrev.com/ed5a6bb9f4321670482acc32a7cb4246a1e22f25/components/mus/public/interfaces/command_buffer.mojom
[modify] https://crrev.com/ed5a6bb9f4321670482acc32a7cb4246a1e22f25/content/renderer/pepper/pepper_plugin_instance_impl.cc
[modify] https://crrev.com/ed5a6bb9f4321670482acc32a7cb4246a1e22f25/content/renderer/pepper/pepper_plugin_instance_impl.h
[modify] https://crrev.com/ed5a6bb9f4321670482acc32a7cb4246a1e22f25/content/renderer/pepper/ppb_graphics_3d_impl.cc
[modify] https://crrev.com/ed5a6bb9f4321670482acc32a7cb4246a1e22f25/content/renderer/pepper/ppb_graphics_3d_impl.h
[modify] https://crrev.com/ed5a6bb9f4321670482acc32a7cb4246a1e22f25/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/ed5a6bb9f4321670482acc32a7cb4246a1e22f25/gpu/command_buffer/service/gles2_cmd_decoder.h
[modify] https://crrev.com/ed5a6bb9f4321670482acc32a7cb4246a1e22f25/gpu/command_buffer/service/gles2_cmd_decoder_mock.h
[modify] https://crrev.com/ed5a6bb9f4321670482acc32a7cb4246a1e22f25/gpu/command_buffer/tests/gl_texture_mailbox_unittest.cc
[modify] https://crrev.com/ed5a6bb9f4321670482acc32a7cb4246a1e22f25/gpu/ipc/client/command_buffer_proxy_impl.cc
[modify] https://crrev.com/ed5a6bb9f4321670482acc32a7cb4246a1e22f25/gpu/ipc/client/command_buffer_proxy_impl.h
[modify] https://crrev.com/ed5a6bb9f4321670482acc32a7cb4246a1e22f25/gpu/ipc/common/gpu_messages.h
[modify] https://crrev.com/ed5a6bb9f4321670482acc32a7cb4246a1e22f25/gpu/ipc/service/gpu_command_buffer_stub.cc
[modify] https://crrev.com/ed5a6bb9f4321670482acc32a7cb4246a1e22f25/gpu/ipc/service/gpu_command_buffer_stub.h
[modify] https://crrev.com/ed5a6bb9f4321670482acc32a7cb4246a1e22f25/ppapi/proxy/ppapi_messages.h
[modify] https://crrev.com/ed5a6bb9f4321670482acc32a7cb4246a1e22f25/ppapi/proxy/ppb_graphics_3d_proxy.cc
[modify] https://crrev.com/ed5a6bb9f4321670482acc32a7cb4246a1e22f25/ppapi/proxy/ppb_graphics_3d_proxy.h
[modify] https://crrev.com/ed5a6bb9f4321670482acc32a7cb4246a1e22f25/ppapi/thunk/ppb_graphics_3d_api.h

Project Member

Comment 8 by bugdroid1@chromium.org, May 2 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2e2db2fd684658f0c75b8ab730ac3d2cc36f9958

commit 2e2db2fd684658f0c75b8ab730ac3d2cc36f9958
Author: erikchen <erikchen@chromium.org>
Date: Mon May 02 17:45:48 2016

Revert of Pepper takes ownership of a mailbox before passing it to the texture layer. (patchset #19 id:350001 of https://codereview.chromium.org/1912833002/ )

Reason for revert:
Top crasher on renderer and gpu processes:

https://bugs.chromium.org/p/chromium/issues/detail?id=608163

Original issue's description:
> Pepper takes ownership of a mailbox before passing it to the texture layer.
>
> I replaced the IPC message GpuCommandBufferMsg_ProduceFrontBuffer with
> GpuCommandBufferMsg_TakeFrontBuffer and GpuCommandBufferMsg_ReturnFrontBuffer.
> TakeFrontBuffer gives ownership of the front buffer to the client. When the
> client returns it with ReturnFrontBuffer, the command buffer may choose to reuse
> it.
>
> This means that pepper no longer needs to use
> SetTextureMailboxWithoutReleaseCallback.
>
> BUG= 350204 ,  602484 
> CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
>
> Committed: https://crrev.com/ed5a6bb9f4321670482acc32a7cb4246a1e22f25
> Cr-Commit-Position: refs/heads/master@{#390570}

TBR=ccameron@chromium.org,piman@chromium.org,bbudge@chromium.org,tsepez@chromium.org,sky@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 350204 ,  602484 

Review-Url: https://codereview.chromium.org/1937173002
Cr-Commit-Position: refs/heads/master@{#390986}

[modify] https://crrev.com/2e2db2fd684658f0c75b8ab730ac3d2cc36f9958/cc/layers/texture_layer.cc
[modify] https://crrev.com/2e2db2fd684658f0c75b8ab730ac3d2cc36f9958/cc/layers/texture_layer.h
[modify] https://crrev.com/2e2db2fd684658f0c75b8ab730ac3d2cc36f9958/cc/layers/texture_layer_unittest.cc
[modify] https://crrev.com/2e2db2fd684658f0c75b8ab730ac3d2cc36f9958/components/mus/gles2/command_buffer_impl.cc
[modify] https://crrev.com/2e2db2fd684658f0c75b8ab730ac3d2cc36f9958/components/mus/gles2/command_buffer_impl.h
[modify] https://crrev.com/2e2db2fd684658f0c75b8ab730ac3d2cc36f9958/components/mus/public/interfaces/command_buffer.mojom
[modify] https://crrev.com/2e2db2fd684658f0c75b8ab730ac3d2cc36f9958/content/renderer/pepper/pepper_plugin_instance_impl.cc
[modify] https://crrev.com/2e2db2fd684658f0c75b8ab730ac3d2cc36f9958/content/renderer/pepper/pepper_plugin_instance_impl.h
[modify] https://crrev.com/2e2db2fd684658f0c75b8ab730ac3d2cc36f9958/content/renderer/pepper/ppb_graphics_3d_impl.cc
[modify] https://crrev.com/2e2db2fd684658f0c75b8ab730ac3d2cc36f9958/content/renderer/pepper/ppb_graphics_3d_impl.h
[modify] https://crrev.com/2e2db2fd684658f0c75b8ab730ac3d2cc36f9958/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/2e2db2fd684658f0c75b8ab730ac3d2cc36f9958/gpu/command_buffer/service/gles2_cmd_decoder.h
[modify] https://crrev.com/2e2db2fd684658f0c75b8ab730ac3d2cc36f9958/gpu/command_buffer/service/gles2_cmd_decoder_mock.h
[modify] https://crrev.com/2e2db2fd684658f0c75b8ab730ac3d2cc36f9958/gpu/command_buffer/tests/gl_texture_mailbox_unittest.cc
[modify] https://crrev.com/2e2db2fd684658f0c75b8ab730ac3d2cc36f9958/gpu/ipc/client/command_buffer_proxy_impl.cc
[modify] https://crrev.com/2e2db2fd684658f0c75b8ab730ac3d2cc36f9958/gpu/ipc/client/command_buffer_proxy_impl.h
[modify] https://crrev.com/2e2db2fd684658f0c75b8ab730ac3d2cc36f9958/gpu/ipc/common/gpu_messages.h
[modify] https://crrev.com/2e2db2fd684658f0c75b8ab730ac3d2cc36f9958/gpu/ipc/service/gpu_command_buffer_stub.cc
[modify] https://crrev.com/2e2db2fd684658f0c75b8ab730ac3d2cc36f9958/gpu/ipc/service/gpu_command_buffer_stub.h
[modify] https://crrev.com/2e2db2fd684658f0c75b8ab730ac3d2cc36f9958/ppapi/proxy/ppapi_messages.h
[modify] https://crrev.com/2e2db2fd684658f0c75b8ab730ac3d2cc36f9958/ppapi/proxy/ppb_graphics_3d_proxy.cc
[modify] https://crrev.com/2e2db2fd684658f0c75b8ab730ac3d2cc36f9958/ppapi/proxy/ppb_graphics_3d_proxy.h
[modify] https://crrev.com/2e2db2fd684658f0c75b8ab730ac3d2cc36f9958/ppapi/thunk/ppb_graphics_3d_api.h

Comment 9 by ihf@chromium.org, May 3 2016

Cc: ihf@chromium.org gkihumba@chromium.org gurcheta...@chromium.org avkodipelli@chromium.org rohi...@chromium.org pucchakayala@chromium.org h...@chromium.org erikc...@chromium.org
 Issue 608857  has been merged into this issue.
Labels: -Pri-3 OS-Chrome OS-Linux Pri-1
Not sure how this Mac specific issue is causing  Issue 608857  which is on Linux and CrOS.
Labels: ReleaseBlock-Dev M-52
Labels: -OS-Linux -Pri-1 -ReleaseBlock-Dev -OS-Chrome Pri-2
@#10 the infrastructure (phase 1 in comment 4) is cross-platform.

This doesn't need to block dev. The first iteration that had issue was reverted.
Project Member

Comment 13 by bugdroid1@chromium.org, May 4 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4734ed1b6740b772201b8accb3e2bd88cdea5c99

commit 4734ed1b6740b772201b8accb3e2bd88cdea5c99
Author: erikchen <erikchen@chromium.org>
Date: Wed May 04 23:23:45 2016

[Reland 1] Pepper takes ownership of a mailbox before passing it to the texture layer.

This CL makes three changes from the original.

1. Replace a call to std::remove_if() with vec.erase(std::remove_if(), ...).
This was a logic error in the original CL that caused a crash any time the size
of the buffer was changed. This CL also adds a test that catches this bug.

2. Add some simple reference counting to PepperPluginInstanceImpl to track the
fact that a cc::TextureMailbox may be passed to |texture_layer_| more than once.

3. The SyncToken signal is now processed in the context of its own message:
WaitSyncToken.

> I replaced the IPC message GpuCommandBufferMsg_ProduceFrontBuffer with
> GpuCommandBufferMsg_TakeFrontBuffer and GpuCommandBufferMsg_ReturnFrontBuffer.
> TakeFrontBuffer gives ownership of the front buffer to the client. When the
> client returns it with ReturnFrontBuffer, the command buffer may choose to reuse
> it.
>
> This means that pepper no longer needs to use
> SetTextureMailboxWithoutReleaseCallback.

BUG= 350204 ,  602484 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
TEST=Run Chromium with the Pepper Flash plugin. Visit a site that supports Flash
video, such as http://vudu.com. Start playing a video, and then fullscreen the
video. Observe that Chromium does not crash. Please extensively test Chromium on
Flash 3D games and Flash video and make sure nothing else is working
incorrectly.
TBR=ccameron@chromium.org, bbudge@chromium.org, sky@chromium.org

Review-Url: https://codereview.chromium.org/1943513002
Cr-Commit-Position: refs/heads/master@{#391686}

[modify] https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99/cc/layers/texture_layer.cc
[modify] https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99/cc/layers/texture_layer.h
[modify] https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99/cc/layers/texture_layer_unittest.cc
[modify] https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99/components/mus/gles2/command_buffer_impl.cc
[modify] https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99/components/mus/gles2/command_buffer_impl.h
[modify] https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99/components/mus/public/interfaces/command_buffer.mojom
[modify] https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99/content/renderer/pepper/pepper_plugin_instance_impl.cc
[modify] https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99/content/renderer/pepper/pepper_plugin_instance_impl.h
[modify] https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99/content/renderer/pepper/ppb_graphics_3d_impl.cc
[modify] https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99/content/renderer/pepper/ppb_graphics_3d_impl.h
[modify] https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99/gpu/command_buffer/service/gles2_cmd_decoder.h
[modify] https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99/gpu/command_buffer/service/gles2_cmd_decoder_mock.h
[modify] https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99/gpu/command_buffer/tests/gl_texture_mailbox_unittest.cc
[modify] https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99/gpu/ipc/client/command_buffer_proxy_impl.cc
[modify] https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99/gpu/ipc/client/command_buffer_proxy_impl.h
[modify] https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99/gpu/ipc/common/gpu_messages.h
[modify] https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99/gpu/ipc/service/gpu_channel.cc
[modify] https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99/gpu/ipc/service/gpu_command_buffer_stub.cc
[modify] https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99/gpu/ipc/service/gpu_command_buffer_stub.h
[modify] https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99/ppapi/proxy/ppapi_messages.h
[modify] https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99/ppapi/proxy/ppb_graphics_3d_proxy.cc
[modify] https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99/ppapi/proxy/ppb_graphics_3d_proxy.h
[modify] https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99/ppapi/thunk/ppb_graphics_3d_api.h

Project Member

Comment 15 by bugdroid1@chromium.org, May 5 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5fcc55335e3b5ba36a0de1a2e7a0976c859f6fff

commit 5fcc55335e3b5ba36a0de1a2e7a0976c859f6fff
Author: erikchen <erikchen@chromium.org>
Date: Thu May 05 22:49:01 2016

Pepper: Associate TextureMailboxes with the Graphics3D they were obtained from.

This allows the TextureMailbox to be returned to the appropriate Graphics3D,
which implicitly deals with trying to return a TextureMailbox to a null
Graphics3D.

BUG= 602484 

Review-Url: https://codereview.chromium.org/1951263003
Cr-Commit-Position: refs/heads/master@{#391934}

[modify] https://crrev.com/5fcc55335e3b5ba36a0de1a2e7a0976c859f6fff/content/renderer/pepper/pepper_plugin_instance_impl.cc
[modify] https://crrev.com/5fcc55335e3b5ba36a0de1a2e7a0976c859f6fff/content/renderer/pepper/pepper_plugin_instance_impl.h

Comment 16 by ihf@chromium.org, May 10 2016

This broke webgl tests on most CrOS boards, see  issue 610411 .
Project Member

Comment 17 by bugdroid1@chromium.org, May 10 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e8e99f4cc7787cd52aad8f514a03db5aeadf1feb

commit e8e99f4cc7787cd52aad8f514a03db5aeadf1feb
Author: ihf <ihf@chromium.org>
Date: Tue May 10 04:15:35 2016

Revert of Pepper: Add a nullptr check. (patchset #3 id:40001 of https://codereview.chromium.org/1951263003/ )

Reason for revert:
Breaks WebGL on CrOS,  crbug.com/610411 .

Original issue's description:
> Pepper: Associate TextureMailboxes with the Graphics3D they were obtained from.
>
> This allows the TextureMailbox to be returned to the appropriate Graphics3D,
> which implicitly deals with trying to return a TextureMailbox to a null
> Graphics3D.
>
> BUG= 602484 
>
> Committed: https://crrev.com/5fcc55335e3b5ba36a0de1a2e7a0976c859f6fff
> Cr-Commit-Position: refs/heads/master@{#391934}

TBR=piman@chromium.org,erikchen@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 602484 

Review-Url: https://codereview.chromium.org/1963633003
Cr-Commit-Position: refs/heads/master@{#392542}

[modify] https://crrev.com/e8e99f4cc7787cd52aad8f514a03db5aeadf1feb/content/renderer/pepper/pepper_plugin_instance_impl.cc
[modify] https://crrev.com/e8e99f4cc7787cd52aad8f514a03db5aeadf1feb/content/renderer/pepper/pepper_plugin_instance_impl.h

Project Member

Comment 18 by bugdroid1@chromium.org, May 10 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8c46143b0ca822b010d5f9c284eadc810b0678e8

commit 8c46143b0ca822b010d5f9c284eadc810b0678e8
Author: ihf <ihf@chromium.org>
Date: Tue May 10 05:14:05 2016

Revert of [Reland 1] Pepper takes ownership of a mailbox before passing it to the texture layer. (patchset #8 id:140001 of https://codereview.chromium.org/1943513002/ )

Reason for revert:
Breaks WebGL on CrOS,  crbug.com/610411 .

Original issue's description:
> [Reland 1] Pepper takes ownership of a mailbox before passing it to the texture layer.
>
> This CL makes three changes from the original.
>
> 1. Replace a call to std::remove_if() with vec.erase(std::remove_if(), ...).
> This was a logic error in the original CL that caused a crash any time the size
> of the buffer was changed. This CL also adds a test that catches this bug.
>
> 2. Add some simple reference counting to PepperPluginInstanceImpl to track the
> fact that a cc::TextureMailbox may be passed to |texture_layer_| more than once.
>
> 3. The SyncToken signal is now processed in the context of its own message:
> WaitSyncToken.
>
> > I replaced the IPC message GpuCommandBufferMsg_ProduceFrontBuffer with
> > GpuCommandBufferMsg_TakeFrontBuffer and GpuCommandBufferMsg_ReturnFrontBuffer.
> > TakeFrontBuffer gives ownership of the front buffer to the client. When the
> > client returns it with ReturnFrontBuffer, the command buffer may choose to reuse
> > it.
> >
> > This means that pepper no longer needs to use
> > SetTextureMailboxWithoutReleaseCallback.
>
> BUG= 350204 ,  602484 
> CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
> TEST=Run Chromium with the Pepper Flash plugin. Visit a site that supports Flash
> video, such as http://vudu.com. Start playing a video, and then fullscreen the
> video. Observe that Chromium does not crash. Please extensively test Chromium on
> Flash 3D games and Flash video and make sure nothing else is working
> incorrectly.
> TBR=ccameron@chromium.org, bbudge@chromium.org, sky@chromium.org
>
> Committed: https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99
> Cr-Commit-Position: refs/heads/master@{#391686}

TBR=piman@chromium.org,ccameron@chromium.org,bbudge@chromium.org,tsepez@chromium.org,sky@chromium.org,sunnyps@chromium.org,erikchen@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 350204 ,  602484 

Review-Url: https://codereview.chromium.org/1964793002
Cr-Commit-Position: refs/heads/master@{#392548}

[modify] https://crrev.com/8c46143b0ca822b010d5f9c284eadc810b0678e8/cc/layers/texture_layer.cc
[modify] https://crrev.com/8c46143b0ca822b010d5f9c284eadc810b0678e8/cc/layers/texture_layer.h
[modify] https://crrev.com/8c46143b0ca822b010d5f9c284eadc810b0678e8/cc/layers/texture_layer_unittest.cc
[modify] https://crrev.com/8c46143b0ca822b010d5f9c284eadc810b0678e8/components/mus/gles2/command_buffer_impl.cc
[modify] https://crrev.com/8c46143b0ca822b010d5f9c284eadc810b0678e8/components/mus/gles2/command_buffer_impl.h
[modify] https://crrev.com/8c46143b0ca822b010d5f9c284eadc810b0678e8/components/mus/public/interfaces/command_buffer.mojom
[modify] https://crrev.com/8c46143b0ca822b010d5f9c284eadc810b0678e8/content/renderer/pepper/pepper_plugin_instance_impl.cc
[modify] https://crrev.com/8c46143b0ca822b010d5f9c284eadc810b0678e8/content/renderer/pepper/pepper_plugin_instance_impl.h
[modify] https://crrev.com/8c46143b0ca822b010d5f9c284eadc810b0678e8/content/renderer/pepper/ppb_graphics_3d_impl.cc
[modify] https://crrev.com/8c46143b0ca822b010d5f9c284eadc810b0678e8/content/renderer/pepper/ppb_graphics_3d_impl.h
[modify] https://crrev.com/8c46143b0ca822b010d5f9c284eadc810b0678e8/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/8c46143b0ca822b010d5f9c284eadc810b0678e8/gpu/command_buffer/service/gles2_cmd_decoder.h
[modify] https://crrev.com/8c46143b0ca822b010d5f9c284eadc810b0678e8/gpu/command_buffer/service/gles2_cmd_decoder_mock.h
[modify] https://crrev.com/8c46143b0ca822b010d5f9c284eadc810b0678e8/gpu/command_buffer/tests/gl_texture_mailbox_unittest.cc
[modify] https://crrev.com/8c46143b0ca822b010d5f9c284eadc810b0678e8/gpu/ipc/client/command_buffer_proxy_impl.cc
[modify] https://crrev.com/8c46143b0ca822b010d5f9c284eadc810b0678e8/gpu/ipc/client/command_buffer_proxy_impl.h
[modify] https://crrev.com/8c46143b0ca822b010d5f9c284eadc810b0678e8/gpu/ipc/common/gpu_messages.h
[modify] https://crrev.com/8c46143b0ca822b010d5f9c284eadc810b0678e8/gpu/ipc/service/gpu_channel.cc
[modify] https://crrev.com/8c46143b0ca822b010d5f9c284eadc810b0678e8/gpu/ipc/service/gpu_command_buffer_stub.cc
[modify] https://crrev.com/8c46143b0ca822b010d5f9c284eadc810b0678e8/gpu/ipc/service/gpu_command_buffer_stub.h
[modify] https://crrev.com/8c46143b0ca822b010d5f9c284eadc810b0678e8/ppapi/proxy/ppapi_messages.h
[modify] https://crrev.com/8c46143b0ca822b010d5f9c284eadc810b0678e8/ppapi/proxy/ppb_graphics_3d_proxy.cc
[modify] https://crrev.com/8c46143b0ca822b010d5f9c284eadc810b0678e8/ppapi/proxy/ppb_graphics_3d_proxy.h
[modify] https://crrev.com/8c46143b0ca822b010d5f9c284eadc810b0678e8/ppapi/thunk/ppb_graphics_3d_api.h

Project Member

Comment 19 by bugdroid1@chromium.org, May 11 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/438b0447d618fdba716b5898422f7dea8fce472b

commit 438b0447d618fdba716b5898422f7dea8fce472b
Author: erikchen <erikchen@chromium.org>
Date: Wed May 11 18:33:30 2016

[Reland 2] Pepper takes ownership of a mailbox before passing it to the texture layer.

This CL is a reland of two CLs:

Pepper: Associate TextureMailboxes with the Graphics3D they were obtained from.
https://codereview.chromium.org/1951263003

[Reland 1] Pepper takes ownership of a mailbox before passing it to the texture
layer. https://codereview.chromium.org/1943513002

This CL fixes a logic error in GpuChannel where preemption was incorrectly
handled.

BUG= 350204 ,  602484 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
TBR=ccameron@chromium.org, bbudge@chromium.org, sky@chromium.org, tsepez@chromium.org

Review-Url: https://codereview.chromium.org/1965253002
Cr-Commit-Position: refs/heads/master@{#393003}

[modify] https://crrev.com/438b0447d618fdba716b5898422f7dea8fce472b/cc/layers/texture_layer.cc
[modify] https://crrev.com/438b0447d618fdba716b5898422f7dea8fce472b/cc/layers/texture_layer.h
[modify] https://crrev.com/438b0447d618fdba716b5898422f7dea8fce472b/cc/layers/texture_layer_unittest.cc
[modify] https://crrev.com/438b0447d618fdba716b5898422f7dea8fce472b/components/mus/gles2/command_buffer_impl.cc
[modify] https://crrev.com/438b0447d618fdba716b5898422f7dea8fce472b/components/mus/gles2/command_buffer_impl.h
[modify] https://crrev.com/438b0447d618fdba716b5898422f7dea8fce472b/components/mus/public/interfaces/command_buffer.mojom
[modify] https://crrev.com/438b0447d618fdba716b5898422f7dea8fce472b/content/renderer/pepper/pepper_plugin_instance_impl.cc
[modify] https://crrev.com/438b0447d618fdba716b5898422f7dea8fce472b/content/renderer/pepper/pepper_plugin_instance_impl.h
[modify] https://crrev.com/438b0447d618fdba716b5898422f7dea8fce472b/content/renderer/pepper/ppb_graphics_3d_impl.cc
[modify] https://crrev.com/438b0447d618fdba716b5898422f7dea8fce472b/content/renderer/pepper/ppb_graphics_3d_impl.h
[modify] https://crrev.com/438b0447d618fdba716b5898422f7dea8fce472b/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/438b0447d618fdba716b5898422f7dea8fce472b/gpu/command_buffer/service/gles2_cmd_decoder.h
[modify] https://crrev.com/438b0447d618fdba716b5898422f7dea8fce472b/gpu/command_buffer/service/gles2_cmd_decoder_mock.h
[modify] https://crrev.com/438b0447d618fdba716b5898422f7dea8fce472b/gpu/command_buffer/tests/gl_texture_mailbox_unittest.cc
[modify] https://crrev.com/438b0447d618fdba716b5898422f7dea8fce472b/gpu/ipc/client/command_buffer_proxy_impl.cc
[modify] https://crrev.com/438b0447d618fdba716b5898422f7dea8fce472b/gpu/ipc/client/command_buffer_proxy_impl.h
[modify] https://crrev.com/438b0447d618fdba716b5898422f7dea8fce472b/gpu/ipc/common/gpu_messages.h
[modify] https://crrev.com/438b0447d618fdba716b5898422f7dea8fce472b/gpu/ipc/service/gpu_channel.cc
[modify] https://crrev.com/438b0447d618fdba716b5898422f7dea8fce472b/gpu/ipc/service/gpu_command_buffer_stub.cc
[modify] https://crrev.com/438b0447d618fdba716b5898422f7dea8fce472b/gpu/ipc/service/gpu_command_buffer_stub.h
[modify] https://crrev.com/438b0447d618fdba716b5898422f7dea8fce472b/ppapi/proxy/ppapi_messages.h
[modify] https://crrev.com/438b0447d618fdba716b5898422f7dea8fce472b/ppapi/proxy/ppb_graphics_3d_proxy.cc
[modify] https://crrev.com/438b0447d618fdba716b5898422f7dea8fce472b/ppapi/proxy/ppb_graphics_3d_proxy.h
[modify] https://crrev.com/438b0447d618fdba716b5898422f7dea8fce472b/ppapi/thunk/ppb_graphics_3d_api.h

Project Member

Comment 20 by sheriffbot@chromium.org, Jun 1 2016

Labels: -M-52 M-53 MovedFrom-52
Moving this nonessential bug to the next milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Blockedon: 608026
piman: Request for comment.

Here's my proposal for getting IOSurface-backed backbuffers for 3d pepper content.

1. Add a new member should_use_native_gmb_for_backbuffer to ContextCreationAttribHelper.

2. Add a new member gpu_memory_buffer_factory_ to ContextGroup. It will be passed to the constructor by GpuCommandBufferStub.

3. Update BackTexture to support allocation and consumption of native GMBs. This requires (4), (5) and (6).

4. GLES2CmdDecoder will allocate a GMB with client_id=0, gmb_id=0. In effect, this is an anonymous GMB and cannot be passed to other processes.

5. GLES2CmdDecoder will create a GLImage and attach it to the anonymous GMB. There must be a valid GLImage id, and it must not conflict with other image IDs, which are typically allocated by the client [which is a monotonically increasing int, starting from 0]. I suggest the addition of a method CreateServiceClientId() to ImageFactory which returns a monotonically decreasing image id, starting from -1. 

6. IOSurface-backed GMBs require some special logic not shared with other GLImage-backed GMBs. In existing WebKit code [canvas, WebGL], this gets gated behind a #if defined(OS_MACOSX). For the code in gles2_cmd_decoder.cc, I suggest gating the logic behind should_use_native_gmb_for_backbuffer. If, in the future, another platform also wants to use that flag, they can add another flag to ContextCreationAttribHelper.

There are alternatives to (5) but they require changing the semantics of image ID allocation. 

Comment 23 by piman@chromium.org, Jun 20 2016

#1 to #4 sg.

For #5, do we actually need an image id? Today we do create GLImages in various contexts, without an image id - e.g. for video decoding.

For #6, what is the special logic specifically?
#5: Ah, I thought it was important to add the GLImage to the ImageFactory. If we already make GLImages without an ImageId, I'm happy to add another use case of that.

#6: The texture bind point needs to be TEXTURE_RECTANGLE_ARB. There will probably be color-mask and RGB-emulation logic. There might be some other details I haven't thought of yet.

Comment 25 by piman@chromium.org, Jun 20 2016

for #6, if we're doing special cases, let's make sure we defend well against it (and document!). E.g. if we hardcode TEXTURE_RECTANGLE_ARB, then we need to make sure should_use_native_gmb_for_backbuffer is only allowed on Mac, etc.

IWBN in the future to abstract the texture target, so that we don't need these ifdefs. We already abstract it for the compositor, so it should be possible to make sure the information gets to the right place.

I think there is already some color-mask/RGB emulation logic, for onscreen color buffers. It's likely possible to piggy-back on it.
Project Member

Comment 26 by bugdroid1@chromium.org, Jun 20 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/616de0b792791a400a2b2a2ca9507f280ad0e424

commit 616de0b792791a400a2b2a2ca9507f280ad0e424
Author: erikchen <erikchen@chromium.org>
Date: Mon Jun 20 21:59:15 2016

Remove some unnecessary code from BackTexture::Create.

BUG= 97775 ,  602484 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2082753002
Cr-Commit-Position: refs/heads/master@{#400801}

[modify] https://crrev.com/616de0b792791a400a2b2a2ca9507f280ad0e424/gpu/command_buffer/service/gles2_cmd_decoder.cc

Project Member

Comment 27 by bugdroid1@chromium.org, Jun 22 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3dec65ce8a52354cf041ac572ba5f50681f1b575

commit 3dec65ce8a52354cf041ac572ba5f50681f1b575
Author: erikchen <erikchen@chromium.org>
Date: Wed Jun 22 01:05:13 2016

Simplify logic involving BackTexture.

No intended behavior change. This CL removes the member
|offscreen_saved_color_texture_info_| by giving each BackTexture a TextureRef
member. This simplifies logic surrounding BackTexture, and is necessary to
support Image-backed BackTextures.

BUG= 602484 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2087663002
Cr-Commit-Position: refs/heads/master@{#401151}

[modify] https://crrev.com/3dec65ce8a52354cf041ac572ba5f50681f1b575/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/3dec65ce8a52354cf041ac572ba5f50681f1b575/gpu/command_buffer/service/texture_manager.cc
[modify] https://crrev.com/3dec65ce8a52354cf041ac572ba5f50681f1b575/gpu/command_buffer/service/texture_manager.h

Project Member

Comment 28 by bugdroid1@chromium.org, Jun 28 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7ea4608d71ab856d422381c5fd99920f67b80d23

commit 7ea4608d71ab856d422381c5fd99920f67b80d23
Author: erikchen <erikchen@chromium.org>
Date: Tue Jun 28 03:37:08 2016

Implement native GMB backbuffers in the GLES2 Command Decoder.

This will allow pepper apps to draw directly into a native GMB, which allows
them to use the new CoreAnimation compositor.

* Add a new method to ImageFactory to allow the creation of anonymous images.
* Add logic to BackTexture to allow storage allocation via Image.
* Use existing logic to implement RGB emulation.

BUG= 602484 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2061743004
Cr-Commit-Position: refs/heads/master@{#402387}

[modify] https://crrev.com/7ea4608d71ab856d422381c5fd99920f67b80d23/components/mus/gles2/command_buffer_driver.cc
[modify] https://crrev.com/7ea4608d71ab856d422381c5fd99920f67b80d23/gpu/BUILD.gn
[modify] https://crrev.com/7ea4608d71ab856d422381c5fd99920f67b80d23/gpu/command_buffer/common/gles2_cmd_utils.cc
[modify] https://crrev.com/7ea4608d71ab856d422381c5fd99920f67b80d23/gpu/command_buffer/common/gles2_cmd_utils.h
[modify] https://crrev.com/7ea4608d71ab856d422381c5fd99920f67b80d23/gpu/command_buffer/service/context_group.cc
[modify] https://crrev.com/7ea4608d71ab856d422381c5fd99920f67b80d23/gpu/command_buffer/service/context_group.h
[modify] https://crrev.com/7ea4608d71ab856d422381c5fd99920f67b80d23/gpu/command_buffer/service/context_group_unittest.cc
[modify] https://crrev.com/7ea4608d71ab856d422381c5fd99920f67b80d23/gpu/command_buffer/service/feature_info.h
[modify] https://crrev.com/7ea4608d71ab856d422381c5fd99920f67b80d23/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/7ea4608d71ab856d422381c5fd99920f67b80d23/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc
[modify] https://crrev.com/7ea4608d71ab856d422381c5fd99920f67b80d23/gpu/command_buffer/service/image_factory.cc
[modify] https://crrev.com/7ea4608d71ab856d422381c5fd99920f67b80d23/gpu/command_buffer/service/image_factory.h
[modify] https://crrev.com/7ea4608d71ab856d422381c5fd99920f67b80d23/gpu/command_buffer/service/in_process_command_buffer.cc
[modify] https://crrev.com/7ea4608d71ab856d422381c5fd99920f67b80d23/gpu/command_buffer/tests/gl_manager.cc
[modify] https://crrev.com/7ea4608d71ab856d422381c5fd99920f67b80d23/gpu/command_buffer/tests/gl_manager.h
[add] https://crrev.com/7ea4608d71ab856d422381c5fd99920f67b80d23/gpu/command_buffer/tests/gl_native_gmb_backbuffer_unittest.cc
[add] https://crrev.com/7ea4608d71ab856d422381c5fd99920f67b80d23/gpu/command_buffer/tests/texture_image_factory.cc
[add] https://crrev.com/7ea4608d71ab856d422381c5fd99920f67b80d23/gpu/command_buffer/tests/texture_image_factory.h
[modify] https://crrev.com/7ea4608d71ab856d422381c5fd99920f67b80d23/gpu/gles2_conform_support/egl/context.cc
[modify] https://crrev.com/7ea4608d71ab856d422381c5fd99920f67b80d23/gpu/gpu.gyp
[modify] https://crrev.com/7ea4608d71ab856d422381c5fd99920f67b80d23/gpu/ipc/service/gpu_command_buffer_stub.cc
[modify] https://crrev.com/7ea4608d71ab856d422381c5fd99920f67b80d23/gpu/ipc/service/gpu_memory_buffer_factory_io_surface.cc
[modify] https://crrev.com/7ea4608d71ab856d422381c5fd99920f67b80d23/gpu/ipc/service/gpu_memory_buffer_factory_io_surface.h

Project Member

Comment 29 by bugdroid1@chromium.org, Jun 28 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4f2b6a831fd34e728b87e3797686e70983a748a9

commit 4f2b6a831fd34e728b87e3797686e70983a748a9
Author: erikchen <erikchen@chromium.org>
Date: Tue Jun 28 21:25:14 2016

Change destruction order of GLES2CmdDecoder.

Previously, ContextState was destroyed before internal state. Some internal
state (such as BackTexture) require ContextState to still be around before they
are destroyed.

BUG= 602484 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2103913002
Cr-Commit-Position: refs/heads/master@{#402547}

[modify] https://crrev.com/4f2b6a831fd34e728b87e3797686e70983a748a9/gpu/command_buffer/service/gles2_cmd_decoder.cc

Project Member

Comment 30 by bugdroid1@chromium.org, Jul 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8ce92bf1f6eb02e80d516e02ef23842dafc9a0aa

commit 8ce92bf1f6eb02e80d516e02ef23842dafc9a0aa
Author: erikchen <erikchen@chromium.org>
Date: Thu Jul 07 00:36:48 2016

Fix a bug with emulated RGB offscreen native GpuMemoryBuffers.

Some command buffers backed by a surface need to have the back buffer cleared.
This logic doesn't apply correctly to emulated RGB offscreen backbuffers.
There's no need to do the clear to begin with for offscreen buffers since the
offscreen buffer is cleared by ResizeOffscreenFrameBuffer.

BUG= 602484 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2129523004
Cr-Commit-Position: refs/heads/master@{#404017}

[modify] https://crrev.com/8ce92bf1f6eb02e80d516e02ef23842dafc9a0aa/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/8ce92bf1f6eb02e80d516e02ef23842dafc9a0aa/gpu/command_buffer/tests/gl_native_gmb_backbuffer_unittest.cc

Project Member

Comment 31 by bugdroid1@chromium.org, Jul 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8ce92bf1f6eb02e80d516e02ef23842dafc9a0aa

commit 8ce92bf1f6eb02e80d516e02ef23842dafc9a0aa
Author: erikchen <erikchen@chromium.org>
Date: Thu Jul 07 00:36:48 2016

Fix a bug with emulated RGB offscreen native GpuMemoryBuffers.

Some command buffers backed by a surface need to have the back buffer cleared.
This logic doesn't apply correctly to emulated RGB offscreen backbuffers.
There's no need to do the clear to begin with for offscreen buffers since the
offscreen buffer is cleared by ResizeOffscreenFrameBuffer.

BUG= 602484 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2129523004
Cr-Commit-Position: refs/heads/master@{#404017}

[modify] https://crrev.com/8ce92bf1f6eb02e80d516e02ef23842dafc9a0aa/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/8ce92bf1f6eb02e80d516e02ef23842dafc9a0aa/gpu/command_buffer/tests/gl_native_gmb_backbuffer_unittest.cc

Project Member

Comment 32 by bugdroid1@chromium.org, Jul 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/477e47082e76afca7c2e656a0714ee24e67fb702

commit 477e47082e76afca7c2e656a0714ee24e67fb702
Author: erikchen <erikchen@chromium.org>
Date: Thu Jul 07 00:39:10 2016

Fix initialization ordering in GLES2CmdDecoder.

The ContextGroup must be initialized before feature flag checks will work
correctly.

BUG= 602484 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2105153002
Cr-Commit-Position: refs/heads/master@{#404019}

[modify] https://crrev.com/477e47082e76afca7c2e656a0714ee24e67fb702/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/477e47082e76afca7c2e656a0714ee24e67fb702/gpu/command_buffer/tests/gl_manager.cc
[modify] https://crrev.com/477e47082e76afca7c2e656a0714ee24e67fb702/gpu/command_buffer/tests/gl_manager.h
[modify] https://crrev.com/477e47082e76afca7c2e656a0714ee24e67fb702/gpu/command_buffer/tests/gl_native_gmb_backbuffer_unittest.cc

Project Member

Comment 33 by bugdroid1@chromium.org, Jul 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/39de5e1f08c3162866e42b8cf932fdef0237c92a

commit 39de5e1f08c3162866e42b8cf932fdef0237c92a
Author: phoglund <phoglund@chromium.org>
Date: Thu Jul 07 09:24:09 2016

Revert of Fix initialization ordering in GLES2CmdDecoder. (patchset #2 id:20001 of https://codereview.chromium.org/2105153002/ )

Reason for revert:
Appears to break GPU webkit tests on Mac and Linux. This revert is a bit speculative since there are like 4 more gpu-related patches in the blame list, but the error messages does mention gles2_cmd_decoder.cc, so picking this patch as the first victim. https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.9/builds/34148/steps/webkit_tests/logs/stdio
https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20ASAN/builds/24915

Original issue's description:
> Fix initialization ordering in GLES2CmdDecoder.
>
> The ContextGroup must be initialized before feature flag checks will work
> correctly.
>
> BUG= 602484 
> CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
>
> Committed: https://crrev.com/477e47082e76afca7c2e656a0714ee24e67fb702
> Cr-Commit-Position: refs/heads/master@{#404019}

TBR=piman@chromium.org,erikchen@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 602484 

Review-Url: https://codereview.chromium.org/2126183002
Cr-Commit-Position: refs/heads/master@{#404112}

[modify] https://crrev.com/39de5e1f08c3162866e42b8cf932fdef0237c92a/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/39de5e1f08c3162866e42b8cf932fdef0237c92a/gpu/command_buffer/tests/gl_manager.cc
[modify] https://crrev.com/39de5e1f08c3162866e42b8cf932fdef0237c92a/gpu/command_buffer/tests/gl_manager.h
[modify] https://crrev.com/39de5e1f08c3162866e42b8cf932fdef0237c92a/gpu/command_buffer/tests/gl_native_gmb_backbuffer_unittest.cc

Project Member

Comment 34 by bugdroid1@chromium.org, Jul 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2e0756e995089857af2491de1a21188d64248fe3

commit 2e0756e995089857af2491de1a21188d64248fe3
Author: phoglund <phoglund@chromium.org>
Date: Thu Jul 07 13:44:37 2016

Reland of Fix initialization ordering in GLES2CmdDecoder. (patchset #1 id:1 of https://codereview.chromium.org/2126183002/ )

Reason for revert:
This patch was probably innocent, https://codereview.chromium.org/2112783002/ was probably guilty.

Original issue's description:
> Revert of Fix initialization ordering in GLES2CmdDecoder. (patchset #2 id:20001 of https://codereview.chromium.org/2105153002/ )
>
> Reason for revert:
> Appears to break GPU webkit tests on Mac and Linux. This revert is a bit speculative since there are like 4 more gpu-related patches in the blame list, but the error messages does mention gles2_cmd_decoder.cc, so picking this patch as the first victim. https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.9/builds/34148/steps/webkit_tests/logs/stdio
> https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20ASAN/builds/24915
>
> Original issue's description:
> > Fix initialization ordering in GLES2CmdDecoder.
> >
> > The ContextGroup must be initialized before feature flag checks will work
> > correctly.
> >
> > BUG= 602484 
> > CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
> >
> > Committed: https://crrev.com/477e47082e76afca7c2e656a0714ee24e67fb702
> > Cr-Commit-Position: refs/heads/master@{#404019}
>
> TBR=piman@chromium.org,erikchen@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG= 602484 
>
> Committed: https://crrev.com/39de5e1f08c3162866e42b8cf932fdef0237c92a
> Cr-Commit-Position: refs/heads/master@{#404112}

TBR=piman@chromium.org,erikchen@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 602484 

Review-Url: https://codereview.chromium.org/2121293004
Cr-Commit-Position: refs/heads/master@{#404147}

[modify] https://crrev.com/2e0756e995089857af2491de1a21188d64248fe3/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/2e0756e995089857af2491de1a21188d64248fe3/gpu/command_buffer/tests/gl_manager.cc
[modify] https://crrev.com/2e0756e995089857af2491de1a21188d64248fe3/gpu/command_buffer/tests/gl_manager.h
[modify] https://crrev.com/2e0756e995089857af2491de1a21188d64248fe3/gpu/command_buffer/tests/gl_native_gmb_backbuffer_unittest.cc

Project Member

Comment 35 by bugdroid1@chromium.org, Jul 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/45d8c8bc3da648c423f4bc009f6b6cdcaebd18f7

commit 45d8c8bc3da648c423f4bc009f6b6cdcaebd18f7
Author: erikchen <erikchen@chromium.org>
Date: Thu Jul 07 18:19:54 2016

Rename a gpu workaround.

Rename disable_webgl_multisampling_color_mask_usage to
disable_multisampling_color_mask_usage, as the command buffer also needs it for
native GpuMemoryBuffer back buffers. No intended behavior change.

BUG= 602484 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2124183002
Cr-Commit-Position: refs/heads/master@{#404181}

[modify] https://crrev.com/45d8c8bc3da648c423f4bc009f6b6cdcaebd18f7/gpu/command_buffer/common/capabilities.cc
[modify] https://crrev.com/45d8c8bc3da648c423f4bc009f6b6cdcaebd18f7/gpu/command_buffer/common/capabilities.h
[modify] https://crrev.com/45d8c8bc3da648c423f4bc009f6b6cdcaebd18f7/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/45d8c8bc3da648c423f4bc009f6b6cdcaebd18f7/gpu/config/gpu_driver_bug_list_json.cc
[modify] https://crrev.com/45d8c8bc3da648c423f4bc009f6b6cdcaebd18f7/gpu/config/gpu_driver_bug_workaround_type.h
[modify] https://crrev.com/45d8c8bc3da648c423f4bc009f6b6cdcaebd18f7/gpu/ipc/common/gpu_command_buffer_traits_multi.h
[modify] https://crrev.com/45d8c8bc3da648c423f4bc009f6b6cdcaebd18f7/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp

Project Member

Comment 36 by bugdroid1@chromium.org, Jul 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d4e64fc3153e756b8a1398b8d9dba4f220df6e49

commit d4e64fc3153e756b8a1398b8d9dba4f220df6e49
Author: erikchen <erikchen@chromium.org>
Date: Thu Jul 07 20:04:46 2016

Add workaround for multisampling color masks for the command buffer.

On old AMD GPUs on macOS, glColorMask doesn't work correctly for
multisampled renderbuffers and the alpha channel can be overwritten. This
workaround clears the alpha channel before resolving.

BUG= 602484 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2128753002
Cr-Commit-Position: refs/heads/master@{#404223}

[modify] https://crrev.com/d4e64fc3153e756b8a1398b8d9dba4f220df6e49/gpu/command_buffer/service/gles2_cmd_decoder.cc

Project Member

Comment 37 by bugdroid1@chromium.org, Jul 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b13637b2ffc6aca611a407e943ffe587ab7e4aa2

commit b13637b2ffc6aca611a407e943ffe587ab7e4aa2
Author: erikchen <erikchen@chromium.org>
Date: Fri Jul 08 09:38:56 2016

mac: Finish plumbing for Pepper3D Image Chromium.

This allows Flash content to use the new Core Animation compositing path. This
CL includes the last plumbing required to turn on the feature. It will be turned
on in a separate CL to allow for a clean revert if needed.

BUG= 602484 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2086363004
Cr-Commit-Position: refs/heads/master@{#404330}

[modify] https://crrev.com/b13637b2ffc6aca611a407e943ffe587ab7e4aa2/chrome/browser/chromeos/login/chrome_restart_request.cc
[modify] https://crrev.com/b13637b2ffc6aca611a407e943ffe587ab7e4aa2/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/b13637b2ffc6aca611a407e943ffe587ab7e4aa2/content/public/common/content_switches.cc
[modify] https://crrev.com/b13637b2ffc6aca611a407e943ffe587ab7e4aa2/content/public/common/content_switches.h
[modify] https://crrev.com/b13637b2ffc6aca611a407e943ffe587ab7e4aa2/content/renderer/pepper/ppb_graphics_3d_impl.cc
[modify] https://crrev.com/b13637b2ffc6aca611a407e943ffe587ab7e4aa2/content/renderer/pepper/ppb_graphics_3d_impl.h
[modify] https://crrev.com/b13637b2ffc6aca611a407e943ffe587ab7e4aa2/gpu/ipc/common/gpu_param_traits_macros.h
[modify] https://crrev.com/b13637b2ffc6aca611a407e943ffe587ab7e4aa2/ppapi/proxy/ppapi_messages.h
[modify] https://crrev.com/b13637b2ffc6aca611a407e943ffe587ab7e4aa2/ppapi/proxy/ppb_graphics_3d_proxy.cc
[modify] https://crrev.com/b13637b2ffc6aca611a407e943ffe587ab7e4aa2/ppapi/proxy/ppb_graphics_3d_proxy.h
[modify] https://crrev.com/b13637b2ffc6aca611a407e943ffe587ab7e4aa2/ppapi/shared_impl/ppb_graphics_3d_shared.cc
[modify] https://crrev.com/b13637b2ffc6aca611a407e943ffe587ab7e4aa2/ppapi/shared_impl/ppb_graphics_3d_shared.h
[modify] https://crrev.com/b13637b2ffc6aca611a407e943ffe587ab7e4aa2/ppapi/thunk/ppb_graphics_3d_api.h

Project Member

Comment 38 by bugdroid1@chromium.org, Jul 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c09eb27e67e0fa91bf4be375565a6c6fb5ac9030

commit c09eb27e67e0fa91bf4be375565a6c6fb5ac9030
Author: erikchen <erikchen@chromium.org>
Date: Fri Jul 08 21:33:32 2016

mac: Enable Pepper3D Image Chromium.

BUG= 602484 

Review-Url: https://codereview.chromium.org/2102273002
Cr-Commit-Position: refs/heads/master@{#404496}

[modify] https://crrev.com/c09eb27e67e0fa91bf4be375565a6c6fb5ac9030/content/renderer/pepper/ppb_graphics_3d_impl.cc

Project Member

Comment 39 by sheriffbot@chromium.org, Jul 14 2016

Labels: -M-53 MovedFrom-53
This issue has been moved once and is lower than Pri-1. Removing the milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Status: Fixed (was: Assigned)

Sign in to add a comment