[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


Darin Fisher (:fishd, Google) <fishd at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #74915|review?                     |review-
               Flag|                            |




--- Comment #11 from Darin Fisher (:fishd, Google) <fishd at chromium.org>  2010-11-29 08:50:57 PST ---
(From update of attachment 74915)
View in context: https://bugs.webkit.org/attachment.cgi?id=74915&action=review

What is the master plan for ENABLE_CLIENT_BASED_GEOLOCATION?  Will we be
deleting the "else" code in the near future?

> WebKit/chromium/public/WebGeolocationClientMock.h:70
> +    WEBKIT_API void initialize();

nit: looks like 'initialize' does not need to be exported from webkit.dll since
it is only called by a private constructor.  i'd probably just get rid of the
initialize method, and move its implementation into the constructor.

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

nit: please implement the methods in the same order in which they are declared
in the header file.

> WebKitTools/DumpRenderTree/chromium/WebViewHost.h:321
> +    OwnPtr<WebKit::WebGeolocationClientMock> m_webGeolocationClientMock;

nit: for consumers of the WebKit API, it is conventional to drop the "web" prefix
on variable names.  that's why we have m_geolocationServiceMock and not
m_webGeolocationServiceMock.

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