Bug 73440

Created attachment 117207 [details] Proposed patch

Attachment 117207 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/graphics/cairo/TextureMapperCairo.cpp:32: More than one command on the same line in if [whitespace/parens] [4] Total errors found: 1 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 117207 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=117207&action=review Nice. I have a couple quick comments... > Source/WebCore/platform/graphics/cairo/TextureMapperCairo.cpp:6 > + Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies) > + Copyright (C) 2011 Igalia S.L. > + > + This library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Library General Public Do you mind putting the asterisks on the side here when landing? It's typical to do that in WebKit. > Source/WebCore/platform/graphics/cairo/TextureMapperCairo.cpp:32 > + if (m_context) delete m_context; This should be two lines. > Source/WebCore/platform/graphics/cairo/TextureMapperCairo.h:4 > + Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies) > + Copyright (C) 2011 Igalia S.L. > + Same thing here. > Source/WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:922 > + if (webViewAllowsAcceleratedCompositing(m_webView)) > + return AllTriggers; > + You should just do something like: I'm surprised retun 0 doesn't return an error here... #if !USE(ACCELERATED_COMPOSITING) return 0; #else ... #endif > Source/WebKit/gtk/webkit/webkitwebview.cpp:4877 > +void webViewSetRootGraphicsLayer(WebKitWebView* webView, GraphicsLayer* graphicsLayer) In WebKit1 we use the web_view_set_root_graphics_layer style. > Source/WebKit/gtk/webkit/webkitwebview.cpp:4882 > +void webViewMarkForSync(WebKitWebView* webView, gboolean scheduleSync) I would say it's best to use bool here instead of gboolean. > Source/WebKit/gtk/webkit/webkitwebview.cpp:4895 > +gboolean webViewAllowsAcceleratedCompositing(WebKitWebView* webView) > +{ > +#if USE(ACCELERATED_COMPOSITING) > + return TRUE; > +#else > + return FALSE; > +#endif > +} Just put this inline instead of making it an extra function. > Source/WebKit/gtk/webkit/webkitwebviewprivate.h:33 > + You can remove this extra line.

Created attachment 117212 [details] Proposed patch Sorry I thought I had changed this.

Created attachment 117236 [details] Proposed patch Thanks for the review! New version.

Thanks! Landed http://trac.webkit.org/changeset/101548