Bug 68062 - [GTK] WebProcess shouldn't use the GTK+ API
Summary: [GTK] WebProcess shouldn't use the GTK+ API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2011-09-14 00:42 PDT by Carlos Garcia Campos
Modified: 2011-09-20 01:58 PDT (History)
1 user (show)

See Also:


Attachments
Patch (18.41 KB, patch)
2011-09-14 00:49 PDT, Carlos Garcia Campos
mrobinson: review-
Details | Formatted Diff | Diff
Updated patch (20.81 KB, patch)
2011-09-19 04:56 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Fix coding style issues (20.71 KB, patch)
2011-09-19 05:14 PDT, Carlos Garcia Campos
mrobinson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2011-09-14 00:42:03 PDT
In WebProcess/WebCoreSupport/gtk/WebErrorsGtk.cpp the GTK+ API is used to create errors.
Comment 1 Carlos Garcia Campos 2011-09-14 00:49:12 PDT
Created attachment 107302 [details]
Patch

Move common code to webcore so that it can be shared between wk1 and wk2.
Comment 2 Martin Robinson 2011-09-14 08:29:13 PDT
Comment on attachment 107302 [details]
Patch

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.
Comment 3 Carlos Garcia Campos 2011-09-19 04:56:23 PDT
Created attachment 107833 [details]
Updated patch
Comment 4 WebKit Review Bot 2011-09-19 04:58:29 PDT
Attachment 107833 [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/WebKit/gtk/WebCoreSupport/AssertMatchingEnums.cpp:33:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Carlos Garcia Campos 2011-09-19 05:14:56 PDT
Created attachment 107835 [details]
Fix coding style issues
Comment 6 Martin Robinson 2011-09-19 09:54:14 PDT
Comment on attachment 107835 [details]
Fix coding style issues

Nice cleanup!
Comment 7 Carlos Garcia Campos 2011-09-20 01:58:04 PDT
Committed r95521: <http://trac.webkit.org/changeset/95521>