[Webkit-unassigned] [Bug 40374] Client-based Geolocation does not pass high power option to controller and client
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Jul 26 09:15:20 PDT 2010
https://bugs.webkit.org/show_bug.cgi?id=40374
Jeremy Orlow <jorlow at chromium.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #60005|review? |review-
Flag| |
--- Comment #13 from Jeremy Orlow <jorlow at chromium.org> 2010-07-26 09:15:20 PST ---
(From update of attachment 60005)
WebCore/ChangeLog:3
+ Reviewed by Alexey Proskuryakov.
change
WebKit/mac/ChangeLog:3
+ Reviewed by Alexey Proskuryakov.
ditto
WebKit/win/ChangeLog:3
+ Reviewed by Alexey Proskuryakov.
and again
WebCore/page/GeolocationController.cpp:80
+ m_client->setEnableHighAccuracy(false);
Why not just always set this? Let it decide if it's a no-op or not. I hate having state stored in multiple places when it's remotely possible to avoid it.
WebCore/page/GeolocationController.cpp:61
+ m_client->setEnableHighAccuracy(true);
ditto
WebCore/page/GeolocationController.h:62
+ ObserversSet m_observers;
I wonder if it'd be more clear to always have observers in just one set or the other. I.e. s/m_observers/m_lowAccuracyObserver/? Maybe not...but when I first looked at this code I was confused because I made this assumption.
I actually think using the high powered terminology in WebKit is the right answer since we can't really change that in the future and we know the spec is headed in that way. In WebCore, maybe add fixmes and wait to do it until the spec changes?
Sorry for so much thrashing. I'll try to do a follow up review quickly once you make these changes so you can get it in.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list