Created attachment 185162 [details] Patch

Attachment 185162 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/Target.pri', u'Source/WebCore/platform/graphics/texmap/coordinated/AreaAllocator.cpp', u'Source/WebCore/platform/graphics/texmap/coordinated/AreaAllocator.h', u'Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedBackingStore.cpp', u'Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedBackingStore.h', u'Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp', u'Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h', u'Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedImageBacking.cpp', u'Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedImageBacking.h', u'Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedLayerInfo.h', u'Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedSurface.h', u'Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedTile.cpp', u'Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedTile.h', u'Source/WebCore/platform/graphics/texmap/coordinated/LayerTreeRenderer.cpp', u'Source/WebCore/platform/graphics/texmap/coordinated/LayerTreeRenderer.h', u'Source/WebCore/platform/graphics/texmap/coordinated/SurfaceUpdateInfo.h', u'Source/WebCore/platform/graphics/texmap/coordinated/UpdateAtlas.cpp', u'Source/WebCore/platform/graphics/texmap/coordinated/UpdateAtlas.h', u'Source/WebCore/platform/graphics/texmap/coordinated/WebCustomFilterOperation.h', u'Source/WebCore/platform/graphics/texmap/coordinated/WebCustomFilterProgram.h', u'Source/WebKit2/CMakeLists.txt', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsArgumentCoders.cpp', u'Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsArgumentCoders.h', u'Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedLayerInfo.h', u'Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedSurface.h', u'Source/WebKit2/Shared/CoordinatedGraphics/SurfaceUpdateInfo.h', u'Source/WebKit2/Shared/CoordinatedGraphics/WebCoordinatedSurface.cpp', u'Source/WebKit2/Shared/CoordinatedGraphics/WebCoordinatedSurface.h', u'Source/WebKit2/Shared/CoordinatedGraphics/WebCustomFilterOperation.h', u'Source/WebKit2/Shared/CoordinatedGraphics/WebCustomFilterProgram.h', u'Source/WebKit2/Target.pri', u'Source/WebKit2/UIProcess/API/efl/EwkView.cpp', u'Source/WebKit2/UIProcess/API/efl/EwkView.h', u'Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp', u'Source/WebKit2/UIProcess/API/qt/raw/qrawwebview.cpp', u'Source/WebKit2/UIProcess/API/qt/raw/qrawwebview_p.h', u'Source/WebKit2/UIProcess/CoordinatedGraphics/CoordinatedBackingStore.cpp', u'Source/WebKit2/UIProcess/CoordinatedGraphics/CoordinatedBackingStore.h', u'Source/WebKit2/UIProcess/CoordinatedGraphics/CoordinatedLayerTreeHostProxy.cpp', u'Source/WebKit2/UIProcess/CoordinatedGraphics/CoordinatedLayerTreeHostProxy.h', u'Source/WebKit2/UIProcess/CoordinatedGraphics/CoordinatedLayerTreeHostProxy.messages.in', u'Source/WebKit2/UIProcess/CoordinatedGraphics/LayerTreeRenderer.cpp', u'Source/WebKit2/UIProcess/CoordinatedGraphics/LayerTreeRenderer.h', u'Source/WebKit2/UIProcess/efl/PageClientBase.cpp', u'Source/WebKit2/UIProcess/efl/PageViewportControllerClientEfl.cpp', u'Source/WebKit2/UIProcess/qt/QtWebPageSGNode.cpp', u'Source/WebKit2/UIProcess/qt/QtWebPageSGNode.h', u'Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/AreaAllocator.cpp', u'Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/AreaAllocator.h', u'Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.cpp', u'Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.h', u'Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedImageBacking.cpp', u'Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedImageBacking.h', u'Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedLayerTreeHost.cpp', u'Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedLayerTreeHost.h', u'Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedTile.cpp', u'Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedTile.h', u'Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/UpdateAtlas.cpp', u'Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/UpdateAtlas.h']" exit_code: 1 Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:411: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Total errors found: 1 in 39 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 185162 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=185162&action=review Apart from some rename suggestions, this seems to be going in the right direction for me. The next step would be to remove most of the stuff from CoordinatedGraphicsArgumentCoders. Anders, Simon, your feedback is of course welcome. > Source/WebCore/ChangeLog:24 > + 3. WebCustomFilterProgram and WebCustomFilterOperation is moved to WebCore in They should be renamed CoordinatedCustom*, the Web prefix is only for WebKit/WebKit2. > Source/WebCore/ChangeLog:239 > + (WebCore::LayerTreeRenderer::dispatchOnMainThread): I think LayerTreeRenderer should be renamed TextureMapperScene.

Comment on attachment 185162 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=185162&action=review > Source/WebCore/ChangeLog:17 > + 1. Removing CoordinatedLayerTreeHost dependency from LayerTreeRenderer. This > + patch introduces LayerTreeRendererClient, which is implemented in > + CoordinatedLayerTreeHost. LayerTreeRenderer uses this client, instead of using > + CoordinatedLayerTreeHost directly. Maybe we should do this first, with a separate patch.

