Bug 51557 – Visiting macnn.com often causes SQL spew via geolocation database

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Simon Fraser (smfr)]

Sometimes, when loading macnn.com, I get this output on the console:

ERROR: SQLite database failed to load from 
Cause - out of memory
(/OpenSource/WebCore/platform/sql/SQLiteDatabase.cpp:70 bool WebCore::SQLiteDatabase::open(const WTF::String&, bool))

This is coming out of geolocation code:

#0  WebCore::SQLiteDatabase::open (this=0x13739ad20, filename=@0x10bcc9008, forWebSQLDatabase=false) at /Volumes/InternalData/Development/webkit/OpenSource/WebCore/platform/sql/SQLiteDatabase.cpp:71
#1  0x000000010320e81c in WebCore::GeolocationPositionCache::readFromDatabaseImpl (this=0x10bcc8f10) at /Volumes/InternalData/Development/webkit/OpenSource/WebCore/page/GeolocationPositionCache.cpp:140
#2  0x000000010320ecb9 in WebCore::GeolocationPositionCache::readFromDatabase (cache=0x10bcc8f10) at /Volumes/InternalData/Development/webkit/OpenSource/WebCore/page/GeolocationPositionCache.cpp:132
#3  0x000000010320f504 in WebCore::CrossThreadTask1<WebCore::GeolocationPositionCache*, WebCore::GeolocationPositionCache*>::performTask (this=0x10bccd4c0, context=0x0) at CrossThreadTask.h:81
#4  0x000000010320ed29 in WebCore::GeolocationPositionCache::threadEntryPointImpl (this=0x10bcc8f10) at /Volumes/InternalData/Development/webkit/OpenSource/WebCore/page/GeolocationPositionCache.cpp:121
#5  0x000000010320ed57 in WebCore::GeolocationPositionCache::threadEntryPoint (object=0x10bcc8f10) at /Volumes/InternalData/Development/webkit/OpenSource/WebCore/page/GeolocationPositionCache.cpp:113
#6  0x0000000101a8d149 in WTF::threadEntryPoint (contextData=0x10bcbb0b0) at /Volumes/InternalData/Development/webkit/OpenSource/JavaScriptCore/wtf/Threading.cpp:65

filename is a null string.

So:
1. Why is getting window.geolocatiaon spawning a thread and trying to open a database?
2. Why is the database path empty?
3. Why is the SQL error so totally bogus?

[Simon Fraser (smfr)]

<rdar://problem/8800389>

[Steve Block]

Apologies. This was caused by http://trac.webkit.org/changeset/74354

I'll look into this ASAP, but please feel free to roll back my change if required.

[Steve Block]

> 1. Why is getting window.geolocatiaon spawning a thread and trying to open a database?
This thread is to handle reading from and writing to the database used to cache Geolocation positions. We start the thread as soon as the navigator.geolocation object is accessed so that it's ready when Geolocation is used.

> 2. Why is the database path empty?
This path can be set by calling GeolocationPositionCache::setDatabasePath(). It seems this is not called by the Mac port (Geolocation will work even without a cache). Prior to the refactoring in http://trac.webkit.org/changeset/74226, if the path is not set we early-out when trying to open the DB. Since that refactoring, if the path is not set we use an empty path so the DB open fails. I'll send a patch to restore the early-out behaviour to avoid the log spew.

> 3. Why is the SQL error so totally bogus?
I have no idea. I don't think this is specific to Geolocation.

> Apologies. This was caused by http://trac.webkit.org/changeset/74354
It seems this is not the cause after all.

[Steve Block]

Created attachment 77634 [details]
Patch

[Sam Weinig]

Comment on attachment 77634 [details]
Patch

Why are we hitting the cache when just fetching the geolocation object.  This seems silly since people often fetch the geolocation object in feature detection, even if they are not using it.

r=me.

[Steve Block]

> Why are we hitting the cache when just fetching the geolocation object.
The idea is to start the DB thread as soon as navigator.geolocation is accessed. Then when a Geolocation method is called, we potentially use a cached position if one is available, but we don't wait for the cache. If we don't start the DB thread until a Geolocation method is called, the cache will almost certainly never be used for this first method call.

> This 
> seems silly since people often fetch the geolocation object in feature
> detection, even if they are not using it.
Really, do you think it's likely that people feature detect for a feature they don't plan to use?

An alternative is to change the logic such that the DB thread is not started until a Geolocation method is called, but then have the Geolocation object wait for a cache response before instructing the GeolocationController to do work to get a position fix. This is a more involved change which could complicate the logic considerably. I'll look into it.

[WebKit Commit Bot]

Comment on attachment 77634 [details]
Patch

Clearing flags on attachment: 77634

Committed r74794: <http://trac.webkit.org/changeset/74794>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Fraser (smfr)]

(In reply to comment #3)
> > 1. Why is getting window.geolocatiaon spawning a thread and trying to open a database?
> This thread is to handle reading from and writing to the database used to cache Geolocation positions. We start the thread as soon as the navigator.geolocation object is accessed so that it's ready when Geolocation is used.

I agree with Sam that this should not happen. Please file a bug on this too.

> > 2. Why is the database path empty?
> This path can be set by calling GeolocationPositionCache::setDatabasePath(). It seems this is not called by the Mac port 

Please file a bug on that.

[Steve Block]

> I agree with Sam that this should not happen. Please file a bug on this too.
Filed Bug 51814

> > This path can be set by calling GeolocationPositionCache::setDatabasePath().
> > It seems this is not called by the Mac port 
> Please file a bug on that.
Filed Bug 51813.