Bug 40767

Summary: DOM storage should delete databases when they are empty.
Product: WebKit Reporter: Hans Wennborg <hans>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, commit-queue, jorlow, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch none

Description Hans Wennborg 2010-06-17 02:25:48 PDT
Even if a page clears its local storage, the database file for it stays around, and there is currently no mechanism that deletes them.

After doing the "final sync" of a storage area, the StorageAreaSyncMaster should see if the database is empty, and in that case delete it. This way, an empty local storage will never have a database file.
Comment 1 Jeremy Orlow 2010-06-17 13:19:05 PDT
Sounds like a good plan.
Comment 2 Hans Wennborg 2010-06-18 01:24:40 PDT
Created attachment 59079 [details]
Patch
Comment 3 WebKit Review Bot 2010-06-18 01:26:08 PDT
Attachment 59079 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/storage/StorageAreaSync.cpp:35:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Hans Wennborg 2010-06-18 01:37:26 PDT
(In reply to comment #3)
> Attachment 59079 [details] did not pass style-queue:
> 
> Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
> WebCore/storage/StorageAreaSync.cpp:35:  Alphabetical sorting problem.  [build/include_order] [4]
> Total errors found: 1 in 8 files
> 
> 
> If any of these errors are false positives, please file a bug against check-webkit-style.

Sorry about that. I now realise the sort should be case sensitive (and the original file must have been in error to begin with).
Comment 5 Hans Wennborg 2010-06-18 01:37:56 PDT
Created attachment 59081 [details]
Patch
Comment 6 Jeremy Orlow 2010-06-18 18:19:51 PDT
Comment on attachment 59081 [details]
Patch

Looks good to me....2 double-checking type questions tho...


WebCore/storage/StorageAreaSync.cpp:421
 +          m_database.close();
Are we super extra ultra sure that there's no way the database can be used after it's closed?  I almost wondr if we should make it an OwnPtr and null it out here so that if we do try to use it later we crash.

WebCore/storage/StorageAreaSync.cpp:403
 +      if (!m_database.isOpen())
When can this happen?  Only if it was never opened or something?
Comment 7 Hans Wennborg 2010-06-21 01:27:31 PDT
(In reply to comment #6)
> (From update of attachment 59081 [details])
> Looks good to me....2 double-checking type questions tho...
> 
> 
> WebCore/storage/StorageAreaSync.cpp:421
>  +          m_database.close();
> Are we super extra ultra sure that there's no way the database can be used after it's closed?  I almost wondr if we should make it an OwnPtr and null it out here so that if we do try to use it later we crash.
Pretty sure at least. The DeleteEmptyDatabase task is scheduled in scheduleFinalSync(), which is called by StorageAreaImpl::close() which is called by StorageNamespaceImpl::close() which is called by StorageNamespaceImpl's destructor. In effect, StorageAreaImpl should be destructed after deleteEmptyDatabases().

Anyway, the member functions in StorageAreaImpl that use m_database all check for it to be open first, so I don't really see any danger here.

> 
> WebCore/storage/StorageAreaSync.cpp:403
>  +      if (!m_database.isOpen())
> When can this happen?  Only if it was never opened or something?
Exactly. Since http://trac.webkit.org/changeset/61023, if there was no previous database file, no new database file is created (thus no db opened) unless something needs to be written to it.
Comment 8 WebKit Commit Bot 2010-06-21 07:50:44 PDT
Comment on attachment 59081 [details]
Patch

Clearing flags on attachment: 59081

Committed r61542: <http://trac.webkit.org/changeset/61542>
Comment 9 WebKit Commit Bot 2010-06-21 07:50:48 PDT
All reviewed patches have been landed.  Closing bug.