Bug 32066 - [Chromium] Add enable geolocation flag to WebCore::Settings for Chromium
Summary: [Chromium] Add enable geolocation flag to WebCore::Settings for Chromium
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: Jonathan Dixon
URL:
Keywords:
Depends on:
Blocks: 32065 32067 33467
  Show dependency treegraph
 
Reported: 2009-12-02 06:35 PST by Jonathan Dixon
Modified: 2010-01-11 06:46 PST (History)
5 users (show)

See Also:


Attachments
Patch 1 for Bug 32066 (8.80 KB, patch)
2009-12-02 07:36 PST, Jonathan Dixon
no flags Details | Formatted Diff | Diff
Patch 2 for Bug 32066 (9.21 KB, patch)
2009-12-02 08:13 PST, Jonathan Dixon
no flags Details | Formatted Diff | Diff
Patch 3 for Bug 32066 (9.21 KB, patch)
2009-12-03 10:33 PST, Jonathan Dixon
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Dixon 2009-12-02 06:35:32 PST
In the Chromium port, the geolocation implementation will be ready for testing at different times on different platforms. Further, testing can commence before final UI and feature completion.
To ease integration, we need run time control of whether geolocation is enabled, to allow testing (e.g. in the dev channel) without requiring a complete rebuild.

This flag is not intended to be revealed in the UI. (e.g. on chromium it will be a command line flag available during the development cycle only)
When the flag is off, the navigator.geolocation object should not be set, indicating the browser has no geolocation support.

The ENABLE(GEOLOCATION) compile time flag is still required to completely compile out support for those platforms that do not require it.
Comment 1 Jonathan Dixon 2009-12-02 07:36:03 PST
Created attachment 44146 [details]
Patch 1 for Bug 32066
Comment 2 WebKit Review Bot 2009-12-02 07:39:03 PST
style-queue ran check-webkit-style on attachment 44146 [details] without any errors.
Comment 3 Jonathan Dixon 2009-12-02 08:13:14 PST
Created attachment 44150 [details]
Patch 2 for Bug 32066

Updated with comments from colleague
Comment 4 WebKit Review Bot 2009-12-02 08:15:40 PST
style-queue ran check-webkit-style on attachment 44150 [details] without any errors.
Comment 5 Jonathan Dixon 2009-12-03 02:29:43 PST
Would you be able to look at this Eric, or suggest someone else to?
Comment 6 Dimitri Glazkov (Google) 2009-12-03 08:00:24 PST
Comment on attachment 44150 [details]
Patch 2 for Bug 32066

> +    , m_geolocationEnabled(true)

Is the case for other ports? Do we need to plumb this through for WebKit/mac&win?
Comment 7 Jonathan Dixon 2009-12-03 08:20:11 PST
(In reply to comment #6)
> (From update of attachment 44150 [details])
> > +    , m_geolocationEnabled(true)
> 
> Is the case for other ports? Do we need to plumb this through for
> WebKit/mac&win?

The intention is in the "normal case" a port either has no support for the feature, so does not define ENABLE_GEOLOCATION, or has full support, in which case this flag can default to true -- i.e. equivalent to the existing situation without the existence of this flag, and without plumbing in calls to it.
(It also happens that at the current time no port has ENABLE_GEOLOCATION defined, so plumbing it would have no effect anyway)

I'm very happy to explain this in code comments some place, but not sure where is the best location for it?
Comment 8 Jonathan Dixon 2009-12-03 10:33:30 PST
Created attachment 44254 [details]
Patch 3 for Bug 32066

TODO => FIXME
Comment 9 Dimitri Glazkov (Google) 2009-12-03 10:35:52 PST
Comment on attachment 44254 [details]
Patch 3 for Bug 32066

r=me.
Comment 10 Eric Seidel (no email) 2009-12-03 11:48:35 PST
Seems a little strange to have two ways to enable Geolocation, but I'm also not sure I have anything better to suggest.  This isn't the first feature for which we have a runtime switch, certainly.
Comment 11 WebKit Commit Bot 2009-12-03 13:54:18 PST
Comment on attachment 44254 [details]
Patch 3 for Bug 32066

Clearing flags on attachment: 44254

Committed r51660: <http://trac.webkit.org/changeset/51660>
Comment 12 WebKit Commit Bot 2009-12-03 13:54:25 PST
All reviewed patches have been landed.  Closing bug.