Bug 49132 - [GTK] Use __attribute__((constructor)) to call webkit_init at library init time
Summary: [GTK] Use __attribute__((constructor)) to call webkit_init at library init time
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-11-06 14:03 PDT by Xan Lopez
Modified: 2011-04-14 15:45 PDT (History)
4 users (show)

See Also:


Attachments
webkitinit.diff (15.12 KB, patch)
2010-11-06 14:09 PDT, Xan Lopez
no flags Details | Formatted Diff | Diff
Version of the patch with Win32 support (14.80 KB, patch)
2010-11-07 00:39 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff
Patch with Fridrich's suggestions (15.38 KB, patch)
2010-11-11 07:59 PST, Martin Robinson
xan.lopez: review-
commit-queue: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Xan Lopez 2010-11-06 14:03:17 PDT
Right new we call it in all (?) our class constructors and non-OOP methods, which is a gross hack and error prone. This is the future.
Comment 1 Xan Lopez 2010-11-06 14:09:40 PDT
Created attachment 73176 [details]
webkitinit.diff
Comment 2 Martin Robinson 2010-11-07 00:39:51 PDT
Created attachment 73190 [details]
Version of the patch with Win32 support
Comment 3 Fridrich Strba 2010-11-10 23:13:41 PST
1) Generalize the define of WEBKIT_CONSTRUCTOR so that it is defined not only for not Windows
2) Use OS(WINDOWS) instead of PLATFORM(WIN)
3) Move the DLLMain definition under the webkit_init definition since it is used in the DLLMain.
Apart that, it compiles on Windows.
Comment 4 Martin Robinson 2010-11-11 07:59:37 PST
Created attachment 73613 [details]
Patch with Fridrich's suggestions
Comment 5 Gustavo Noronha (kov) 2010-11-11 13:29:07 PST
Comment on attachment 73613 [details]
Patch with Fridrich's suggestions

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

You guys are great =).

> WebCore/platform/graphics/gtk/ImageGtk.cpp:46
> +    if (!module) {
> +        GetModuleHandleEx(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT,
> +                          static_cast<LPCSTR>(getWebKitDataDirectory), &module);
> +    }

Should not have the bracers here.
Comment 6 Martin Robinson 2010-11-11 13:44:45 PST
(In reply to comment #5)

Thanks for the review!

> > WebCore/platform/graphics/gtk/ImageGtk.cpp:46
> > +    if (!module) {
> > +        GetModuleHandleEx(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT,
> > +                          static_cast<LPCSTR>(getWebKitDataDirectory), &module);
> > +    }
> 

> Should not have the bracers here.

I think they've recently updated the rule at http://webkit.org/coding/coding-style.html: "One-line control clauses should not use braces unless comments are included or a single statement spans multiple lines."
Comment 7 WebKit Commit Bot 2010-12-14 01:45:14 PST
Comment on attachment 73613 [details]
Patch with Fridrich's suggestions

Rejecting attachment 73613 [details] from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'apply-attachment', '--non-interactive', 73613]" exit_code: 2
Last 500 characters of output:
t/webkitwebsettings.cpp
Hunk #1 succeeded at 241 (offset 1 line).
patching file WebKit/gtk/webkit/webkitwebview.cpp
Hunk #1 succeeded at 1752 (offset 109 lines).
Hunk #2 succeeded at 5066 (offset 254 lines).
Hunk #3 succeeded at 5096 (offset 254 lines).
Hunk #4 succeeded at 5146 (offset 256 lines).
patching file WebKit/gtk/webkit/webkitwebwindowfeatures.cpp

Failed to run "[u'/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Gustavo Noronha Silva', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/6946108
Comment 8 Xan Lopez 2010-12-14 04:31:02 PST
Comment on attachment 73613 [details]
Patch with Fridrich's suggestions

Unfortunately I think this *still* is not good enough for Windows, and the patch would need some updates by now. Marking as r- :/
Comment 9 Martin Robinson 2011-04-14 15:45:11 PDT
Unfortunately I don't think this will ever be good enough. We aren't supposed to do very much in the shared object constructors and this patch does way too much there.