Modernize Geolocation code
Created attachment 320098 [details]
Created attachment 320099 [details]
Created attachment 320102 [details]
Comment on attachment 320102 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=320102&action=review
> - copyToVector(m_observers, observersVector);
Seems like we should have an overload of copyToVector that looks like:
template<typename T, typename U, typename V>
inline Vector<T> copyToVector(const HashSet<T, U, V>& collection)
for (auto& value : collection)
Where WTFCopy is a new function that usually just assigns, but is overloaded for Ref<> to call copyRef().
Maybe a big enough bit of infrastructure that you don't want to do it here.
> + * NetworkProcess/CustomProtocols/LegacyCustomProtocolManager.cpp:
> + (WebKit::LegacyCustomProtocolManager::LegacyCustomProtocolManager):
> + (WebKit::LegacyCustomProtocolManager::startLoading):
ChangeLog could use some additional information. Much of this looks un-Geolocation related.
The biggest cleanup / improvement in this code that you could do would be to find a way to ensure that GeoNotifier::runSuccessCallback() always took a reference. Largely this revolves around figuring out what to do with Geolocation::lastPosition().
Created attachment 320134 [details]
I'm trying to make this have no change in behavior and that would be a change. We can also do the vector cleanup later.