[webkit-reviews] review denied: [Bug 79354] [EFL] Add dummy GeolocationClientEfl.cpp | h : [Attachment 133741] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied Byungwoo Lee
<bw80.lee at samsung.com>'s request for review:
Bug 79354: [EFL] Add dummy GeolocationClientEfl.cpp | h
https://bugs.webkit.org/show_bug.cgi?id=79354

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

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=133741&action=review


> Source/WebKit/efl/WebCoreSupport/GeolocationClientEfl.cpp:32
> +    ASSERT(m_view.get());

ASSERT(m_view) will do.

> Source/WebKit/efl/WebCoreSupport/GeolocationClientEfl.cpp:74
> +
> +

Extra spaces

> Source/WebKit/efl/WebCoreSupport/GeolocationClientEfl.h:45
> +    void geolocationDestroyed();
> +
> +    void startUpdating();
> +    void stopUpdating();
> +    void setEnableHighAccuracy(bool);
> +    GeolocationPosition* lastPosition();
> +
> +    void requestPermission(Geolocation*);
> +    void cancelPermissionRequest(Geolocation*);

I suggest you to explicitly mark the client methods with virtual, and to use
OVERRIDE.

> Source/WebKit/efl/WebCoreSupport/GeolocationClientEfl.h:48
> +    ~GeolocationClientEfl() { };

Why did you set the destructor as protected for a a concrete class?