Bug 205512

Summary: Remove unneeded MemoryIDBBackingStore::create
Product: WebKit Reporter: Darin Adler <darin>
Component: WebCore Misc.Assignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, alecflett, beidson, cdumez, ews-watchlist, jsbell, rbuis, sam, sihui_liu, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch youennf: review+

Darin Adler
Reported 2019-12-20 11:20:00 PST
Remove unneeded MemoryIDBBackingStore::create
Attachments
Patch (5.93 KB, patch)
2019-12-20 11:22 PST, Darin Adler 		no flags
DetailsFormatted DiffDiff
Patch (6.02 KB, patch)
2019-12-20 11:28 PST, Darin Adler 		youennf: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Darin Adler
Comment 1 2019-12-20 11:22:36 PST Comment hidden (obsolete)
Darin Adler
Comment 2 2019-12-20 11:28:29 PST
Created attachment 386230 [details] Patch
Sihui Liu
Comment 3 2019-12-20 11:46:37 PST
Looks good! Probably we can move functions in SQLiteIDBBackingStore to private too
Rob Buis
Comment 4 2019-12-23 23:03:07 PST
Comment on attachment 386230 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=386230&action=review > Source/WebCore/ChangeLog:7 > + Optionally something like "Covered by existing tests" can be added. > Source/WebCore/ChangeLog:10 > + instead of MemoryIDBBackingStore::create. I am guessing there are quite a few places in WebKit where we can do the same. > Source/WebCore/Modules/indexeddb/server/MemoryIDBBackingStore.h:45 > static std::unique_ptr<MemoryIDBBackingStore> create(PAL::SessionID, const IDBDatabaseIdentifier&); This can now be removed as well, right?
Darin Adler
Comment 5 2019-12-26 17:33:00 PST
Comment on attachment 386230 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=386230&action=review >> Source/WebCore/ChangeLog:10 >> + instead of MemoryIDBBackingStore::create. > > I am guessing there are quite a few places in WebKit where we can do the same. Some, but maybe not as many as you think. We mostly use create functions when it’s more than just a makeUnique, for example for reference counted objects, where we want to prevent the mistake of allocating on the stack. There’s no reason to use create functions in cases like this one. >> Source/WebCore/Modules/indexeddb/server/MemoryIDBBackingStore.h:45 >> static std::unique_ptr<MemoryIDBBackingStore> create(PAL::SessionID, const IDBDatabaseIdentifier&); > > This can now be removed as well, right? Yes, oops, good catch. I should have removed that!
Darin Adler
Comment 6 2019-12-29 09:10:00 PST
Anyone interested in reviewing? As you can see, this is passing all the tests on EWS. I will make changes based on Rob’s comments.
youenn fablet
Comment 7 2020-01-07 05:19:43 PST
Comment on attachment 386230 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=386230&action=review > Source/WebCore/Modules/indexeddb/server/MemoryIDBBackingStore.h:50 > IDBError getOrEstablishDatabaseInfo(IDBDatabaseInfo&) final; s/~MemoryIDBBackingStore() final;/~MemoryIDBBackingStore();/ as well now the class is final.
Darin Adler
Comment 8 2020-01-12 11:10:14 PST
Committed r254414: <https://trac.webkit.org/changeset/254414>
Radar WebKit Bug Importer
Comment 9 2020-01-12 11:11:14 PST
<rdar://problem/58513490>
Note You need to log in before you can comment on or make changes to this bug.