WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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-
Details
Formatted Diff
Diff
Rebased patch
(15.75 KB, patch)
2017-06-12 10:21 PDT
,
Carlos Garcia Campos
achristensen
: review+
Details
Formatted Diff
Diff
Patch for landing
(16.86 KB, patch)
2017-06-12 22:54 PDT
,
Carlos Garcia Campos
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Carlos Garcia Campos
Comment 1
2017-06-09 04:23:45 PDT
Created
attachment 312423
[details]
Patch
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
Committed
r218165
: <
http://trac.webkit.org/changeset/218165
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug