[Webkit-unassigned] [Bug 47586] Get CLIENT_BASED_GEOLOCATION building

[bugzilla-daemon at webkit.org]

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


Marcus Bulach <bulach at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bulach at chromium.org




--- Comment #6 from Marcus Bulach <bulach at chromium.org>  2010-10-13 08:18:30 PST ---
(In reply to comment #5)
> (From update of attachment 70610 [details])
> LGTM, but best to wait for a review from chromium.

looks fine by me as well, just two minor suggestions:

View in context: https://bugs.webkit.org/attachment.cgi?id=70610&action=review

> WebCore/WebCore.gyp/WebCore.gyp:1207
> +            ['exclude', '/GeolocationService.*$'],

I'm not too familiar with gyp, but would this exclude platform/chromium/GeolocationServiceChromium.*$ ?

you may also want to add something like:
#if ENABLE(CLIENT_BASED_GEOLOCATION)
#error "This file should not be compiled when ENABLE(CLIENT_BASED_GEOLOCATION)"
#endif  // ENABLE(CLIENT_BASED_GEOLOCATION)

to those files, so that it makes a clearer documentation (on top of removing the entry points..)

View in context: https://bugs.webkit.org/attachment.cgi?id=70610&action=review

> WebKit/chromium/src/WebGeolocationServiceMock.cpp:46
> +// FIXME: Implement mock bindings for client-based geolocation

and remove WebGeolocationServiceMockClientBasedImpl?

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