Switch all contexts in a share group to be on the same screen as the window they present to.

Switch all contexts in a share group to be on the same screen as the window they present to.

When OS X switches GPUs, some contexts that were created beforehand and aren't associated with an NSView can be left behind on the old GPU, which will cause problems when other contexts attempt to share textures with them.

To fix this, track what renderer is being used to present to the screen, transmit that to the IOSurfaceImageTransportSurface in the GPU process, and change every context in the same share group to use that renderer.

BUG=174149
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=186388

[greggman, 2013-03-01 03:06:41]

I'm gonna defer to Ken as he has a lot more experience with mac stuff

My only comment, does the Set/Get/renderer_ field stuff need to be mac only? If
it's not used on other platforms sees okay to me but looks good to me either
way.

[Ken Russell (switch to Gerrit), 2013-03-01 22:00:58]

LGTM

The logic all looks good. Would like to see a small renaming of the "renderer"
field throughout. I'm OK with keeping these methods Mac-specific on the share
group since they don't make sense for other platforms.

https://codereview.chromium.org/12383042/diff/1/content/browser/renderer_host...
File content/browser/renderer_host/compositing_iosurface_mac.h (right):

https://codereview.chromium.org/12383042/diff/1/content/browser/renderer_host...
content/browser/renderer_host/compositing_iosurface_mac.h:53: // Get the CGL
renderer currently associated with this context.
Please adjust renderer -> renderer ID for clarity.

https://codereview.chromium.org/12383042/diff/1/content/common/gpu/gpu_messag...
File content/common/gpu/gpu_messages.h (right):

https://codereview.chromium.org/12383042/diff/1/content/common/gpu/gpu_messag...
content/common/gpu/gpu_messages.h:84: IPC_STRUCT_MEMBER(int32, renderer)
Please rename renderer -> renderer_id for clarity.

https://codereview.chromium.org/12383042/diff/1/content/common/gpu/image_tran...
File content/common/gpu/image_transport_surface_mac.cc (right):

https://codereview.chromium.org/12383042/diff/1/content/common/gpu/image_tran...
content/common/gpu/image_transport_surface_mac.cc:276:
context_->share_group()->SetRenderer(params.renderer);
Please rename SetRenderer -> SetRendererID for clarity.

https://codereview.chromium.org/12383042/diff/1/ui/gl/gl_context_cgl.cc
File ui/gl/gl_context_cgl.cc (right):

https://codereview.chromium.org/12383042/diff/1/ui/gl/gl_context_cgl.cc#newco...
ui/gl/gl_context_cgl.cc:23: static CGLPixelFormatObj format;
Does this need to be initialized to NULL?

https://codereview.chromium.org/12383042/diff/1/ui/gl/gl_context_cgl.h
File ui/gl/gl_context_cgl.h (right):

https://codereview.chromium.org/12383042/diff/1/ui/gl/gl_context_cgl.h#newcode45
ui/gl/gl_context_cgl.h:45: int renderer_;
renderer -> renderer_id_

https://codereview.chromium.org/12383042/diff/1/ui/gl/gl_share_group.h
File ui/gl/gl_share_group.h (right):

https://codereview.chromium.org/12383042/diff/1/ui/gl/gl_share_group.h#newcode45
ui/gl/gl_share_group.h:45: int GetRenderer();
Let's name these SetRendererID / GetRendererID.

[jbauman, 2013-03-02 03:52:22]

https://codereview.chromium.org/12383042/diff/1/content/browser/renderer_host...
File content/browser/renderer_host/compositing_iosurface_mac.h (right):

https://codereview.chromium.org/12383042/diff/1/content/browser/renderer_host...
content/browser/renderer_host/compositing_iosurface_mac.h:53: // Get the CGL
renderer currently associated with this context.
On 2013/03/01 22:00:58, kbr wrote:
> Please adjust renderer -> renderer ID for clarity.

Ok, done everywhere.

https://codereview.chromium.org/12383042/diff/1/ui/gl/gl_context_cgl.cc
File ui/gl/gl_context_cgl.cc (right):

https://codereview.chromium.org/12383042/diff/1/ui/gl/gl_context_cgl.cc#newco...
ui/gl/gl_context_cgl.cc:23: static CGLPixelFormatObj format;
On 2013/03/01 22:00:58, kbr wrote:
> Does this need to be initialized to NULL?

No, static variables are always initialized to 0 (essentially).

[jbauman, 2013-03-02 03:54:27]

Adding thakis@ for content/browser/renderer_host/*mac* OWNERS review, and
jschuh@ to rubber-stamp the trivial IPC change.

[jschuh, 2013-03-02 18:46:31]

Doesn't seem to have a security impact, so lgtm for ipc.

[Nico, 2013-03-04 08:39:33]

lgtm.

kbr should be an owner for content/browser/renderer_host/*mac* too.

[commit-bot: I haz the power, 2013-03-05 01:41:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jbauman@chromium.org/12383042/3002

[commit-bot: I haz the power, 2013-03-05 06:45:25]

Retried try job too often on win_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-03-06 04:15:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jbauman@chromium.org/12383042/20001

[commit-bot: I haz the power, 2013-03-06 09:31:38]

Change committed as 186388