Bug 108259

Created attachment 185347 [details] Patch

Created attachment 185348 [details] Patch

Comment on attachment 185348 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=185348&action=review I think CoordinatedSurface::Factory can be static in CoordinatedSurface, the same way it is in GraphicsLayer. > Source/WebKit2/ChangeLog:16 > + that creates CoordinatedSurface. CoordinatedImageBacking and UpdateAtlas that creates CoordinatedSurfaces.

Created attachment 185456 [details] Patch

Comment on attachment 185456 [details] Patch LGTM, please ask a WK2 owner to sign off.

Comment on attachment 185456 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=185456&action=review I sign off on this change, Noam can review this for WebKit2. > Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedSurface.cpp:18 > +/* > + Copyright (C) 2013 Company 100, Inc. > + > + This library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Library General Public > + License as published by the Free Software Foundation; either > + version 2 of the License, or (at your option) any later version. > + > + This library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Library General Public License for more details. > + > + You should have received a copy of the GNU Library General Public License > + along with this library; see the file COPYING.LIB. If not, write to > + the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, > + Boston, MA 02110-1301, USA. > + */ Not the standard header. > Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedSurface.cpp:24 > +#include "config.h" > + > +#if USE(COORDINATED_GRAPHICS) > + > +#include "CoordinatedSurface.h" Should be: #include "config.h" #include "CoordinatedSurface.h" #if FOOBAR > Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedSurface.cpp:43 > +#endif // USE(COORDINATED_GRAPHICS) > + > +} // namespace WebKit the #define does not match the namespace scope. > Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedLayerTreeHost.h:119 > + // CoordinatedSurface Useless comment.

Created attachment 185672 [details] Patch

Comment on attachment 185672 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=185672&action=review > Source/WebKit2/ChangeLog:18 > + This patch introduces CoordinatedSurface::Factory, which is a function pointer > + that creates CoordinatedSurfaces. CoordinatedImageBacking and UpdateAtlas > + create a CoordinatedSurface using the function pointer passed by > + CoordinatedLayerTreeHost, instead of using CoordinatedSurface::create. This We don't pass the function pointer to those classes, the changelog need to be updated.

Created attachment 185687 [details] Remove WebCoordinatedSurface dependency from CoordinatedSurface

Created attachment 185689 [details] Patch

(In reply to comment #8) > (From update of attachment 185672 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=185672&action=review > > > Source/WebKit2/ChangeLog:18 > > + This patch introduces CoordinatedSurface::Factory, which is a function pointer > > + that creates CoordinatedSurfaces. CoordinatedImageBacking and UpdateAtlas > > + create a CoordinatedSurface using the function pointer passed by > > + CoordinatedLayerTreeHost, instead of using CoordinatedSurface::create. This > > We don't pass the function pointer to those classes, the changelog need to be updated. Fixed. Thanks for the review!

Comment on attachment 185689 [details] Patch Clearing flags on attachment: 185689 Committed r141384: <http://trac.webkit.org/changeset/141384>

All reviewed patches have been landed. Closing bug.