Bug 123347 - Refactor IDB factory creation
Summary: Refactor IDB factory creation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-10-25 10:56 PDT by Brady Eidson
Modified: 2013-10-28 21:17 PDT (History)
7 users (show)

See Also:


Attachments
Patch v1 (42.90 KB, patch)
2013-10-25 12:57 PDT, Brady Eidson
kling: review+
Details | Formatted Diff | Diff
Patch for landing (42.88 KB, patch)
2013-10-28 13:29 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2013-10-25 10:56:55 PDT
Refactor how IDB databases get their disk locations

Contrary to a comment and unused static variable, IDBFactoryBackends are not global singletons.  They have a 1:1 relationship with IDBFactory.

And it doesn't make sense to have one factory support databases in different locations on disk.

So this patch moves the directory argument from IDBDatabaseBackend to IDBFactoryBackend.

It also adds the top-level security origin as an argument into database opening, as that should further differentiate the uniqueness of a database in the future.
Comment 1 Brady Eidson 2013-10-25 12:35:42 PDT
Retitling to Refactor IDB factory creation:

-Rework directory location
-Make (some) SecurityOrigin arguments be references instead of pointers
-Add two SecurityOrigin arguments to opening databases for future use
Comment 2 Brady Eidson 2013-10-25 12:57:15 PDT
Created attachment 215200 [details]
Patch v1
Comment 3 Sergio Correia (qrwteyrutiyoup) 2013-10-25 13:08:14 PDT
Comment on attachment 215200 [details]
Patch v1 

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

> Source/WebKit2/WebProcess/Databases/IndexedDB/WebProcessIDBDatabaseBackend.h:90
> +    OwnPtr<WebCore::IDBDatabaseMetadata> m_metadata;

Is this being used somewhere?
Comment 4 Brady Eidson 2013-10-25 14:13:21 PDT
(In reply to comment #3)
> (From update of attachment 215200 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=215200&action=review
> 
> > Source/WebKit2/WebProcess/Databases/IndexedDB/WebProcessIDBDatabaseBackend.h:90
> > +    OwnPtr<WebCore::IDBDatabaseMetadata> m_metadata;
> 
> Is this being used somewhere?

Probably not, leaked through from a separate patch.  Thanks for catching it.
Comment 5 Brady Eidson 2013-10-25 14:13:27 PDT
Fixed locally.
Comment 6 Andreas Kling 2013-10-28 13:15:16 PDT
Comment on attachment 215200 [details]
Patch v1 

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

r=me

> Source/WebCore/Modules/indexeddb/PageGroupIndexedDatabase.h:47
> +    PageGroupIndexedDatabase(const String& databaseDirectoryIdentifier);

explicit

> Source/WebCore/Modules/indexeddb/WorkerGlobalScopeIndexedDatabase.h:49
> +    WorkerGlobalScopeIndexedDatabase(const String& databaseDirectoryIdentifier);

explicit

> Source/WebCore/Modules/indexeddb/leveldb/IDBFactoryBackendLevelDB.h:72
> +    IDBFactoryBackendLevelDB(const String& databaseDirectory);

explicit

> Source/WebKit2/WebProcess/Databases/IndexedDB/WebIDBFactoryBackend.h:51
> +    WebIDBFactoryBackend(const String& databaseDirectoryIdentifier);

explicit

> Source/WebKit2/WebProcess/Databases/IndexedDB/WebProcessIDBDatabaseBackend.h:46
> +    static PassRefPtr<WebProcessIDBDatabaseBackend> create(WebIDBFactoryBackend* factory, const String& name) { return adoptRef(new WebProcessIDBDatabaseBackend(factory, name)); }

We should pass the WebIDBFactoryBackend by reference here, since null is not a valid input.
Comment 7 Brady Eidson 2013-10-28 13:29:37 PDT
Created attachment 215326 [details]
Patch for landing

Since it's been a few days, want to give EWS another shot at this before landing.
Comment 8 WebKit Commit Bot 2013-10-28 21:17:57 PDT
Comment on attachment 215326 [details]
Patch for landing

Clearing flags on attachment: 215326

Committed r158170: <http://trac.webkit.org/changeset/158170>
Comment 9 WebKit Commit Bot 2013-10-28 21:17:59 PDT
All reviewed patches have been landed.  Closing bug.