Bug 178148 - Modernize Geolocation code
Summary: Modernize Geolocation code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks: 178173
  Show dependency treegraph
 
Reported: 2017-10-10 15:59 PDT by Chris Dumez
Modified: 2017-10-11 10:43 PDT (History)
9 users (show)

See Also:


Attachments
Patch (63.04 KB, patch)
2017-10-10 20:08 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (63.43 KB, patch)
2017-10-10 20:18 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (63.49 KB, patch)
2017-10-10 20:32 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (64.43 KB, patch)
2017-10-10 20:49 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (64.67 KB, patch)
2017-10-10 20:56 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (64.68 KB, patch)
2017-10-10 21:00 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (64.70 KB, patch)
2017-10-10 21:08 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (65.14 KB, patch)
2017-10-10 22:39 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (65.16 KB, patch)
2017-10-11 08:44 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (65.17 KB, patch)
2017-10-11 08:47 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (65.41 KB, patch)
2017-10-11 09:37 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2017-10-10 15:59:31 PDT
Modernize Geolocation code.
Comment 1 Chris Dumez 2017-10-10 20:08:43 PDT
Created attachment 323374 [details]
Patch
Comment 2 Build Bot 2017-10-10 20:11:20 PDT
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
Comment 3 Chris Dumez 2017-10-10 20:18:47 PDT
Created attachment 323376 [details]
Patch
Comment 4 Chris Dumez 2017-10-10 20:32:44 PDT
Created attachment 323378 [details]
Patch
Comment 5 Chris Dumez 2017-10-10 20:49:45 PDT
Created attachment 323380 [details]
Patch
Comment 6 Chris Dumez 2017-10-10 20:56:54 PDT
Created attachment 323381 [details]
Patch
Comment 7 Chris Dumez 2017-10-10 21:00:07 PDT
Created attachment 323382 [details]
Patch
Comment 8 Chris Dumez 2017-10-10 21:08:40 PDT
Created attachment 323383 [details]
Patch
Comment 9 Ryosuke Niwa 2017-10-10 22:23:09 PDT
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.
Comment 10 Chris Dumez 2017-10-10 22:39:14 PDT
Created attachment 323388 [details]
Patch
Comment 11 Chris Dumez 2017-10-11 08:44:00 PDT
Created attachment 323418 [details]
Patch
Comment 12 Chris Dumez 2017-10-11 08:47:42 PDT
Created attachment 323419 [details]
Patch
Comment 13 Chris Dumez 2017-10-11 09:37:10 PDT
Created attachment 323422 [details]
Patch
Comment 14 Chris Dumez 2017-10-11 10:42:04 PDT
Comment on attachment 323422 [details]
Patch

Clearing flags on attachment: 323422

Committed r223192: <https://trac.webkit.org/changeset/223192>
Comment 15 Chris Dumez 2017-10-11 10:42:05 PDT
All reviewed patches have been landed.  Closing bug.
Comment 16 Radar WebKit Bug Importer 2017-10-11 10:43:09 PDT
<rdar://problem/34936394>