Bug 32772 - Missing checks for Geolocation in Navigator.cpp
Summary: Missing checks for Geolocation in Navigator.cpp
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-12-19 09:05 PST by Philippe Normand
Modified: 2009-12-19 10:55 PST (History)
2 users (show)

See Also:


Attachments
proposed patch (2.05 KB, patch)
2009-12-19 09:07 PST, Philippe Normand
no flags Details | Formatted Diff | Diff
revert (5.14 KB, patch)
2009-12-19 10:44 PST, Evan Martin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Normand 2009-12-19 09:05:05 PST
If geolocation is disabled at configure time, the link will fail:

  CCLD   Programs/unittests/testhttpbackend
./.libs/libwebkit-1.0.so: undefined reference to `WebCore::Geolocation::disconnectFrame()'
./.libs/libwebkit-1.0.so: undefined reference to `WebCore::Geolocation::Geolocation(WebCore::Frame*)'
collect2: ld returned 1 exit status

Some ifdefs are missing.
Comment 1 Philippe Normand 2009-12-19 09:07:51 PST
Created attachment 45238 [details]
proposed patch

Reviewed by NOBODY (OOPS!).

        Missing checks for Geolocation in Navigator.cpp
        https://bugs.webkit.org/show_bug.cgi?id=32772

        Added ifdefs around Geolocation-related code.

        * page/Navigator.cpp:
        (WebCore::Navigator::disconnectFrame):
Comment 2 Darin Adler 2009-12-19 10:13:30 PST
Comment on attachment 45238 [details]
proposed patch

Is this the approach we're taking to ifdef'ing Geolocation? I thought that we were disabling this at the GeolocationService level, not here at the Navigator level.

If we are doing an ifdef at this level, then perhaps we need to put ifdefs in the header file too.
Comment 3 Philippe Normand 2009-12-19 10:19:51 PST
(In reply to comment #2)
> (From update of attachment 45238 [details])
> Is this the approach we're taking to ifdef'ing Geolocation? I thought that we
> were disabling this at the GeolocationService level, not here at the Navigator
> level.
> 
> If we are doing an ifdef at this level, then perhaps we need to put ifdefs in
> the header file too.

You are right, everything is already taken care of in GeolocationService, the problem is that this file doesn't seem to be built anymore after r52382 and if geolocation is disabled.
Comment 4 Philippe Normand 2009-12-19 10:29:53 PST
Comment on attachment 45238 [details]
proposed patch

This patch is a bad workaround for the bug. The correct fix would be to readd some of the files (including GeolocationService.cpp) in the build, even if geolocation is disabled.
Comment 5 Evan Martin 2009-12-19 10:44:17 PST
Created attachment 45241 [details]
revert
Comment 6 WebKit Review Bot 2009-12-19 10:46:03 PST
style-queue ran check-webkit-style on attachment 45241 [details] without any errors.
Comment 7 WebKit Commit Bot 2009-12-19 10:55:33 PST
Comment on attachment 45241 [details]
revert

Clearing flags on attachment: 45241

Committed r52392: <http://trac.webkit.org/changeset/52392>
Comment 8 WebKit Commit Bot 2009-12-19 10:55:38 PST
All reviewed patches have been landed.  Closing bug.