RESOLVED FIXED 173150
Add API::GeolocationProvider
https://bugs.webkit.org/show_bug.cgi?id=173150
Summary Add API::GeolocationProvider
Carlos Garcia Campos
Reported 2017-06-09 04:22:01 PDT
We will use this instead of the C API in GTK+.
Attachments
Patch (15.75 KB, patch)
2017-06-09 04:23 PDT, Carlos Garcia Campos
achristensen: review-
Rebased patch (15.75 KB, patch)
2017-06-12 10:21 PDT, Carlos Garcia Campos
achristensen: review+
Patch for landing (16.86 KB, patch)
2017-06-12 22:54 PDT, Carlos Garcia Campos
no flags
Carlos Garcia Campos
Comment 1 2017-06-09 04:23:45 PDT
Alex Christensen
Comment 2 2017-06-09 10:23:37 PDT
Comment on attachment 312423 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=312423&action=review > Source/WebKit2/UIProcess/WebGeolocationManagerProxy.h:84 > + std::unique_ptr<API::GeolocationProvider> m_provider; I think this is wrong. Usually we have the API object own the WebKit namespace object.
Carlos Garcia Campos
Comment 3 2017-06-12 10:21:59 PDT
Created attachment 312677 [details] Rebased patch It should apply now
Alex Christensen
Comment 4 2017-06-12 10:27:34 PDT
Comment on attachment 312677 [details] Rebased patch View in context: https://bugs.webkit.org/attachment.cgi?id=312677&action=review > Source/WebKit2/UIProcess/WebGeolocationManagerProxy.h:50 > + void setProvider(std::unique_ptr<API::GeolocationProvider>); && > Source/WebKit2/UIProcess/WebGeolocationProvider.h:44 > +class WebGeolocationProvider : public API::GeolocationProvider, API::Client<WKGeolocationProviderBase> { extra space
Carlos Garcia Campos
Comment 5 2017-06-12 10:33:50 PDT
(In reply to Alex Christensen from comment #4) > Comment on attachment 312677 [details] > Rebased patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=312677&action=review > > > Source/WebKit2/UIProcess/WebGeolocationManagerProxy.h:50 > > + void setProvider(std::unique_ptr<API::GeolocationProvider>); > > && That was also my idea at first, but then again I followed what we do for all other clients. In this particular case, I think that what we have is equivalent to && because we always pass a std::make_unique<> (or nullptr) to the function, so there aren't copies anyway. > > Source/WebKit2/UIProcess/WebGeolocationProvider.h:44 > > +class WebGeolocationProvider : public API::GeolocationProvider, API::Client<WKGeolocationProviderBase> { > > extra space Good catch!
Darin Adler
Comment 6 2017-06-12 14:57:25 PDT
(In reply to Carlos Garcia Campos from comment #5) > (In reply to Alex Christensen from comment #4) > > > Source/WebKit2/UIProcess/WebGeolocationManagerProxy.h:50 > > > + void setProvider(std::unique_ptr<API::GeolocationProvider>); > > > > && > > That was also my idea at first, but then again I followed what we do for all > other clients. In this particular case, I think that what we have is > equivalent to && because we always pass a std::make_unique<> (or nullptr) to > the function, so there aren't copies anyway. I don’t know if we ever resolved this. Here is what Scott Meyers has to say: http://scottmeyers.blogspot.com/2014/07/should-move-only-types-ever-be-passed.html
Carlos Garcia Campos
Comment 7 2017-06-12 22:54:01 PDT
Created attachment 312751 [details] Patch for landing
Carlos Garcia Campos
Comment 8 2017-06-12 23:59:34 PDT
Note You need to log in before you can comment on or make changes to this bug.