Summary: | [Chromium] Move geolocationEnabled setting into an EnabledAtRuntime v8 setting | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jonathan Dixon <joth> | ||||||||
Component: | WebCore Misc. | Assignee: | Jonathan Dixon <joth> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, darin, jorlow, oliver, steveblock | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | OS X 10.5 | ||||||||||
Bug Depends on: | 32066 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
Description
Jonathan Dixon
2010-01-11 06:46:11 PST
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. |