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

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>