[webkit-reviews] review denied: [Bug 50698] [GTK] Split webkitprivate.{cpp, h} in more manageable chunks : [Attachment 75930] split webkitwebviewprivate.h out of webkitprivate.h

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied Gustavo Noronha (kov)
<gns at gnome.org>'s request for review:
Bug 50698: [GTK] Split webkitprivate.{cpp,h} in more manageable chunks
https://bugs.webkit.org/show_bug.cgi?id=50698

Attachment 75930: split webkitwebviewprivate.h out of webkitprivate.h
https://bugs.webkit.org/attachment.cgi?id=75930&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=75930&action=review

I love this change.

> WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:60
>  #include <wtf/text/CString.h>
> -
>  #include <glib.h>
>  #include <glib/gi18n-lib.h>
>  #include <gtk/gtk.h>

These should be in alphabetical order.

> WebKit/gtk/WebCoreSupport/ContextMenuClientGtk.cpp:22
>  

No newline here.

> WebKit/gtk/WebCoreSupport/ContextMenuClientGtk.cpp:-32
>  #include <wtf/text/CString.h>
> -
>  #include <glib/gi18n-lib.h>
>  #include <glib-object.h>
>  #include <gtk/gtk.h>
> -#include "webkitprivate.h"

These should be in alphabetical order.

> WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:74
>  #include <wtf/text/CString.h>
>  #include <wtf/text/StringConcatenate.h>
> -
>  #include <JavaScriptCore/APICast.h>
>  #include <gio/gio.h>
>  #include <glib.h>

Run these through sort too.

> WebKit/gtk/webkit/webkitwebview.cpp:57
> +#include "GOwnPtr.h"
> +#include "GOwnPtrGtk.h"

I think should be like <wtf/gobject/*> since they are WTF includes.

> WebKit/gtk/webkit/webkitwebview.cpp:93
>  #include <wtf/text/CString.h>
> -
>  #include <gdk/gdkkeysyms.h>
> +#include <glib/gi18n-lib.h>

Alpha ordering here.

> WebKit/gtk/webkit/webkitwebviewprivate.h:50
> +extern "C" {
> +    #define WEBKIT_WEB_VIEW_GET_PRIVATE(obj)   
(G_TYPE_INSTANCE_GET_PRIVATE((obj), WEBKIT_TYPE_WEB_VIEW,
WebKitWebViewPrivate))

No indentation for this block.

> WebKit/gtk/webkit/webkitwebviewprivate.h:124
> +    void
> +    webkit_web_view_notify_ready(WebKitWebView* web_view);
> +
> +    void
> +    webkit_web_view_request_download(WebKitWebView* web_view,
WebKitNetworkRequest* request, const WebCore::ResourceResponse& response =
WebCore::ResourceResponse(), WebCore::ResourceHandle* handle = 0);
> +
> +    void
> +    webkit_web_view_add_resource(WebKitWebView*, const char*,
WebKitWebResource*);
> +
> +    void
> +    webkit_web_view_add_main_resource(WebKitWebView*, const char*,
WebKitWebResource*);
> +
> +    void
> +    webkit_web_view_remove_resource(WebKitWebView*, const char*);

We should switch all these declarations to WebKit style since they aren't
public API. Remove the newline after the type and between them and remove all
undeeded variable names.