Bug 176496 – Modernize Geolocation code

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alex Christensen]

Modernize Geolocation code

[Alex Christensen]

Created attachment 320098 [details]
Patch

[Alex Christensen]

Created attachment 320099 [details]
Patch

[Alex Christensen]

Created attachment 320102 [details]
Patch

[Sam Weinig]

Comment on attachment 320102 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=320102&action=review

> Source/WebCore/Modules/geolocation/GeolocationController.cpp:-114
> -    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)
    {
        Vector<T> result;
        result.reserveInitialCapacity(collection.size());
        for (auto& value : collection)
            result.uncheckedAppend(WTFCopy(value));
    }

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.

> Source/WebKit/ChangeLog:10
> +        * NetworkProcess/CustomProtocols/LegacyCustomProtocolManager.cpp:
> +        (WebKit::LegacyCustomProtocolManager::LegacyCustomProtocolManager):
> +        (WebKit::LegacyCustomProtocolManager::startLoading):

ChangeLog could use some additional information.  Much of this looks un-Geolocation related.

[Sam Weinig]

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

[Alex Christensen]

Created attachment 320134 [details]
Patch

[Alex Christensen]

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.

[Alex Christensen]

http://trac.webkit.org/r221743

[Radar WebKit Bug Importer]

<rdar://problem/34693666>