(In reply to comment #3) > (From update of attachment 185162 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=185162&action=review > > Apart from some rename suggestions, this seems to be going in the right direction for me. > The next step would be to remove most of the stuff from CoordinatedGraphicsArgumentCoders. > > Anders, Simon, your feedback is of course welcome. > > > Source/WebCore/ChangeLog:24 > > + 3. WebCustomFilterProgram and WebCustomFilterOperation is moved to WebCore in > > They should be renamed CoordinatedCustom*, the Web prefix is only for WebKit/WebKit2. > > > Source/WebCore/ChangeLog:239 > > + (WebCore::LayerTreeRenderer::dispatchOnMainThread): > > I think LayerTreeRenderer should be renamed TextureMapperScene. (In reply to comment #4) > (From update of attachment 185162 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=185162&action=review > > > Source/WebCore/ChangeLog:17 > > + 1. Removing CoordinatedLayerTreeHost dependency from LayerTreeRenderer. This > > + patch introduces LayerTreeRendererClient, which is implemented in > > + CoordinatedLayerTreeHost. LayerTreeRenderer uses this client, instead of using > > + CoordinatedLayerTreeHost directly. > > Maybe we should do this first, with a separate patch. Thanks for the review! In the separate patch that introduces LayerTreeRendererClient, should I just rename LayerTreeRenderer to TextureMapperScene?

> In the separate patch that introduces LayerTreeRendererClient, should I just rename LayerTreeRenderer to TextureMapperScene? Let's do the rename as part of moving to WebCore. For now we should introduce the client without renames.

Comment on attachment 185162 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=185162&action=review > Source/WebCore/ChangeLog:22 > + 2. Create CoordinatedSurface using function pointer. Since CoordinatedSurface > + reside in WebCore now, it doesn't know WebCoordinatedSurface. So we pass > + CoordinatedSurface::Factory instead of using CoordinatedSurface::create > + method. I think this could be done in a separate patch as well. This patch ideally will only move the files and fix whatever needs to be fixed to get it compiling again.

(In reply to comment #6) > > In the separate patch that introduces LayerTreeRendererClient, should I just rename LayerTreeRenderer to TextureMapperScene? > Let's do the rename as part of moving to WebCore. For now we should introduce the client without renames. Ok. Created a patch in https://bugs.webkit.org/show_bug.cgi?id=108164

(In reply to comment #7) > (From update of attachment 185162 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=185162&action=review > > > Source/WebCore/ChangeLog:22 > > + 2. Create CoordinatedSurface using function pointer. Since CoordinatedSurface > > + reside in WebCore now, it doesn't know WebCoordinatedSurface. So we pass > > + CoordinatedSurface::Factory instead of using CoordinatedSurface::create > > + method. > > I think this could be done in a separate patch as well. This patch ideally will only move the files and fix whatever needs to be fixed to get it compiling again. Thanks for the review! I'll create another patch for using function pointer to create CoordinatedSurface.

Created attachment 185693 [details] Patch

Created attachment 185714 [details] rebased

Created attachment 185722 [details] Patch

Comment on attachment 185722 [details] Patch Looks good to me! Let's see if the WK2 owners are ok with this.

WK2 parts look good to me. You should remove all the function declarations from the file renames in the ChangeLog though.

Created attachment 185922 [details] Patch

Comment on attachment 185922 [details] Patch Let's go!

Comment on attachment 185922 [details] Patch Rejecting attachment 185922 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 185922, '--port=chromium-xvfb']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: return self.open(self.click(*args, **kwds)) File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_mechanize.py", line 203, in open return self._mech_open(url, data, timeout=timeout) File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_mechanize.py", line 255, in _mech_open raise response webkitpy.thirdparty.autoinstalled.mechanize._response.httperror_seek_wrapper: HTTP Error 500: Internal Server Error Full output: http://queues.webkit.org/results/16302414

Comment on attachment 185922 [details] Patch Trying again with cq. If this fails I'll lent manually.

Comment on attachment 185922 [details] Patch Clearing flags on attachment: 185922 Committed r141543: <http://trac.webkit.org/changeset/141543>

All reviewed patches have been landed. Closing bug.

(In reply to comment #15) > Created an attachment (id=185922) [details] > Patch Hello, Unfortunately, this patch breaks our buildbot: http://build.webkit.org/builders/Qt%20Linux%20SH4%20Release/builds/19109 I'll take a look this week if I find time to do so.

(In reply to comment #21) > (In reply to comment #15) > > Created an attachment (id=185922) [details] [details] > > Patch > Hello, > > Unfortunately, this patch breaks our buildbot: http://build.webkit.org/builders/Qt%20Linux%20SH4%20Release/builds/19109 > > I'll take a look this week if I find time to do so. Seems like some GL includes are not protected with the right #ifdefs.

(In reply to comment #22) > > Seems like some GL includes are not protected with the right #ifdefs. I think the issue is in Target.pri file. Shouldn't new files be into conditionnal use?(3D_GRAPHICS) block instead ?

I've created a new bug for this: https://bugs.webkit.org/show_bug.cgi?id=108862

(In reply to comment #24) > I've created a new bug for this: https://bugs.webkit.org/show_bug.cgi?id=108862 I'm sorry for breaking the build, and thanks for providing a fix!