Followup cleanup for files in IndexedDB

RESOLVED FIXED

Status

()

Core
DOM: IndexedDB
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: janv, Assigned: janv)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

24.60 KB, patch
sicking
: review+
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
Details | Diff | Splinter Review
1.31 KB, patch
sicking
: review+
Details | Diff | Splinter Review
Comment hidden (empty)
(Assignee)

Comment 1

6 years ago
> /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()
(Assignee)

Comment 2

6 years ago
Created attachment 582568 [details] [diff] [review]
patch
Assignee: nobody → Jan.Varga
Status: NEW → ASSIGNED
Attachment #582568 - Flags: review?(bent.mozilla)
(Assignee)

Comment 3

6 years ago
Created attachment 582903 [details] [diff] [review]
fix warnings on windows
Attachment #582903 - Flags: review?(bent.mozilla)
Attachment #582903 - Flags: review?(bent.mozilla) → review+
Comment on attachment 582568 [details] [diff] [review]
patch

r=me on everything except the IndexedDatabaseManager::EnsureOriginIsInitialized changes which bent better have a look at.
Attachment #582568 - Flags: review?(bent.mozilla) → review+
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.
Attachment #582568 - Flags: review+
(Assignee)

Comment 6

6 years ago
https://hg.mozilla.org/mozilla-central/rev/f3c943d2e763
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11

Updated

6 years ago
Blocks: 187528
Component: DOM → DOM: IndexedDB
Target Milestone: mozilla11 → ---
You need to log in before you can comment on or make changes to this bug.