Bug 178148

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:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

178173

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none

Chris Dumez

Reported 2017-10-10 15:59:31 PDT

Modernize Geolocation code.

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
Show Obsolete (10) View All Add attachment proposed patch, testcase, etc.

Chris Dumez

Comment 1 2017-10-10 20:08:43 PDT

Created attachment 323374 [details] Patch

Build Bot

Comment 2 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

Chris Dumez

Comment 3 2017-10-10 20:18:47 PDT

Created attachment 323376 [details] Patch

Chris Dumez

Comment 4 2017-10-10 20:32:44 PDT

Created attachment 323378 [details] Patch

Chris Dumez

Comment 5 2017-10-10 20:49:45 PDT

Created attachment 323380 [details] Patch

Chris Dumez

Comment 6 2017-10-10 20:56:54 PDT

Created attachment 323381 [details] Patch

Chris Dumez

Comment 7 2017-10-10 21:00:07 PDT

Created attachment 323382 [details] Patch

Chris Dumez

Comment 8 2017-10-10 21:08:40 PDT

Created attachment 323383 [details] Patch

Ryosuke Niwa

Comment 9 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.

Chris Dumez

Comment 10 2017-10-10 22:39:14 PDT

Created attachment 323388 [details] Patch

Chris Dumez

Comment 11 2017-10-11 08:44:00 PDT

Created attachment 323418 [details] Patch

Chris Dumez

Comment 12 2017-10-11 08:47:42 PDT

Created attachment 323419 [details] Patch

Chris Dumez

Comment 13 2017-10-11 09:37:10 PDT

Created attachment 323422 [details] Patch