Bug 54334 - [QT] Implement mock client-based geolocation for layout testing
Summary: [QT] Implement mock client-based geolocation for layout testing
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Mahesh Kulkarni
URL:
Keywords: Qt
Depends on: 42629
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-11 21:25 PST by Mahesh Kulkarni
Modified: 2014-02-03 03:17 PST (History)
12 users (show)

See Also:


Attachments
patch (15.22 KB, patch)
2011-03-02 15:52 PST, Mahesh Kulkarni
no flags Details | Formatted Diff | Diff
patch (15.79 KB, patch)
2011-03-03 12:38 PST, Mahesh Kulkarni
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mahesh Kulkarni 2011-02-11 21:25:27 PST
This is a follow up bug #42629

This bug is to track implementation of layout testing for client-based geolocation implementation for qtwebkit.
Comment 1 Mahesh Kulkarni 2011-03-02 15:52:29 PST
Created attachment 84479 [details]
patch

Enables layout testing for client-based geolocation. 

One case related to multiple windows escaped and will raise a different bug to fix this. This needs little more work in DRT.

Please review.
Comment 2 Kenneth Rohde Christiansen 2011-03-03 03:24:44 PST
Comment on attachment 84479 [details]
patch

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

> Source/WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.h:146
> +    static void mockGeolocationReset(QWebPage*);

resetGeolocationMock feels more natural to me.

> Source/WebKit/qt/Api/qwebpage.cpp:338
> +    // Incase running in DumpRenderTree mode set the controller to mock provider.

in case are two words

> Source/WebKit/qt/Api/qwebpage.cpp:340
> +    if (QWebPagePrivate::drtRun)
> +        static_cast<GeolocationClientMock*>(pageClients.geolocationClient)->setController(page->geolocationController());

You could create a 

GeolocationClientMock* toGeolocationClientMock(...)
{
     ASSERT(QWebPagePrivate::drtRun);
     return static...
}

if you ever need to use this in more places.

> Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp:425
> +    // set running in DRT mode for qwebpage to create testable objects

In general we try to write proper sentences, ie. start with capital and end with a punctuation mark of some kind.
Comment 3 Mahesh Kulkarni 2011-03-03 12:38:54 PST
Created attachment 84609 [details]
patch

Updated patch as per above comments
Comment 4 Laszlo Gombos 2011-03-03 12:44:23 PST
Comment on attachment 84609 [details]
patch

cq+ for commit.
Comment 5 WebKit Commit Bot 2011-03-03 21:13:30 PST
Comment on attachment 84609 [details]
patch

Clearing flags on attachment: 84609

Committed r80319: <http://trac.webkit.org/changeset/80319>
Comment 6 Laszlo Gombos 2011-03-06 04:53:43 PST
Comment on attachment 84479 [details]
patch

Clearing r+ to take it out from the pending commit list as the patch has been landed.
Comment 7 Steve Block 2011-03-07 04:10:39 PST
Comment on attachment 84609 [details]
patch

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

> Source/WebCore/page/GeolocationController.h:60
> +    GeolocationClient* client() { return m_client; }

I'm surprised that you had to add this - it wasn't needed by other ports - can you explain why? It seems odd that a port would need to query the Page/GeolocationController to get a client that it has itself supplied to that Page.
Comment 8 Jocelyn Turcotte 2014-02-03 03:17:15 PST
=== Bulk closing of Qt bugs ===

If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it and remove [Qt] from the summary.

If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.