Bug 106257 – [WK2] Set IndexedDB database path in WK2

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Michael Pruett]

The IndexedDB database path should be initialized with the same directory used for web databases in WK2.

[Michael Pruett]

Created attachment 181570 [details]
Patch

[Sam Weinig]

Comment on attachment 181570 [details]
Patch

I don't understand the reason for this new manager.  It seems like overkill to set a single string.

[Michael Pruett]

Created attachment 181583 [details]
Patch

I've eliminated the IndexedDB database manager.

[Benjamin Poulain]

Comment on attachment 181583 [details]
Patch

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
> +#if ENABLE(INDEXED_DATABASE)
> +    String indexedDBDatabasePath() const { return m_indexedDBDatabasePath; }
> +#endif

The return type should be const String&.

> Source/WebKit2/WebProcess/WebProcess.h:366
> +#if ENABLE(INDEXED_DATABASE)
> +    String m_indexedDBDatabasePath;
> +#endif

This looks like the wrong place to store something like this, this is what Settings are for.

[Michael Pruett]

Created attachment 181827 [details]
Patch

I've moved the IndexedDB database directory from WebProcess to WebKeyValueStorageManager.

[Benjamin Poulain]

Comment on attachment 181827 [details]
Patch

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.

[Michael Pruett]

Created attachment 181992 [details]
Patch

I've removed an unnecessary pointer-to-reference conversion.

[Benjamin Poulain]

Comment on attachment 181992 [details]
Patch

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.

[Benjamin Poulain]

Comment on attachment 181992 [details]
Patch

No news from Sam. Let's go ahead.

[WebKit Review Bot]

Comment on attachment 181992 [details]
Patch

Clearing flags on attachment: 181992

Committed r139364: <http://trac.webkit.org/changeset/139364>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.