Summary: | Modernize Geolocation code | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||||||||||||||||||
Component: | WebCore Misc. | Assignee: | Chris Dumez <cdumez> | ||||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||||
Severity: | Normal | CC: | benjamin, berto, buildbot, cgarcia, commit-queue, gustavo, mcatanzaro, rniwa, webkit-bug-importer | ||||||||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||||||
Bug Depends on: | |||||||||||||||||||||||||||
Bug Blocks: | 178173 | ||||||||||||||||||||||||||
Attachments: |
|
Description
Chris Dumez
2017-10-10 15:59:31 PDT
Created attachment 323374 [details]
Patch
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API Created attachment 323376 [details]
Patch
Created attachment 323378 [details]
Patch
Created attachment 323380 [details]
Patch
Created attachment 323381 [details]
Patch
Created attachment 323382 [details]
Patch
Created attachment 323383 [details]
Patch
Comment on attachment 323383 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=323383&action=review r=me > Source/WebCore/Modules/geolocation/Coordinates.h:48 > + double latitude() const { return m_position.latitude; } > + double longitude() const { return m_position.longitude; } We should probably assert that these values aren't NaN/-1. > Source/WebCore/Modules/geolocation/GeolocationPosition.h:56 > + double latitude { -1 }; it's worth mentioning that these values are mandatory and cannot be omitted. We should also add asserts where it's used to make sure we never see -1. Also, we should probably use NaN instead of -1 per IRC discussion since latitude & longitude can take a negative value. Created attachment 323388 [details]
Patch
Created attachment 323418 [details]
Patch
Created attachment 323419 [details]
Patch
Created attachment 323422 [details]
Patch
Comment on attachment 323422 [details] Patch Clearing flags on attachment: 323422 Committed r223192: <https://trac.webkit.org/changeset/223192> All reviewed patches have been landed. Closing bug. |