714921 - IndexedDB: deleteDatabase is broken

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect)

Description

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

deleteDatabase is broken because we don't remove the subdir and then we can never open the db again.

Comment 1

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

Comment on attachment 585512 [details] [diff] [review]
Patch

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

r=me if you address these:

::: dom/indexedDB/OpenDatabaseHelper.cpp
@@ +2371,5 @@
>        NS_WARNING("Failed to delete db file!");
>        return NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
>      }
> +
> +    dbFile->Remove(true);

error check

@@ +2401,5 @@
>        NS_WARNING("Failed to delete file directory!");
>        return NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
>      }
> +
> +    fileManagerDirectory->Remove(true);

here too.

Comment 2

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

Could you also add a test for this. Specifically the following two things should be tested:

* Delete a database which doesn't exist and then open it and make sure that opening succeeds.
* Create a new database with an objectstore. Close it. Delete it. Open database and make sure that opening succeeds.

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Jonas Sicking (:sicking) from comment #2)
> Could you also add a test for this. Specifically the following two things
> should be tested:
> 
> * Delete a database which doesn't exist and then open it and make sure that
> opening succeeds.

Done.

> * Create a new database with an objectstore. Close it. Delete it. Open
> database and make sure that opening succeeds.

This is tested by the test in this patch.

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

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

Comment 6

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 585512 [details] [diff] [review]
Patch

[Approval Request Comment]
Regression caused by (bug #): Bug 661877
User impact if declined: deleteDatabase will be broken, web apps may break
Testing completed (on m-c, etc.): Automated testing
Risk to taking this patch (and alternatives if risky): Limited.  Backing out Bug 661877 is far more risky.

Comment 7

[Alex Keybl [:akeybl]]

Comment on attachment 585512 [details] [diff] [review]
Patch

[Triage Comment]
Limited risk, 11 regression, it's had a day on m-c, and unbreaks the web. Sounds righteous for Aurora.

Comment 8

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/673da4a818a0

Comment 9

[u279076]

Is there a way QA can manually verify this? I assume some form of comment 2?