Last Comment Bug 714921 - IndexedDB: deleteDatabase is broken
: IndexedDB: deleteDatabase is broken
Status: RESOLVED FIXED
[qa-]
: regression
Product: Core
Classification: Components
Component: DOM: IndexedDB (show other bugs)
: unspecified
: x86_64 Windows 7
: -- normal (vote)
: ---
Assigned To: Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
:
:
Mentors:
: 714816 (view as bug list)
Depends on:
Blocks: 661877
  Show dependency treegraph
 
Reported: 2012-01-03 12:42 PST by Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
Modified: 2012-03-29 15:11 PDT (History)
5 users (show)
khuey: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
fixed
fixed


Attachments
Patch (9.62 KB, patch)
2012-01-03 12:42 PST, Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
bent.mozilla: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-01-03 12:42:53 PST
Created attachment 585512 [details] [diff] [review]
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!) 2012-01-03 13:29:06 PST
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 2012-01-03 20:09:31 PST
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 3 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-01-04 07:45:12 PST
*** Bug 714816 has been marked as a duplicate of this bug. ***
Comment 4 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-01-04 07:50:25 PST
(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 [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-01-05 06:05:58 PST
https://hg.mozilla.org/mozilla-central/rev/e6fb800eb24a
Comment 6 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-01-05 06:07:44 PST
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] 2012-01-06 12:48:58 PST
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 [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-01-11 00:37:25 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/673da4a818a0
Comment 9 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-02-09 15:55:13 PST
Is there a way QA can manually verify this? I assume some form of comment 2?

Note You need to log in before you can comment on or make changes to this bug.