Bug https://bugs.webkit.org/show_bug.cgi?id=32066 added a new parameter, geolocationEnabled, to the Settings class. However nothing makes use of this -- somehow the change to Navigator.cpp to use it got missed out of the patch. Need to add code to conditionally (runtime) decide whether to create the Navigtor::m_geolocation object based on the above setting.
Created attachment 46272 [details] First patch
Comment on attachment 46272 [details] First patch Note that this changes behavior if the first access to geolocation occurs when m_frame is 0 or m_frame is no longer attached to a page even if geolocation is enabled. Maybe that's OK; it could be strange edge case to check something like this in a frame after the window is closed (one case where m_frame is 0) or after the frame is removed from its parent frame (one case where m_frame->page() is 0). But it would be fairly straightforward to construct a test case that could see the difference.
In the past we have instead made the object in question behave differently based on the setting rather than using 0 instead of the object.
(In reply to comment #3) > In the past we have instead made the object in question behave differently > based on the setting rather than using 0 instead of the object. Thanks for the comments Darin: digging into them, Jeremy pointed me to a much better way to do what I wanted using the [EnabledAtRuntime] flag in the IDL file to control this, which will address both your comments. Just creating a patch now to do this.
Created attachment 46680 [details] Patch 2 - using [EnabledAtRuntime] modifier
Created attachment 46681 [details] Patch 3 - missed a ChangeLog in previous one.
LGTM, but I'm not a reviewer.
Comment on attachment 46681 [details] Patch 3 - missed a ChangeLog in previous one. LGTM. I wish this code were auto-generated from the IDL attribute.
Comment on attachment 46681 [details] Patch 3 - missed a ChangeLog in previous one. Clearing flags on attachment: 46681 Committed r53406: <http://trac.webkit.org/changeset/53406>
All reviewed patches have been landed. Closing bug.