[webkit-reviews] review granted: [Bug 77991] [BlackBerry] Upstream DumpRenderTreeSupport : [Attachment 125859] Patch

[bugzilla-daemon at webkit.org]

Antonio Gomes <tonikitoo at webkit.org> has granted Rob Buis <rwlbuis at gmail.com>'s
request for review:
Bug 77991: [BlackBerry] Upstream DumpRenderTreeSupport
https://bugs.webkit.org/show_bug.cgi?id=77991

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

------- Additional Comments from Antonio Gomes <tonikitoo at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=125859&action=review


r+ with nits fixed.

> Source/WebKit/blackberry/WebKitSupport/DumpRenderTreeSupport.cpp:2
> + * Copyright (C) 2011 Research In Motion Limited. All rights reserved.

2012

> Source/WebKit/blackberry/WebKitSupport/DumpRenderTreeSupport.cpp:100
> +#else
> +    return -1;

unused_param(webpage) here.

> Source/WebKit/blackberry/WebKitSupport/DumpRenderTreeSupport.cpp:110
> +void DumpRenderTreeSupport::resetGeolocationMock(WebPage* webPage)
> +{
> +#if ENABLE(CLIENT_BASED_GEOLOCATION)
> +    GeolocationClientMock* mockClient =
toGeolocationClientMock(webPage->mainFrame()->page()->geolocationController()->
client());
> +    mockClient->reset();
> +#endif
> +}

ditto

> Source/WebKit/blackberry/WebKitSupport/DumpRenderTreeSupport.cpp:128
> +void DumpRenderTreeSupport::setMockGeolocationError(WebPage* webPage, int
errorCode, const String message)
> +{
> +#if ENABLE(CLIENT_BASED_GEOLOCATION)
> +    GeolocationError::ErrorCode code =
GeolocationError::PositionUnavailable;
> +    switch (errorCode) {
> +    case PositionError::PERMISSION_DENIED:
> +	   code = GeolocationError::PermissionDenied;
> +	   break;
> +    case PositionError::POSITION_UNAVAILABLE:
> +	   code = GeolocationError::PositionUnavailable;
> +	   break;
> +    }
> +
> +    GeolocationClientMock* mockClient =
static_cast<GeolocationClientMock*>(webPage->mainFrame()->page()->geolocationCo
ntroller()->client());
> +    mockClient->setError(GeolocationError::create(code, message));
> +#endif
> +}

ditto

> Source/WebKit/blackberry/WebKitSupport/DumpRenderTreeSupport.cpp:136
> +void DumpRenderTreeSupport::setMockGeolocationPermission(WebPage* webPage,
bool allowed)
> +{
> +#if ENABLE(CLIENT_BASED_GEOLOCATION)
> +    GeolocationClientMock* mockClient =
toGeolocationClientMock(webPage->mainFrame()->page()->geolocationController()->
client());
> +    mockClient->setPermission(allowed);
> +#endif
> +}

ditto, and everywhere else...

> Source/WebKit/blackberry/WebKitSupport/DumpRenderTreeSupport.h:2
> + * Copyright (C) 2011 Research In Motion Limited. All rights reserved.

2012

> Source/WebKit/blackberry/WebKitSupport/DumpRenderTreeSupport.h:52
> +    static int
numberOfPendingGeolocationPermissionRequests(BlackBerry::WebKit::WebPage*);

no need for blackberry::webkit (?)

> Source/WebKit/blackberry/WebKitSupport/DumpRenderTreeSupport.h:53
> +    static void resetGeolocationMock(BlackBerry::WebKit::WebPage*);

ditto and everyelse