Bug 123663 – [GTK] Missing symbols in 2.3.1, ABI break?

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Emilio Pozuelo Monfort]

libwebkitgtk-1.0.so, libwebkitgtk-3.0.so and libwebkit2gtk-3.0.so used to have these symbols:

 webkit_dom_html_head_element_get_profile@Base 1.3.10
 webkit_dom_html_head_element_set_profile@Base 1.3.10
 webkit_dom_processing_instruction_get_data@Base 1.3.10
 webkit_dom_processing_instruction_set_data@Base 1.3.10

They were exported in public headers, but are now gone. Isn't this an ABI break or is there a special reason why these were considered private?

[Zan Dobersek]

These are (were) generated GObject bindings functions that had their underlying implementation removed. Stubs should be put in place, I believe.

[Carlos Garcia Campos]

Yes, we need to bring the symbols back.

[Emilio Pozuelo Monfort]

I'm testing a patch for this.

[Emilio Pozuelo Monfort]

Created attachment 215881 [details]
Patch

[Emilio Pozuelo Monfort]

(In reply to comment #4)
> Created an attachment (id=215881) [details]
> Patch

The patch is working fine for webkit_dom_processing_instruction_[gs]et_data but not for webkit_dom_html_head_element_[gs]et_profile for some reason...

[Emilio Pozuelo Monfort]

(In reply to comment #5)
> (In reply to comment #4)
> > Created an attachment (id=215881) [details] [details]
> > Patch
> 
> The patch is working fine for webkit_dom_processing_instruction_[gs]et_data but not for webkit_dom_html_head_element_[gs]et_profile for some reason...

Maybe I need to add them to WebCore/bindings/gobject/webkitdom.symbols instead of to WebKitDOMCustom.symbols. Carlos, you have written all this symbols logic, any idea about this?

[Emilio Pozuelo Monfort]

It looks like webkitdom.symbols is a superset of WebKitDOMCustom.symbols but is not autogenerated, perhaps WebKitDOMCustom.symbols is obsolete?

[Carlos Garcia Campos]

(In reply to comment #7)
> It looks like webkitdom.symbols is a superset of WebKitDOMCustom.symbols but is not autogenerated, perhaps WebKitDOMCustom.symbols is obsolete?

webkitdom.symbols contains all the API symbols and it's generated by the script gobject-run-api-break-test when manually run with --reset-results. The patch looks correct, you have to add the symbols to WebKitDOMCustom.symbols. What's exactly the error you are getting?

[Emilio Pozuelo Monfort]

I'm not getting any error, but the resulting libwebkitgtk-[13].0.so.0 export webkit_dom_processing_instruction_[gs]et_data but not webkit_dom_html_head_element_[gs]et_profile as they should.

[Carlos Garcia Campos]

(In reply to comment #9)
> I'm not getting any error, but the resulting libwebkitgtk-[13].0.so.0 export webkit_dom_processing_instruction_[gs]et_data but not webkit_dom_html_head_element_[gs]et_profile as they should.

That's weird. The .symbols file is only to check API breaks while building and to generate the API docs, it doesn't affect in any way the symbols actually exported in the lib. Any public symbol prefixed with WEBKIT_API should be public in the lib.

[Emilio Pozuelo Monfort]

I have tried with git master (I was testing on 2.3.1 before), same results:

emilio@titan:/opt/WebKit/lib$ nm -D libwebkitgtk-1.0.so.0 | grep webkit_dom_html_head_element_get_profile
emilio@titan:/opt/WebKit/lib$ nm -D libwebkitgtk-1.0.so.0 | grep webkit_dom_processing_instruction_get_data
0000000001091270 T webkit_dom_processing_instruction_get_data


Interesting though:
emilio@titan:/opt/WebKit/lib$ grep webkit_dom_html_head_element_get_profile libwebkitgtk-1.0.so.0.20.0
Binary file libwebkitgtk-1.0.so.0.20.0 matches


Poking a bit more:

emilio@titan:~/src/WebKit/.libs$ nm libGObjectDOMBindings.a | grep webkit_dom_html_head_element_get_profile
0000000000001020 T _Z40webkit_dom_html_head_element_get_profileP25_WebKitDOMHTMLHeadElement
00000000000000c0 r _ZZ40webkit_dom_html_head_element_get_profileP25_WebKitDOMHTMLHeadElementE8__func__
emilio@titan:~/src/WebKit/.libs$ nm libGObjectDOMBindings.a | grep webkit_dom_processing_instruction_get_data
0000000000001070 T webkit_dom_processing_instruction_get_data
0000000000000040 r _ZZ42webkit_dom_processing_instruction_get_dataE8__func__


emilio@titan:~/src/WebKit/.libs$ echo "_Z40webkit_dom_html_head_element_get_profileP25_WebKitDOMHTMLHeadElement" | c++filt 
webkit_dom_html_head_element_get_profile(_WebKitDOMHTMLHeadElement*)
emilio@titan:~/src/WebKit/.libs$ echo "_ZZ42webkit_dom_processing_instruction_get_dataE8__func__" | c++filt 
webkit_dom_processing_instruction_get_data::__func__


I don't know much about c++ symbol mangling but that seems suspicious.

[Carlos Garcia Campos]

I've just build with your patch (after fixing the conflicts) and it seems to work fine here:

$ nm -D libwebkitgtk-3.0.so.0.20.0 | grep webkit_dom_html_head_element_get_profile
00000000019a57a0 T webkit_dom_html_head_element_get_profile
$ nm -D libwebkitgtk-3.0.so.0.20.0 | grep webkit_dom_processing_instruction_get_data
00000000019a57f0 T webkit_dom_processing_instruction_get_data
$ nm -D libwebkit2gtk-3.0.so.25.5.0 | grep webkit_dom_html_head_element_get_profile
0000000001f1ceb0 T webkit_dom_html_head_element_get_profile
$ nm -D libwebkit2gtk-3.0.so.25.5.0 | grep webkit_dom_processing_instruction_get_data
0000000001f1cf00 T webkit_dom_processing_instruction_get_data

[Emilio Pozuelo Monfort]

Do you have GCC 4.8 ?

[Carlos Garcia Campos]

Comment on attachment 215881 [details]
Patch

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

Please make sure the patch applies in current git master.

> Source/WebCore/bindings/gobject/WebKitDOMCustom.cpp:563
> +// WebKitDOMHTMLElement

This should be WebKitDOMHTMLHeadElement

> Source/WebCore/bindings/gobject/WebKitDOMCustom.h:478
> + * Returns:

Add something after the Returns: tag to avoid a warning when generating docs. It could be just Returns: a #gchar

> Source/WebCore/bindings/gobject/WebKitDOMCustom.h:481
> +WEBKIT_API gchar* webkit_dom_html_head_element_get_profile(void* self);

Use WebKitDOMHTMLHeadElement* instead of void*. We use only void* when the whole class has been removed from the API.

> Source/WebCore/bindings/gobject/WebKitDOMCustom.h:493
> + * Returns:
> + *
> +**/
> +WEBKIT_API void webkit_dom_html_head_element_set_profile(void* self, const gchar* value);

Same comments here.

[Carlos Garcia Campos]

(In reply to comment #13)
> Do you have GCC 4.8 ?

g++ (Debian 4.8.2-1) 4.8.2

[Emilio Pozuelo Monfort]

Created attachment 216020 [details]
Patch

[Emilio Pozuelo Monfort]

New patch attached addressing all your comments.

(In reply to comment #14)
> > Source/WebCore/bindings/gobject/WebKitDOMCustom.h:481
> > +WEBKIT_API gchar* webkit_dom_html_head_element_get_profile(void* self);
> 
> Use WebKitDOMHTMLHeadElement* instead of void*. We use only void* when the whole class has been removed from the API.
> 
> > Source/WebCore/bindings/gobject/WebKitDOMCustom.h:493
> > + * Returns:
> > + *
> > +**/
> > +WEBKIT_API void webkit_dom_html_head_element_set_profile(void* self, const gchar* value);
> 
> Same comments here.

I think that may have been the cause of the problem I was seeing (which I don't anymore) as the prototype in WebKitDOMCustom.h and WebKitDOMCustom.cpp differed. Everything's working fine now.

[Carlos Garcia Campos]

Comment on attachment 216020 [details]
Patch

Thanks!

[WebKit Commit Bot]

Comment on attachment 216020 [details]
Patch

Rejecting attachment 216020 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 216020, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in Source/WebCore/ChangeLog contains OOPS!.

Full output: http://webkit-queues.appspot.com/results/19578976

[Emilio Pozuelo Monfort]

Created attachment 216023 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 216023 [details]
Patch

Clearing flags on attachment: 216023

Committed r158662: <http://trac.webkit.org/changeset/158662>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.