Bug 30206 – Geolocation lacks V8 bindings

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Steve Block]

Geolocation lacks V8 bindings.

[Steve Block]

Created attachment 43930 [details]
Patch 1 for Bug 30206

[Steve Block]

Created attachment 44021 [details]
Patch 2 for Bug 30206

Updates build files for Chromium

[Adam Barth]

Attachment 44021 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/bindings/v8/custom/V8CoordinatesCustom.cpp:33:  You should add a blank line after implementation file's own header.  [build/include_order] [4]
WebCore/bindings/v8/custom/V8CoordinatesCustom.cpp:38:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/v8/custom/V8CoordinatesCustom.cpp:42:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
WebCore/bindings/v8/custom/V8CoordinatesCustom.cpp:51:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
WebCore/bindings/v8/custom/V8CoordinatesCustom.cpp:60:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
WebCore/bindings/v8/custom/V8CoordinatesCustom.cpp:69:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
WebCore/bindings/v8/custom/V8CoordinatesCustom.cpp:42:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
WebCore/bindings/v8/custom/V8CoordinatesCustom.cpp:51:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
WebCore/bindings/v8/custom/V8CoordinatesCustom.cpp:60:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
WebCore/bindings/v8/custom/V8CoordinatesCustom.cpp:69:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Done processing WebCore/bindings/v8/custom/V8CoordinatesCustom.cpp
Done processing WebCore/bindings/v8/DOMObjectsInclude.h
WebCore/bindings/v8/DerivedSourcesAllInOne.cpp:107:  Alphabetical sorting problem.  [build/include_order] [4]
Done processing WebCore/bindings/v8/DerivedSourcesAllInOne.cpp
Done processing WebCore/bindings/v8/custom/V8CustomPositionCallback.h
Done processing WebCore/bindings/v8/custom/V8CustomBinding.h
Done processing WebCore/bindings/v8/custom/V8CustomPositionErrorCallback.cpp
Done processing WebCore/bindings/v8/V8Index.cpp
Done processing WebCore/bindings/v8/custom/V8CustomPositionCallback.cpp
Done processing WebCore/bindings/v8/V8Index.h
WebCore/bindings/v8/custom/V8GeolocationCustom.cpp:33:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Done processing WebCore/bindings/v8/custom/V8GeolocationCustom.cpp
Done processing WebCore/bindings/v8/custom/V8CustomPositionErrorCallback.h
Total errors found: 12

[Dimitri Glazkov (Google)]

Comment on attachment 44021 [details]
Patch 2 for Bug 30206

Yeah, what the style-elf said.

BTW, do we need compile guards around geolocation?

[Steve Block]

Created attachment 44067 [details]
Patch 3 for Bug 30206

> Yeah, what the style-elf said.
Fixed. The script still reports a couple of errors regarding include order, because V8GeolocationCustom and V8CoordinatesCustom don't have a 'primary header' that follows the standard naming pattern.

> BTW, do we need compile guards around geolocation?
No, the decision taken for Geolocation was to not guard the (majority of the) code with enable flags, so as to make sure it's always built and doesn't rot (See Bug 25756). The guards are used only in Navigator.idl (to conditionally expose the JS geolocation object) and in GeolocationService.cpp (to satisfy the linker on platforms that don't implement GeolocationService).

[WebKit Review Bot]

Attachment 44067 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/bindings/v8/custom/V8CoordinatesCustom.cpp:27:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebCore/bindings/v8/custom/V8GeolocationCustom.cpp:27:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 2

[Dimitri Glazkov (Google)]

Comment on attachment 44067 [details]
Patch 3 for Bug 30206

ok.

[Eric Seidel (no email)]

(In reply to comment #6)
> Attachment 44067 [details] did not pass style-queue:
> 
> Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
> WebCore/bindings/v8/custom/V8CoordinatesCustom.cpp:27:  Found other header
> before a header this file implements. Should be: config.h, primary header,
> blank line, and then alphabetically sorted.  [build/include_order] [4]
> WebCore/bindings/v8/custom/V8GeolocationCustom.cpp:27:  Found other header
> before a header this file implements. Should be: config.h, primary header,
> blank line, and then alphabetically sorted.  [build/include_order] [4]
> Total errors found: 2

Do we need to file a bug against check-webkit-style here?

[WebKit Commit Bot]

Comment on attachment 44067 [details]
Patch 3 for Bug 30206

Clearing flags on attachment: 44067

Committed r51540: <http://trac.webkit.org/changeset/51540>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Adam Barth]

> Do we need to file a bug against check-webkit-style here?

Yes.  I think it doesn't understand includes for the custom bindings.  Can you file this one?

[Eric Seidel (no email)]

Filed bug 32033 to cover the false positives.