Bug 106257 - [WK2] Set IndexedDB database path in WK2
Summary: [WK2] Set IndexedDB database path in WK2
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
Depends on:
Reported: 2013-01-07 14:51 PST by Michael Pruett
Modified: 2013-01-10 13:50 PST (History)
8 users (show)

See Also:

Patch (10.74 KB, patch)
2013-01-07 14:53 PST, Michael Pruett
sam: review-
Details | Formatted Diff | Diff
Patch (3.81 KB, patch)
2013-01-07 15:49 PST, Michael Pruett
benjamin: review-
Details | Formatted Diff | Diff
Patch (4.19 KB, patch)
2013-01-08 20:06 PST, Michael Pruett
benjamin: review+
benjamin: commit-queue-
Details | Formatted Diff | Diff
Patch (4.22 KB, patch)
2013-01-09 14:57 PST, Michael Pruett
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Pruett 2013-01-07 14:51:06 PST
The IndexedDB database path should be initialized with the same directory used for web databases in WK2.
Comment 1 Michael Pruett 2013-01-07 14:53:53 PST
Created attachment 181570 [details]
Comment 2 Sam Weinig 2013-01-07 15:27:34 PST
Comment on attachment 181570 [details]

I don't understand the reason for this new manager.  It seems like overkill to set a single string.
Comment 3 Michael Pruett 2013-01-07 15:49:44 PST
Created attachment 181583 [details]

I've eliminated the IndexedDB database manager.
Comment 4 Benjamin Poulain 2013-01-08 16:54:49 PST
Comment on attachment 181583 [details]

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

What this is doing is basically create a global variable m_indexedDBDatabasePath indirectly through WebProcess, and use that to initialize IndexedDBDatabasePath.

We should probably pass the path down to WebPageGroupProxy instead of making it global(?).

If you really want to make this global, please generalize WebKeyValueStorageManager instead of extending WebProcess (if I am not mistaken, IndexedDB is still a kind of key-value store).

> Source/WebKit2/WebProcess/WebProcess.h:183
> +    String indexedDBDatabasePath() const { return m_indexedDBDatabasePath; }
> +#endif

The return type should be const String&.

> Source/WebKit2/WebProcess/WebProcess.h:366
> +    String m_indexedDBDatabasePath;
> +#endif

This looks like the wrong place to store something like this, this is what Settings are for.
Comment 5 Michael Pruett 2013-01-08 20:06:55 PST
Created attachment 181827 [details]

I've moved the IndexedDB database directory from WebProcess to WebKeyValueStorageManager.
Comment 6 Benjamin Poulain 2013-01-09 14:39:33 PST
Comment on attachment 181827 [details]

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

It is not ideal but I think that looks okay to bootstrap IndexedDB. It will probably evolve separately.

> Source/WebKit2/WebProcess/WebPage/WebPageGroupProxy.cpp:71
> +    WebKeyValueStorageManager& keyValueStorageManager = *WebProcess::shared().supplement<WebKeyValueStorageManager>();
> +    m_pageGroup->groupSettings()->setIndexedDBDatabasePath(keyValueStorageManager.indexedDBDatabaseDirectory());

Please don't do the pointer-to-reference conversion. Just use the pointer directly.
Comment 7 Michael Pruett 2013-01-09 14:57:02 PST
Created attachment 181992 [details]

I've removed an unnecessary pointer-to-reference conversion.
Comment 8 Benjamin Poulain 2013-01-09 15:20:17 PST
Comment on attachment 181992 [details]

I assume you want to commit queue too? (you did not set cq?)
I set cq- for now, I'll try to get Sam have an other look.
Comment 9 Benjamin Poulain 2013-01-10 13:26:18 PST
Comment on attachment 181992 [details]

No news from Sam. Let's go ahead.
Comment 10 WebKit Review Bot 2013-01-10 13:50:51 PST
Comment on attachment 181992 [details]

Clearing flags on attachment: 181992

Committed r139364: <http://trac.webkit.org/changeset/139364>
Comment 11 WebKit Review Bot 2013-01-10 13:50:56 PST
All reviewed patches have been landed.  Closing bug.