[Webkit-unassigned] [Bug 34084] Geolocation service does not cache position information between browser sessions

[bugzilla-daemon at webkit.org]

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


Ariya Hidayat <ariya.hidayat at gmail.com> changed:

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




--- Comment #7 from Ariya Hidayat <ariya.hidayat at gmail.com>  2010-02-11 14:59:57 PST ---
(From update of attachment 47923)
I'll add more remarks as I am trying to understand the whole Geolocation stuff,
but meanwhile..

> +        * Android.mk: Modified. Added GeolocationPositionCache.cpp
> +        * GNUmakefile.am: Modified. Added GeolocationPositionCache.[cpp|h]
> +        * WebCore.gypi: Modified. Added GeolocationPositionCache.[cpp|h]
> +        * WebCore.xcodeproj/project.pbxproj: Modified. Added GeolocationPositionCache.[cpp|h]

Even if Qt port does not fully support Geolocation yet, shouldn't you add
GeolocationPositionCache.[cpp|h] to WebCore/WebCore.pro?

> +static const char* databaseName = "/CachedPosition.db";

Is there a standard name for this? Or can we just pick a sensible one?

> +void GeolocationPositionCache::setCachedPosition(Geoposition* cachedPosition)
> +{
> +    // We do not take owenership from the caller, but add our own ref count.
> +    *s_cachedPosition = cachedPosition;
> +}

Small typo there. Also. maybe mention why the ownership is not taken (following
the usual practice of commenting why instead of what the next line does).

> +PassRefPtr<Geoposition> GeolocationPositionCache::readFromDB()
> +{
> +    SQLiteDatabase database;
> +    if (!s_databaseFile || !database.open(*s_databaseFile))
> +        return 0;

Also for other returns in this function: is it safe to return 0 here instead of
"default" position (if default makes sense at all)?

> +void GeolocationPositionCache::writeToDB(Geoposition* position)

Can't the parameter be a const?

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