| Summary: | [GTK] Clean up unecessary boilerplate from WebKitWebSettings and make private members meet WebKit style guidelines | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Martin Robinson <mrobinson> | ||||||
| Component: | WebKitGTK | Assignee: | Martin Robinson <mrobinson> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | eric, webkit.review.bot, xan.lopez | ||||||
| Priority: | P3 | Keywords: | Gtk | ||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | PC | ||||||||
| OS: | Linux | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 62424 | ||||||||
| Attachments: |
|
||||||||
|
Description
Martin Robinson
2011-06-02 15:07:57 PDT
Created attachment 95825 [details]
Patch
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 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.
Created attachment 96678 [details]
Patch that does not use assign private data members in the copy constructor
Created this followup bug to address problems with webkit_web_settings_copy: https://bugs.webkit.org/show_bug.cgi?id=62424 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 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 Committed r89282: <http://trac.webkit.org/changeset/89282> Comment on attachment 96678 [details]
Patch that does not use assign private data members in the copy constructor
Clearing flags after landing.
|