Bug 54334

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 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.

Created attachment 84609 [details] patch Updated patch as per above comments

Comment on attachment 84609 [details] patch cq+ for commit.

Comment on attachment 84609 [details] patch Clearing flags on attachment: 84609 Committed r80319: <http://trac.webkit.org/changeset/80319>

Comment on attachment 84479 [details] patch Clearing r+ to take it out from the pending commit list as the patch has been landed.

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.

=== 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.