Bug 49132 – [GTK] Use __attribute__((constructor)) to call webkit_init at library init time

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebKitGTK

[Xan Lopez]

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.

[Xan Lopez]

Created attachment 73176 [details]
webkitinit.diff

[Martin Robinson]

Created attachment 73190 [details]
Version of the patch with Win32 support

[Fridrich Strba]

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.

[Martin Robinson]

Created attachment 73613 [details]
Patch with Fridrich's suggestions

[Gustavo Noronha (kov)]

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.

[Martin Robinson]

(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."

[WebKit Commit Bot]

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

[Xan Lopez]

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- :/

[Martin Robinson]

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.