[webkit-reviews] review denied: [Bug 68062] [GTK] WebProcess shouldn't use the GTK+ API : [Attachment 107302] Patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 68062: [GTK] WebProcess shouldn't use the GTK+ API
https://bugs.webkit.org/show_bug.cgi?id=68062

Attachment 107302: Patch
https://bugs.webkit.org/attachment.cgi?id=107302&action=review

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


Looks good, but we need to guard the duplicated enum.

> Source/WebCore/platform/gtk/ErrorsGtk.h:57
> +enum NetworkError {
> +    NetworkErrorFailed			     = 399,
> +    NetworkErrorTransport			     = 300,
> +    NetworkErrorUnknownProtocol		     = 301,
> +    NetworkErrorCancelled			     = 302,
> +    NetworkErrorFileDoesNotExist		     = 303
> +};
> +
> +// Sync'd with Mac's WebKit Errors.
> +enum PolicyError {
> +    PolicyErrorFailed			     = 199,
> +    PolicyErrorCannotShowMimeType		     = 100,
> +    PolicyErrorCannotShowURL 		     = 101,
> +    PolicyErrorFrameLoadInterruptedByPolicyChange = 102,
> +    PolicyErrorCannotUseRestrictedPort	     = 103
> +};
> +
> +enum PluginError {
> +    PluginErrorFailed			     = 299,
> +    PluginErrorCannotFindPlugin		     = 200,
> +    PluginErrorCannotLoadPlugin		     = 201,
> +    PluginErrorJavaUnavailable		     = 202,
> +    PluginErrorConnectionCancelled		     = 203,
> +    PluginErrorWillHandleLoad		     = 204
> +};

Since you are duplicating this enum from WebKit you shoud use the approach in
Source/WebKit/WebCoreSupport/AssertMatchingEnums.cpp to sure they stay in sync.


Because some of these lines are so long, it's my opinion that it helps
readability to not line them up. Please consider unaligning them.