Summary: | Geolocation lacks V8 bindings | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Steve Block <steveblock> | ||||||||
Component: | WebCore Misc. | Assignee: | Steve Block <steveblock> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, android-webkit-unforking, commit-queue, eric, joth, steveblock, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Steve Block
2009-10-08 05:01:03 PDT
Created attachment 43930 [details] Patch 1 for Bug 30206 Created attachment 44021 [details] Patch 2 for Bug 30206 Updates build files for Chromium 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
Comment on attachment 44021 [details] Patch 2 for Bug 30206 Yeah, what the style-elf said. BTW, do we need compile guards around geolocation? 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). 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
Comment on attachment 44067 [details] Patch 3 for Bug 30206 ok. (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? Comment on attachment 44067 [details] Patch 3 for Bug 30206 Clearing flags on attachment: 44067 Committed r51540: <http://trac.webkit.org/changeset/51540> All reviewed patches have been landed. Closing bug. > 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?
Filed bug 32033 to cover the false positives. |