This is a preparation patch for Threaded Coordinated Graphics. LayerTreeHost uses a native window handle to make glContext for accelerated compositing. Therefore it is natural for DrawingArea to take responsibility for the native window handle. And, in Coordinated Graphics case, WebPage creates LayerTreeHost before receiving a native window handle from UIProcess. Therefore we need a method to set the native window handle to already created LayerTreeHost. This patch uses DrawingAreaProxy::setNativeCompositingSurfaceHandle instead of WebCoreProxy::setAcceleratedCompositingWindowId to set window ID for accelerated compositing. Also, this patch renames the setAcceleratedCompositingWindowId with a more generic name, setNativeCompositingSurfaceHandle. No new tests. No change in functionality.
Created attachment 203997 [details] Patch
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment on attachment 203997 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=203997&action=review This change seems okay to me. I have suggestions for a few changes and after that it needs the approval of a WebKit owner. > Source/WebKit2/UIProcess/DrawingAreaProxyImpl.cpp:375 > +void DrawingAreaProxyImpl::setNativeCompositingSurfaceHandle(uint64_t handle) May I suggest a new name: setNativeSurfaceHandleForCompositing or setCompositingNativeSurfaceHandle. This eliminates any confusion between native "compositing surface handle" or a "surface handle" for native compositing. > Source/WebKit2/WebProcess/WebPage/LayerTreeHost.h:99 > +#if USE(TEXTURE_MAPPER_GL) && PLATFORM(GTK) > + virtual void setNativeCompositingSurfaceHandle(uint64_t) { } > +#endif > + This is unused for this patch, so maybe it can be removed.
Created attachment 205474 [details] Patch
(In reply to comment #3) > (From update of attachment 203997 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=203997&action=review > > This change seems okay to me. I have suggestions for a few changes and after that it needs the approval of a WebKit owner. > Sorry for late. :( > > Source/WebKit2/UIProcess/DrawingAreaProxyImpl.cpp:375 > > +void DrawingAreaProxyImpl::setNativeCompositingSurfaceHandle(uint64_t handle) > > May I suggest a new name: setNativeSurfaceHandleForCompositing or setCompositingNativeSurfaceHandle. This eliminates any confusion between native "compositing surface handle" or a "surface handle" for native compositing. > I prefer setNativeSurfaceHandleForCompositing. I've renamed with it. > > Source/WebKit2/WebProcess/WebPage/LayerTreeHost.h:99 > > +#if USE(TEXTURE_MAPPER_GL) && PLATFORM(GTK) > > + virtual void setNativeCompositingSurfaceHandle(uint64_t) { } > > +#endif > > + > > This is unused for this patch, so maybe it can be removed. Yes, exactly. removed.
Created attachment 206037 [details] Rebase after r152375
(In reply to comment #6) > Created an attachment (id=206037) [details] > Rebase after r152375 The previous patch looked good to me, so if this is just a rebase, you simply need an owner to approve it.
Created attachment 242761 [details] Patch
Comment on attachment 242761 [details] Patch Clearing flags on attachment: 242761 Committed r176954: <http://trac.webkit.org/changeset/176954>
All reviewed patches have been landed. Closing bug.