Bug 42545

Summary: WebKitTestRunner needs layoutTestController.setGeolocationPermission
Product: WebKit Reporter: Maciej Stachowiak <mjs>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abecsi, benjamin, hnandor, maheshk, ossy, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 55872    
Attachments:
Description Flags
Implementation of setGeolocationPermission
none
Implementation of setGeolocationPermission
none
Implementation of setGeolocationPermission
none
Implementation of setGeolocationPermission
none
Implementation of setGeolocationPermission
none
Implementation of setGeolocationPermission
none
Implementation of setGeolocationPermission none

Description Maciej Stachowiak 2010-07-18 21:47:15 PDT
WebKitTestRunner needs layoutTestController.setGeolocationPermission
Comment 1 Maciej Stachowiak 2010-07-18 21:52:12 PDT
<rdar://problem/8204915>
Comment 2 Mahesh Kulkarni 2011-04-22 17:09:08 PDT
*** Bug 59202 has been marked as a duplicate of this bug. ***
Comment 3 Nandor Huszka 2012-01-20 00:32:47 PST
Created attachment 123262 [details]
Implementation of setGeolocationPermission

WebKit must be builded with the Tools/Scripts/build-webkit DEFINES+=ENABLE_CLIENT_BASED_GEOLOCATION=1 command, if we want to test this special function.
I have move tests to the appropriate group, most of tests which were at label "# WebKitTestRunner needs layoutTestController.setGeolocationPermission" now need layoutTestController.setMockGeolocationPosition or layoutTestController.setMockGeolocationError. Two of these tests sometimes cause WebProcess crash, they have been labeled too.
Comment 4 WebKit Review Bot 2012-01-20 00:34:04 PST
Attachment 123262 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1

LayoutTests/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Nandor Huszka 2012-01-20 01:14:24 PST
Created attachment 123268 [details]
Implementation of setGeolocationPermission

Fixing the style issue.
Comment 6 Nandor Huszka 2012-01-20 04:19:24 PST
Created attachment 123286 [details]
Implementation of setGeolocationPermission

From win buildlog:
11>..\WebProcess\InjectedBundle\InjectedBundle.cpp(52) : fatal error C1083: Cannot open include file: 'WebCore/page/GeolocationClient.h': No such file or directory

That is why I have replace the imports, e.g. #include <WebCore/page/GeolocationController.h> with #include "GeolocationClient.h".
Comment 7 WebKit Review Bot 2012-01-20 04:22:29 PST
Attachment 123286 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1

Source/WebKit2/WebProcess/InjectedBundle/InjectedBundle.cpp:33:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Nandor Huszka 2012-01-20 04:26:26 PST
Created attachment 123289 [details]
Implementation of setGeolocationPermission

Style fixing again, imports were in wrong order.
Comment 9 Andras Becsi 2012-01-24 06:13:22 PST
(In reply to comment #8)
> Created an attachment (id=123289) [details]
> Implementation of setGeolocationPermission
> 
> Style fixing again, imports were in wrong order.

Try using #include <WebCore/...> for the headers, this should fix the windows build.
Comment 10 Nandor Huszka 2012-01-24 06:48:50 PST
Created attachment 123727 [details]
Implementation of setGeolocationPermission 

Andras Becsi, thank you for the tip, I have done that.
Comment 11 Nandor Huszka 2012-01-24 07:32:58 PST
Created attachment 123731 [details]
Implementation of setGeolocationPermission 

I have inserted a new line in /WebCore/WebCore.vcproj/copyForwardingHeaders.cmd:

xcopy /y /d "%ProjectDir%..\platform\mock\*.h" "%CONFIGURATIONBUILDDIR%\include\WebCore"

I hope GeolocationClientMock.h can be found with this.
Comment 12 Simon Hausmann 2012-01-26 00:21:38 PST
Comment on attachment 123731 [details]
Implementation of setGeolocationPermission 

View in context: https://bugs.webkit.org/attachment.cgi?id=123731&action=review

> Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePrivate.h:73
> +WK_EXPORT void WKBundlesetGeolocationPermission(WKBundleRef bundle, WKBundlePageGroupRef pageGroup, bool enabled);

WKBundlesetGeolocationPermission -> WKBundle_S_etGeolocationPermission

> Source/WebKit2/WebProcess/InjectedBundle/InjectedBundle.cpp:171
> +        (static_cast<GeolocationClientMock*>((*iter)->geolocationController()->client()))->setPermission(enabled);

I think you can remove the parentheses around the static_cast, i.e. write

    static_cast<Foo*>(bar)->blub();

instead of

    (static_cast<Foo*>(bar))->blub();
Comment 13 Nandor Huszka 2012-01-26 01:08:56 PST
Created attachment 124074 [details]
Implementation of setGeolocationPermission

Simon, thank you, I have fixed the style issues that you have mentioned.
Comment 14 Zoltan Herczeg 2012-01-26 02:06:35 PST
Comment on attachment 124074 [details]
Implementation of setGeolocationPermission

r=me
Comment 15 WebKit Review Bot 2012-01-26 04:16:40 PST
Comment on attachment 124074 [details]
Implementation of setGeolocationPermission

Clearing flags on attachment: 124074

Committed r105988: <http://trac.webkit.org/changeset/105988>
Comment 16 WebKit Review Bot 2012-01-26 04:16:45 PST
All reviewed patches have been landed.  Closing bug.
Comment 17 Benjamin Poulain 2012-09-20 16:52:21 PDT
For info, this is not how you are supposed to test Geolocation.

You are supposed to go implement the provider in the UIProcess, not punch through the layers.