Bug 178100 – [WPE] Header cairo.h is used in GLib API headers but cannot be found

• Component: WPE WebKit
• Status: RESOLVED WONTFIX

[Adrian Perez]

When a program tries to use the GLib-based API for WPE (see
bug #178090 for a patch to make it installable), trying to
use the <wpe/webkit.h> header will result in the compilation
error shown below, due to “cairo.h” not being in the headers
search path.

---

In file included from /home/aperez/devel/wpe/buildroot/output/host/arm-buildroot-linux-gnueabihf/sysroot/usr/include/wpe-0.1/WPE/wpe/webkit.h:46:0,
                 from /home/aperez/devel/wpe/buildroot/output/build/dinghy-custom/dinghy.c:8:
/home/aperez/devel/wpe/buildroot/output/host/arm-buildroot-linux-gnueabihf/sysroot/usr/include/wpe-0.1/WPE/wpe/WebKitFaviconDatabase.h:27:19: fatal error: cairo.h: No such file or directory
 #include <cairo.h>
                   ^

[Adrian Perez]

I think we can fix this by adding “cairo” to the “Requires.private”
field of the .pc generated to be used by pkg-config. I'll try that
out and submit a patch.

[Adrian Perez]

Created attachment 323245 [details]
Patch

[Zan Dobersek]

Can we instead just use a forward declaration of cairo_surface_t, and drop the cairo.h include?

[Adrian Perez]

(In reply to Zan Dobersek from comment #3)
> Can we instead just use a forward declaration of cairo_surface_t, and drop
> the cairo.h include?

Very likely! I'll give that a try and update the patch accordingly
once I am sure it works.  :-)

[Adrian Perez]

Created attachment 323299 [details]
Patch

[Adrian Perez]

(In reply to Adrian Perez from comment #4)
> (In reply to Zan Dobersek from comment #3)
> > Can we instead just use a forward declaration of cairo_surface_t, and drop
> > the cairo.h include?
> 
> Very likely! I'll give that a try and update the patch accordingly
> once I am sure it works.  :-)

Worked perfectly, the new version of the patch adds only the
forward declaration and removes the #include \o/

[Adrian Perez]

Ouch, we'll have to add “#include <cairo.h>” to the .cpp files which
make use of “WebKitFaviconDatabase.h”, as Good Guy EWS™ has found out.

[Adrian Perez]

Created attachment 323326 [details]
Patch

[Build Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Zan Dobersek]

Comment on attachment 323326 [details]
Patch

Thanks, I appreciate the change.

[WebKit Commit Bot]

Comment on attachment 323326 [details]
Patch

Clearing flags on attachment: 323326

Committed r223136: <http://trac.webkit.org/changeset/223136>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Michael Catanzaro]

But you need to update the GTK header as well. We have to keep these headers completely in sync. Only exception should be symbols that do not exist in one port or the other.

[Michael Catanzaro]

Reverted r223136 for reason:

Forgot to update GTK API header

Committed r223138: <http://trac.webkit.org/changeset/223138>

[Michael Catanzaro]

(This was probably overkill for a rollout, but it's easier for backporting to land one fixed patch instead of an incremental fixup.)

[Adrian Perez]

(In reply to Michael Catanzaro from comment #13)
> But you need to update the GTK header as well. We have to keep these headers
> completely in sync. Only exception should be symbols that do not exist in
> one port or the other.

I'll prepare a new version of the patch that covers both WPE and GTK+.

[Adrian Perez]

Created attachment 323336 [details]
Patch

[Michael Catanzaro]

You shouldn't have to #include a new header to use any types from our API. Zan's instincts to avoid unneeded forward declarations are good usually, but not in public headers IMO.

Thanks to gtk-doc for catching this problem on the GTK EWS. :P

[Adrian Perez]

Created attachment 323347 [details]
Patch

[Michael Catanzaro]

Comment on attachment 323347 [details]
Patch

Perfect!

[WebKit Commit Bot]

Comment on attachment 323347 [details]
Patch

Clearing flags on attachment: 323347

Committed r223146: <http://trac.webkit.org/changeset/223146>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Michael Catanzaro]

Reverted r223146 for reason:

Better to not expose cairo in the WPE API

Committed r223174: <http://trac.webkit.org/changeset/223174>

[Michael Catanzaro]

We'll address this in bug #178157 instead.