Bug 33467 – [Chromium] Move geolocationEnabled setting into an EnabledAtRuntime v8 setting

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Jonathan Dixon]

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.

[Jonathan Dixon]

Created attachment 46272 [details]
First patch

[Darin Adler]

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.

[Darin Adler]

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.

[Jonathan Dixon]

(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.

[Jonathan Dixon]

Created attachment 46680 [details]
Patch 2 - using [EnabledAtRuntime] modifier

[Jonathan Dixon]

Created attachment 46681 [details]
Patch 3 - missed a ChangeLog in previous one.

[Jeremy Orlow]

LGTM, but I'm not a reviewer.

[Adam Barth]

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.

[WebKit Commit Bot]

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>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.