Bug 81451 - [Gtk] Webkit fails to build with --disable-geolocation
Summary: [Gtk] Webkit fails to build with --disable-geolocation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 80030
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-17 10:09 PDT by tuxator
Modified: 2012-03-23 15:14 PDT (History)
5 users (show)

See Also:


Attachments
Patch (5.55 KB, patch)
2012-03-23 12:55 PDT, Zan Dobersek
benjamin: review+
benjamin: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description tuxator 2012-03-17 10:09:10 PDT
After recent update from trunk WebKitGtk+ now fails to build with when using option --disable-geolocation
with the following output on the console: 

/bin/mkdir -p ./.deps/DerivedSources
  CXX    Source/WebKit/gtk/webkit/libwebkitgtk_1_0_la-webkitgeolocationpolicydecision.lo
In file included from Source/WebKit/gtk/webkit/webkitgeolocationpolicydecision.cpp:24:0:
Source/WebKit/gtk/webkit/webkitgeolocationpolicydecisionprivate.h:30:90: error: 'WebCore' has not been declared
Source/WebKit/gtk/webkit/webkitgeolocationpolicydecisionprivate.h:30:110: error: expected ',' or '...' before '*' token
Source/WebKit/gtk/webkit/webkitgeolocationpolicydecision.cpp:26:17: error: 'WebCore' is not a namespace-name
Source/WebKit/gtk/webkit/webkitgeolocationpolicydecision.cpp:26:24: error: expected namespace-name before ';' token
Source/WebKit/gtk/webkit/webkitgeolocationpolicydecision.cpp:42:5: error: 'Geolocation' does not name a type
Source/WebKit/gtk/webkit/webkitgeolocationpolicydecision.cpp:55:96: error: 'Geolocation' has not been declared
Source/WebKit/gtk/webkit/webkitgeolocationpolicydecision.cpp: In function 'WebKitGeolocationPolicyDecision* webkit_geolocation_policy_decision_new(WebKitWebFrame*, int*)':
Source/WebKit/gtk/webkit/webkitgeolocationpolicydecision.cpp:62:11: error: 'WebKitGeolocationPolicyDecisionPrivate' has no member named 'geolocation'
Source/WebKit/gtk/webkit/webkitgeolocationpolicydecision.cpp: In function 'void webkit_geolocation_policy_allow(WebKitGeolocationPolicyDecision*)':
Source/WebKit/gtk/webkit/webkitgeolocationpolicydecision.cpp:79:11: error: 'WebKitGeolocationPolicyDecisionPrivate' has no member named 'geolocation'
Source/WebKit/gtk/webkit/webkitgeolocationpolicydecision.cpp: In function 'void webkit_geolocation_policy_deny(WebKitGeolocationPolicyDecision*)':
Source/WebKit/gtk/webkit/webkitgeolocationpolicydecision.cpp:95:11: error: 'WebKitGeolocationPolicyDecisionPrivate' has no member named 'geolocation'
make[1]: *** [Source/WebKit/gtk/webkit/libwebkitgtk_1_0_la-webkitgeolocationpolicydecision.lo] Error 1
make[1]: Leaving directory `/home/pawel/src/WebKit'
make: *** [all] Error 2

I think that changeset that introduced this breakage is http://trac.webkit.org/changeset/110595
Comment 1 Adam Barth 2012-03-19 21:26:13 PDT
Do these files just need ENABLE(GEOLOCATION) guards?  I'm not sure what role they play in GTK.
Comment 2 Martin Robinson 2012-03-19 22:14:25 PDT
(In reply to comment #1)
> Do these files just need ENABLE(GEOLOCATION) guards?  I'm not sure what role they play in GTK.

Guarding the entire file wouldn't be too desirable since that would change the API based on the compilation options. Hopefully I can take a moment to fix this tomorrow or the next day.
Comment 3 Zan Dobersek 2012-03-23 12:55:19 PDT
Created attachment 133541 [details]
Patch
Comment 4 Zan Dobersek 2012-03-23 12:57:39 PDT
Comment on attachment 133541 [details]
Patch

Adding the cq flag.
Comment 5 Benjamin Poulain 2012-03-23 13:39:22 PDT
Comment on attachment 133541 [details]
Patch

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

> Source/WebKit/gtk/webkit/webkitgeolocationpolicydecision.cpp:-26
> -
> -using namespace WebCore;

This is a bit backward? Why removing the namespace use?

> Source/WebKit/gtk/webkit/webkitgeolocationpolicydecision.cpp:85
> +    WEBKIT_WARN_FEATURE_NOT_PRESENT("Geolocation")

Where does that macro come from?
A quick grep on the source returns nothing.
Comment 6 Zan Dobersek 2012-03-23 13:52:59 PDT
(In reply to comment #5)
> (From update of attachment 133541 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=133541&action=review
> 
> > Source/WebKit/gtk/webkit/webkitgeolocationpolicydecision.cpp:-26
> > -
> > -using namespace WebCore;
> 
> This is a bit backward? Why removing the namespace use?
> 

Keeping this would require it to be wrapped in an ifdef. Otherwise if geolocation support is disabled, including the Geolocation.h header basically does nothing, meaning the 'using namespace WebCore;' statement causes compilation error.

Because of that I switched to using WebCore::Geolocation rather than polluting the code with another ifdef.

> > Source/WebKit/gtk/webkit/webkitgeolocationpolicydecision.cpp:85
> > +    WEBKIT_WARN_FEATURE_NOT_PRESENT("Geolocation")
> 
> Where does that macro come from?
> A quick grep on the source returns nothing.

This is added in the patch in bug #80030.
Comment 7 Benjamin Poulain 2012-03-23 14:24:50 PDT
Comment on attachment 133541 [details]
Patch

Ok, looks good then. Land this after 80030.
Comment 8 Martin Robinson 2012-03-23 15:14:10 PDT
Committed r111915: <http://trac.webkit.org/changeset/111915>