Bug 205512 - Remove unneeded MemoryIDBBackingStore::create
Summary: Remove unneeded MemoryIDBBackingStore::create
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Darin Adler
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-12-20 11:20 PST by Darin Adler
Modified: 2020-01-12 11:11 PST (History)
11 users (show)

See Also:

Attachments
Patch (5.93 KB, patch)
2019-12-20 11:22 PST, Darin Adler 		no flags Details | Formatted Diff | Diff
Patch (6.02 KB, patch)
2019-12-20 11:28 PST, Darin Adler 		youennf: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2019-12-20 11:20:00 PST 
Remove unneeded MemoryIDBBackingStore::create
Comment 1 Darin Adler 2019-12-20 11:22:36 PST Comment hidden (obsolete)
Comment 2 Darin Adler 2019-12-20 11:28:29 PST 
Created attachment 386230 [details]
Patch
Comment 3 Sihui Liu 2019-12-20 11:46:37 PST 
Looks good! Probably we can move functions in SQLiteIDBBackingStore to private too
Comment 4 Rob Buis 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?
Comment 5 Darin Adler 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!
Comment 6 Darin Adler 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.
Comment 7 youenn fablet 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.
Comment 8 Darin Adler 2020-01-12 11:10:14 PST 
Committed r254414: <https://trac.webkit.org/changeset/254414>
Comment 9 Radar WebKit Bug Importer 2020-01-12 11:11:14 PST 
<rdar://problem/58513490>