711727 - Followup cleanup for files in IndexedDB

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect)

Comment 1

[Jan Varga [:janv]]

> /Users/varga/Sources/Mozilla5/mozilla-central/dom/indexedDB/
> IndexedDatabaseManager.cpp
> @@ +679,5 @@
> > +  // are database files then we need to create file managers for them and also
> > +  // tell SQLite about all of them.
> > +
> > +  nsAutoTArray<nsString, 20> subdirectories;
> > +  nsAutoTArray<nsCOMPtr<nsIFile> , 20> unknownFiles;
> 
> I'd like to change these to be hashsets, but we can do that in a followup.
> 

fixed

> @@ +755,3 @@
> >      NS_ENSURE_SUCCESS(rv, rv);
> >  
> > +    rv = fileManager->InitDirectory(fileManagerDirectory, connection);
> 
> Hm, can you combine FileManager::Init and FileManager::InitDirectory and
> just call it Init?
> 
> @@ +930,5 @@
> >    
> >    if (!fileManager) {
> >      fileManager = new FileManager(aOrigin, aDatabaseName);
> >  
> > +    if (NS_FAILED(fileManager->Init())) {
> 
> This is the only call to Init() without a followup for InitDirectory(). I
> think we should change this to GetFileManager(), not GetOrCreate(), and just
> return null here. Then DOMWindowUtils can deal accordingly. Can do in a
> followup since this is only going to happen in mochitests.
> 

fixed, however GetOrCreateFileManager() is still needed in OpenDatabaseHelper::CreateDatabaseConnection()

Comment 2

[Jan Varga [:janv]]

Attachment: patch

Comment 3

[Jan Varga [:janv]]

Attachment: fix warnings on windows

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 582568 [details] [diff] [review]
patch

r=me on everything except the IndexedDatabaseManager::EnsureOriginIsInitialized changes which bent better have a look at.

Comment 5

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 582568 [details] [diff] [review]
patch

Review of attachment 582568 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/indexedDB/IndexedDatabaseManager.cpp
@@ +770,5 @@
>  
>      rv = ss->UpdateQuotaInformationForFile(file);
>      NS_ENSURE_SUCCESS(rv, rv);
> +
> +    validSubdirs.PutEntry(dbBaseFilename);

This can fail, need to check.

Comment 6

[Jan Varga [:janv]]

https://hg.mozilla.org/mozilla-central/rev/f3c943d2e763