Bug 123347

Summary: Refactor IDB factory creation
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebCore Misc.Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: alecflett, andersca, commit-queue, jsbell, kling, sergio, thorton
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch v1
kling: review+
Patch for landing none

Brady Eidson
Reported 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.
Attachments
Patch v1 (42.90 KB, patch)
2013-10-25 12:57 PDT, Brady Eidson
kling: review+
Patch for landing (42.88 KB, patch)
2013-10-28 13:29 PDT, Brady Eidson
no flags
Brady Eidson
Comment 1 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
Brady Eidson
Comment 2 2013-10-25 12:57:15 PDT
Created attachment 215200 [details] Patch v1
Sergio Correia (qrwteyrutiyoup)
Comment 3 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?
Brady Eidson
Comment 4 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.
Brady Eidson
Comment 5 2013-10-25 14:13:27 PDT
Fixed locally.
Andreas Kling
Comment 6 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.
Brady Eidson
Comment 7 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.
WebKit Commit Bot
Comment 8 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>
WebKit Commit Bot
Comment 9 2013-10-28 21:17:59 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.