Bug 61972 - [GTK] Clean up unecessary boilerplate from WebKitWebSettings and make private members meet WebKit style guidelines
Summary: [GTK] Clean up unecessary boilerplate from WebKitWebSettings and make private...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P3 Normal
Assignee: Martin Robinson
URL:
Keywords: Gtk
Depends on:
Blocks: 62424
  Show dependency treegraph
 
Reported: 2011-06-02 15:07 PDT by Martin Robinson
Modified: 2011-06-20 14:19 PDT (History)
3 users (show)

See Also:


Attachments
Patch (48.30 KB, patch)
2011-06-02 15:51 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff
Patch that does not use assign private data members in the copy constructor (47.83 KB, patch)
2011-06-09 17:56 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Robinson 2011-06-02 15:07:57 PDT
WebKitWebSettings could use a quick cleanup to remove unecessary boilerplate from and make private members meet WebKit style guidelines.
Comment 1 Martin Robinson 2011-06-02 15:51:30 PDT
Created attachment 95825 [details]
Patch
Comment 2 Martin Robinson 2011-06-09 15:10:20 PDT
Comment on attachment 95825 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=95825&action=review

> Source/WebKit/gtk/webkit/webkitwebsettings.cpp:1234
> -    return WEBKIT_WEB_SETTINGS(g_object_new(WEBKIT_TYPE_WEB_SETTINGS, NULL));
> +    return WEBKIT_WEB_SETTINGS(g_object_new(WEBKIT_TYPE_WEB_SETTINGS, 0));

This change is incorrect. When landing I will convert this back to NULL.
Comment 3 Xan Lopez 2011-06-09 16:20:42 PDT
Comment on attachment 95825 [details]
Patch

I honestly don't like assigning the private data instead of just creating the objects normally. Just because you can now do it without writing more code does not mean that you should...

Everything else seems fine.
Comment 4 Martin Robinson 2011-06-09 17:56:12 PDT
Created attachment 96678 [details]
Patch that does not use assign private data members in the copy constructor
Comment 5 Martin Robinson 2011-06-09 19:37:52 PDT
Created this followup bug to address problems with webkit_web_settings_copy: https://bugs.webkit.org/show_bug.cgi?id=62424
Comment 6 Eric Seidel (no email) 2011-06-16 21:20:31 PDT
Comment on attachment 96678 [details]
Patch that does not use assign private data members in the copy constructor

View in context: https://bugs.webkit.org/attachment.cgi?id=96678&action=review

Looks reasonable to me.  If you need a real gtk reviewer you should ask one of your gtk folks though. :)

> Source/WebKit/gtk/webkit/webkitwebsettings.cpp:906
> +    delete WEBKIT_WEB_SETTINGS(object)->priv;

Manual delete?  So sad!
Comment 7 WebKit Review Bot 2011-06-20 12:27:02 PDT
Comment on attachment 96678 [details]
Patch that does not use assign private data members in the copy constructor

Rejecting attachment 96678 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-01', '--port..." exit_code: 2

Last 500 characters of output:
ttingsprivate.h
patching file Source/WebKit/gtk/webkit/webkitwebview.cpp
Hunk #1 FAILED at 142.
Hunk #2 succeeded at 1306 (offset 1 line).
Hunk #3 succeeded at 1395 (offset 1 line).
Hunk #4 succeeded at 3175 (offset -7 lines).
Hunk #5 succeeded at 3474 (offset -7 lines).
1 out of 5 hunks FAILED -- saving rejects to file Source/WebKit/gtk/webkit/webkitwebview.cpp.rej

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Eric Seidel', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/8907682
Comment 8 Martin Robinson 2011-06-20 13:13:09 PDT
Committed r89282: <http://trac.webkit.org/changeset/89282>
Comment 9 Martin Robinson 2011-06-20 14:19:14 PDT
Comment on attachment 96678 [details]
Patch that does not use assign private data members in the copy constructor

Clearing flags after landing.