[webkit-reviews] review granted: [Bug 54334] [QT] Implement mock client-based geolocation for layout testing : [Attachment 84479] patch

[bugzilla-daemon at webkit.org]

Kenneth Rohde Christiansen <kenneth at webkit.org> has granted Mahesh Kulkarni
<mahesh.kulkarni at nokia.com>'s request for review:
Bug 54334: [QT] Implement mock client-based geolocation for layout testing
https://bugs.webkit.org/show_bug.cgi?id=54334

Attachment 84479: patch
https://bugs.webkit.org/attachment.cgi?id=84479&action=review

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
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)->setControll
er(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.