|Summary:||[Chromium] Move geolocationEnabled setting into an EnabledAtRuntime v8 setting|
|Product:||WebKit||Reporter:||Jonathan Dixon <joth>|
|Component:||WebCore Misc.||Assignee:||Jonathan Dixon <joth>|
|Severity:||Normal||CC:||commit-queue, darin, jorlow, oliver, steveblock|
|Version:||528+ (Nightly build)|
|OS:||OS X 10.5|
|Bug Depends on:||32066|
Description Jonathan Dixon 2010-01-11 06:46:11 PST
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.
Comment 2 Darin Adler 2010-01-11 09:51:38 PST
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.
Comment 3 Darin Adler 2010-01-11 09:52:22 PST
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.
Comment 4 Jonathan Dixon 2010-01-15 05:11:33 PST
(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.
Comment 5 Jonathan Dixon 2010-01-15 07:31:30 PST
Created attachment 46680 [details] Patch 2 - using [EnabledAtRuntime] modifier
Comment 6 Jonathan Dixon 2010-01-15 07:41:49 PST
Created attachment 46681 [details] Patch 3 - missed a ChangeLog in previous one.
Comment 7 Jeremy Orlow 2010-01-15 11:15:37 PST
LGTM, but I'm not a reviewer.
Comment 8 Adam Barth 2010-01-15 18:58:27 PST
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 9 WebKit Commit Bot 2010-01-18 06:37:07 PST
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>
Comment 10 WebKit Commit Bot 2010-01-18 06:37:13 PST
All reviewed patches have been landed. Closing bug.