Bug 27866 - [requires-fixup] [Qt] Initialize HTML5 Application Cache for QtWebKit to a sensible default
Summary: [requires-fixup] [Qt] Initialize HTML5 Application Cache for QtWebKit to a se...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-30 21:48 PDT by Laszlo Gombos
Modified: 2009-08-02 00:59 PDT (History)
3 users (show)

See Also:


Attachments
proposed patch (2.58 KB, patch)
2009-07-30 21:59 PDT, Laszlo Gombos
vestbo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Laszlo Gombos 2009-07-30 21:48:48 PDT
QtLauncher is crashing if a page with HTML5 App Cache is visited. This is because the app cache is designed so that WebCore::cacheStorage().setCacheDirectory() needs to be called with a path before the app cache can be used.

As far as I can tell QtWebKit does not call WebCore::cacheStorage().setCacheDirectory to set the path for the app cache neither provides a public API for QtWebKit clients to set the path; the closes I found an internal API called qt_websettings_setOfflineWebApplicationCachePath() which indeed can be used as a work-around.

As I first step I would like to propose that QtWebKit would set the app cache path to a sensible default so that QtWebKit clients does not have to deal with the issue. Later we might want to revisit how can a client set the path puth a public APi if there is a need for that.
Comment 1 Laszlo Gombos 2009-07-30 21:59:56 PDT
Created attachment 33856 [details]
proposed patch
Comment 2 Kenneth Rohde Christiansen 2009-07-31 04:50:05 PDT
We definately need to deal better with both

WebDatabase
WebStorage - Local and Session
OfflineWebApplicationCache

I actually have a patch adding support for that latter sitting around somewhere, but I stopped working on it as our API for these technologies is currently confusing, due to spec renaming.

For instance our WebDatabase is called OfflineStorageDatabase, which leads people to think that it has to do with the appcache or the webstorage. Also, the webdatabase is not limited to offline usage.

I believe that we need to deal with all this as soon as possible.
Comment 3 Kenneth Rohde Christiansen 2009-07-31 05:47:18 PDT
OK, I posted the patch in question

https://bugs.webkit.org/show_bug.cgi?id=27876
Comment 4 Kenneth Rohde Christiansen 2009-07-31 06:30:04 PDT
OK, my patch has been committed (r46628), feel free to update your patch. Notice that you can set the max size now as well.
Comment 5 Kenneth Rohde Christiansen 2009-07-31 06:32:35 PDT
When you set defaults, please update the documentation of the methods to mention these.
Comment 6 Tor Arne Vestbø 2009-07-31 06:34:47 PDT
Comment on attachment 33856 [details]
proposed patch

Looks good! A minor nitpick, just fix before landing:

> +static void WebKitInitializeApplicationCachePathIfNecessary()

Should be named initializeApplicationCachePathIfNecessary(), ie no WebKit prefix, and start with lower-case.

Actually this is not in the coding style guidelines, but I think the lower case variant is more prevalent.
Comment 7 Adam Barth 2009-08-02 00:59:09 PDT
Landed with my bare hands due to https://bugs.webkit.org/show_bug.cgi?id=27930

Sending        WebKit/qt/Api/qwebpage.cpp
Sending        WebKit/qt/ChangeLog
Transmitting file data ..
Committed revision 46691.