[Webkit-unassigned] [Bug 46895] [Chromium] Implement mocks for client-based geolocation

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=46895





--- Comment #7 from Steve Block <steveblock at google.com>  2010-11-25 06:59:19 PST ---
(From update of attachment 74850)
View in context: https://bugs.webkit.org/attachment.cgi?id=74850&action=review

> WebKit/chromium/public/WebGeolocationClientMock.h:51
> +    WEBKIT_API void setMockGeolocationPosition(double latitude, double longitude, double accuracy);

Do you need 'MockGeolocation' in these method names?

> WebKit/chromium/src/WebGeolocationClientMock.cpp:53
> +void WebGeolocationClientMock::initialize()

Why do you need these initialize() and reset() methods? Doesn't the OwnPtr take care of deleting the WebCore mock when this object goes away?

> WebKit/chromium/src/WebGeolocationClientMock.cpp:72
> +    WebGeolocationError::Error code = WebGeolocationError::ErrorPositionUnavailable;

Is there a particular reason for defaulting to this code?

> WebKit/chromium/src/WebGeolocationClientMock.cpp:80
> +    }

what about TIMEOUT?

> WebKit/chromium/src/WebGeolocationClientMock.cpp:118
> +    m_clientMock->setController(controller.controller());

Is this the standard Chromium syntax for getting the WebCore type?

> WebKit/chromium/src/WebGeolocationClientMock.cpp:127
> +    webPosition = PassRefPtr<WebCore::GeolocationPosition>(position);

You shouldn't instantiate locals of type PassRefPtr. Create a local RefPtr and call release() or get() depending on whether you're passing ownership or passing a raw pointer.

> WebKit/chromium/src/WebGeolocationClientMock.cpp:133
> +    m_clientMock->requestPermission(PassRefPtr<WebCore::Geolocation>(request).get());

Again, this shouldn't be a PassRefPtr.

> WebKit/chromium/src/WebGeolocationServiceMock.cpp:-47
> -// move to another class and remove WebGeolocationService*.

Were these methods being called from anywhere?

> WebKitTools/DumpRenderTree/chromium/WebViewHost.cpp:586
> +    if (!m_webGeolocationClientMock.get())

Don't need .get() for boolean test

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.