Bug 59201

Summary: [WK2] WTR missing setMockGeolocationPosition and setMockGeolocationError for layout testing
Product: WebKit Reporter: Mahesh Kulkarni <maheshk>
Component: WebKit2Assignee: Mahesh Kulkarni <maheshk>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: benjamin, gustavo, kenneth, laszlo.gombos, mario, menard, sunaeluv.kim, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Bug Depends on: 88657    
Bug Blocks: 55872, 83876    
Attachments:
Description Flags
Patch proposal benjamin: review-, buildbot: commit-queue-

Description Mahesh Kulkarni 2011-04-22 07:35:22 PDT
Implement LayoutController.setMockGeolocationPosition and LayoutController.setMockGeolocationError for layout testing of geolocation on Webkit2
Comment 1 Mario Sanchez Prada 2012-04-13 07:51:29 PDT
Block also Geolocation support for WebKit2GTK+
Comment 2 Mario Sanchez Prada 2012-06-08 07:31:45 PDT
Created attachment 146566 [details]
Patch proposal

This patch depends on patch for bug 88657 (so don't expect it to pass on EWS bots)
Comment 3 Build Bot 2012-06-08 07:49:26 PDT
Comment on attachment 146566 [details]
Patch proposal

Attachment 146566 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12924318
Comment 4 Early Warning System Bot 2012-06-08 08:00:04 PDT
Comment on attachment 146566 [details]
Patch proposal

Attachment 146566 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12908701
Comment 5 Gustavo Noronha (kov) 2012-06-08 09:05:50 PDT
Comment on attachment 146566 [details]
Patch proposal

Attachment 146566 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12922515
Comment 6 Build Bot 2012-06-08 10:12:04 PDT
Comment on attachment 146566 [details]
Patch proposal

Attachment 146566 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12907980
Comment 7 Benjamin Poulain 2012-09-20 18:06:35 PDT
Comment on attachment 146566 [details]
Patch proposal

This is a terrible idea. On WebKit2, you should go all the way up and test the API as it is.
Comment 8 Mario Sanchez Prada 2012-09-21 13:22:36 PDT
(In reply to comment #7)
> (From update of attachment 146566 [details])
> This is a terrible idea. On WebKit2, you should
> go all the way up and test the API as it is.

Can you elaborate a bit more on this? I'm not sure what you mean.

Thanks for taking your time to review this patch.
Comment 9 Benjamin Poulain 2012-09-21 13:57:57 PDT
> Can you elaborate a bit more on this? I'm not sure what you mean.
> 
> Thanks for taking your time to review this patch.


Geolocation goes like this:
Geolocation->GeolocationController->GeolocationClient(WebGeolocationClient)->WebGeolocationManager->process boundaries->WebGeolocationManagerProxy->WebGeolocationProvider->clients.

The correct way to test is to plug yourself at the end of the chain, on the client interface.


I need this to work correctly soon-ish. I think I'll add what is missing, I started here: https://bugs.webkit.org/show_bug.cgi?id=97278
Comment 10 Mario Sanchez Prada 2012-09-21 14:17:22 PDT
(In reply to comment #9)
> > Can you elaborate a bit more on this? I'm not sure what you mean.
> > 
> > Thanks for taking your time to review this patch.
> 
> 
> Geolocation goes like this:
> Geolocation->GeolocationController->GeolocationClient(WebGeolocationClient)->WebGeolocationManager->process boundaries->WebGeolocationManagerProxy->WebGeolocationProvider->clients.
> 
> The correct way to test is to plug yourself at the end of the chain, on the client interface.
> 
> 
> I need this to work correctly soon-ish. I think I'll add what is missing, I started here: https://bugs.webkit.org/show_bug.cgi?id=97278

Ok. After reading the patch for bug 97278 I understand now better what you mean and I have to say I agree with it being a better and cleaner way to do it.

So, I'm now closing bug 88657, since it's pretty clear now to me that it's an invalid one.

In my defense :), have to say that this was my first patch involving the InjectedBundle I think, and it's obvious I got it wrong. Reading through your comments and your patch was clarifying.

Thanks!
Comment 11 Mario Sanchez Prada 2013-10-01 09:01:48 PDT
This has already been implemented while fixing bug 97278.

*** This bug has been marked as a duplicate of bug 97278 ***