Bug 209532 - IndexedDB: destroy UniqueIDBDatabase when it's not used
Summary: IndexedDB: destroy UniqueIDBDatabase when it's not used
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: Sihui Liu
URL:
Keywords: InRadar
Depends on: 209700
Blocks:
  Show dependency treegraph
 
Reported: 2020-03-24 22:58 PDT by Sihui Liu
Modified: 2020-03-30 12:23 PDT (History)
7 users (show)

See Also:


Attachments
Patch (5.57 KB, patch)
2020-03-24 23:22 PDT, Sihui Liu
no flags Details | Formatted Diff | Diff
Patch for landing (5.71 KB, patch)
2020-03-25 22:44 PDT, Sihui Liu
no flags Details | Formatted Diff | Diff
Patch for landing (5.28 KB, patch)
2020-03-30 11:51 PDT, Sihui Liu
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sihui Liu 2020-03-24 22:58:33 PDT
If connections of a UniqueIDBDatabase are all closed and there is no pending request on the UniqueIDBDatabase, the UniqueIDBDatabase may not be used any more.
Comment 1 Sihui Liu 2020-03-24 23:22:54 PDT
Created attachment 394473 [details]
Patch
Comment 2 Geoffrey Garen 2020-03-25 12:33:53 PDT
What's the motivation for this change?

Can we test it somehow?
Comment 3 Sihui Liu 2020-03-25 13:42:07 PDT
(In reply to Geoffrey Garen from comment #2)
> What's the motivation for this change?
> 
> Can we test it somehow?

It is actually the old behavior before IDB background thread change. We don't want to keep the UniqueIDBDatabase objects in memory if they may never be used again. Current behavior is once UniqueIDBDatabase is created it will be in the map unless IDB data is deleted.

I've tested the patch this manually. This should not cause any behavior change other than less memory in the case of opening a lot of IDBDatabase, closing them, and never using them again.
Comment 4 Geoffrey Garen 2020-03-25 16:32:37 PDT
Comment on attachment 394473 [details]
Patch

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

r=me

> Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp:1198
> +void UniqueIDBDatabase::destroyIfPossible()

Does this function delete the UniqueIDBDatabase object, or just disconnect it from the server?

I think a better name might be "tryRemoveFromServer" or "tryClose".
Comment 5 Sihui Liu 2020-03-25 17:56:18 PDT
Comment on attachment 394473 [details]
Patch

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

>> Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp:1198
>> +void UniqueIDBDatabase::destroyIfPossible()
> 
> Does this function delete the UniqueIDBDatabase object, or just disconnect it from the server?
> 
> I think a better name might be "tryRemoveFromServer" or "tryClose".

IDBServer owns UniqueIDBDatabase; this function deletes the object by removing it from the IDBServer.

Will change the name to tryCloseAndRemoveFromServer.
Comment 6 Sihui Liu 2020-03-25 22:44:55 PDT
Created attachment 394581 [details]
Patch for landing
Comment 7 EWS 2020-03-25 23:26:57 PDT
Committed r259034: <https://trac.webkit.org/changeset/259034>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 394581 [details].
Comment 8 Radar WebKit Bug Importer 2020-03-25 23:27:14 PDT
<rdar://problem/60906908>
Comment 9 WebKit Commit Bot 2020-03-28 06:06:04 PDT
Re-opened since this is blocked by bug 209700
Comment 10 Sihui Liu 2020-03-30 11:51:55 PDT
Created attachment 394943 [details]
Patch for landing
Comment 11 EWS 2020-03-30 12:23:11 PDT
Committed r259215: <https://trac.webkit.org/changeset/259215>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 394943 [details].