Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(38)

Issue 8342024: Fixed bugs with Pepper 3D under dynamic GPU switching. (Closed)

Created:
9 years, 2 months ago by Ken Russell (switch to Gerrit)
Modified:
9 years, 2 months ago
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam, apatrick_chromium, dpranke-watch+content_chromium.org, nfullagar, Brad Chen (chromium)
Visibility:
Public.

Description

Fixed bugs with Pepper 3D under dynamic GPU switching. Made Pepper 3D context creation code fully aware of GPU switching and changed the order of context creation and and fetching of the parent context. Creating the context potentially causes all existing contexts for that renderer, including the parent, to be lost. Detect shutting down of the GPU channel more quickly on the renderer side. Only reject context creation in GLContextCGL upon GpuPreference mismatch if the system supports dual GPUs. Will update tools/histograms/histograms.xml as soon as this is committed; must occur in a separate CL. BUG=100507 TEST=tested navigating to and from, and reloading, NaCl Pepper 3D samples on Mac 10.6.8 and 10.7.1; WebGL tests Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=106394

Patch Set 1 #

Total comments: 4

Patch Set 2 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+150 lines, -47 lines) Patch
M content/common/gpu/gpu_process_launch_causes.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M content/content_renderer.gypi View 1 1 chunk +2 lines, -0 lines 0 comments Download
M content/renderer/gpu/renderer_gl_context.cc View 1 1 chunk +4 lines, -0 lines 0 comments Download
M content/renderer/gpu/webgraphicscontext3d_command_buffer_impl.cc View 1 1 chunk +1 line, -0 lines 0 comments Download
A content/renderer/pepper_parent_context_provider.h View 1 1 chunk +27 lines, -0 lines 0 comments Download
A content/renderer/pepper_parent_context_provider.cc View 1 1 chunk +11 lines, -0 lines 0 comments Download
M content/renderer/pepper_platform_context_3d_impl.h View 1 2 chunks +5 lines, -1 line 0 comments Download
M content/renderer/pepper_platform_context_3d_impl.cc View 1 5 chunks +53 lines, -21 lines 0 comments Download
M content/renderer/pepper_plugin_delegate_impl.h View 1 3 chunks +6 lines, -1 line 0 comments Download
M content/renderer/pepper_plugin_delegate_impl.cc View 1 2 chunks +17 lines, -13 lines 0 comments Download
M content/renderer/render_widget_fullscreen_pepper.h View 1 3 chunks +6 lines, -1 line 0 comments Download
M content/renderer/render_widget_fullscreen_pepper.cc View 1 2 chunks +11 lines, -6 lines 0 comments Download
M ui/gfx/gl/gl_context_cgl.cc View 1 1 chunk +6 lines, -4 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Ken Russell (switch to Gerrit)
Please review. Thanks.
9 years, 2 months ago (2011-10-19 02:49:39 UTC) #1
piman
lgtm
9 years, 2 months ago (2011-10-19 06:18:50 UTC) #2
apatrick_chromium
LGTM http://codereview.chromium.org/8342024/diff/1/content/renderer/pepper_parent_context_provider.h File content/renderer/pepper_parent_context_provider.h (right): http://codereview.chromium.org/8342024/diff/1/content/renderer/pepper_parent_context_provider.h#newcode17 content/renderer/pepper_parent_context_provider.h:17: virtual RendererGLContext* GetParentContextForPlatformContext3D() = 0; You could add ...
9 years, 2 months ago (2011-10-19 18:24:26 UTC) #3
Ken Russell (switch to Gerrit)
9 years, 2 months ago (2011-10-19 22:04:45 UTC) #4
http://codereview.chromium.org/8342024/diff/1/content/renderer/pepper_parent_...
File content/renderer/pepper_parent_context_provider.h (right):

http://codereview.chromium.org/8342024/diff/1/content/renderer/pepper_parent_...
content/renderer/pepper_parent_context_provider.h:17: virtual RendererGLContext*
GetParentContextForPlatformContext3D() = 0;
On 2011/10/19 18:24:27, apatrick_chromium wrote:
> You could add DISALLOW_COPY_AND_ASSIGN here and add a no-op default
constructor.

Right, I should have done that according to the style guide. Done.

http://codereview.chromium.org/8342024/diff/1/content/renderer/pepper_platfor...
File content/renderer/pepper_platform_context_3d_impl.cc (right):

http://codereview.chromium.org/8342024/diff/1/content/renderer/pepper_platfor...
content/renderer/pepper_platform_context_3d_impl.cc:133: 
On 2011/10/19 18:24:27, apatrick_chromium wrote:
> You could null parent_context_provider_ here since it will not be used again
and
> could potentially become dangling.

Done.

Powered by Google App Engine
This is Rietveld 408576698