Last Comment Bug 704464 - IndexedDB: Quota prompt not launched when creating new database
: IndexedDB: Quota prompt not launched when creating new database
Status: RESOLVED FIXED
fixed-in-bs
:
Product: Core
Classification: Components
Component: DOM: IndexedDB (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
:
Mentors:
Depends on:
Blocks: idb
  Show dependency treegraph
 
Reported: 2011-11-22 06:08 PST by Jan Varga [:janv]
Modified: 2012-03-22 11:52 PDT (History)
5 users (show)
khuey: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Part 1: Rework quota handling to use Windows instead of Databases. (27.58 KB, patch)
2011-11-28 04:46 PST, Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
bent.mozilla: review-
Details | Diff | Splinter Review
Part 2: Make database creation hook into the quota system. (9.07 KB, patch)
2011-11-28 04:47 PST, Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
bent.mozilla: review+
Details | Diff | Splinter Review
Part 0: Add TLS wrapper so that I don't have to burn my eyes looking at NSPR calls (3.33 KB, patch)
2011-12-01 05:32 PST, Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
bent.mozilla: review-
Details | Diff | Splinter Review
Part 1: Rework quota handling to use Windows instead of Databases (27.16 KB, patch)
2011-12-01 05:33 PST, Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
bent.mozilla: review+
Details | Diff | Splinter Review

Description Jan Varga [:janv] 2011-11-22 06:08:06 PST
Say the allocated space is near the 50 MB limit, for example 49.9MB. New database takes approximately 0.7 MB. Now when a new database is created the quota exceeded callback is called. However since there's no current IDBDatabase, the operation just silently fails w/o launching the quota check prompt.

This bug blocks the files in idb bug, since we now hit the 50MB limit (some new tests were added recently).
Comment 1 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-11-28 04:46:46 PST
Created attachment 577220 [details] [diff] [review]
Part 1: Rework quota handling to use Windows instead of Databases.
Comment 2 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-11-28 04:47:24 PST
Created attachment 577221 [details] [diff] [review]
Part 2: Make database creation hook into the quota system.
Comment 3 Ben Turner (not reading bugmail, use the needinfo flag!) 2011-11-30 16:16:36 PST
Comment on attachment 577220 [details] [diff] [review]
Part 1: Rework quota handling to use Windows instead of Databases.

Let's stick with TLS and not use a lock... The lock you have is going to be very popular, and we really only need to lock in the unlikely case that we hit our quota limit.
Comment 4 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-12-01 05:32:46 PST
Created attachment 578233 [details] [diff] [review]
Part 0: Add TLS wrapper so that I don't have to burn my eyes looking at NSPR calls
Comment 5 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-12-01 05:33:39 PST
Created attachment 578235 [details] [diff] [review]
Part 1: Rework quota handling to use Windows instead of Databases

Now with less locking.
Comment 6 Ben Turner (not reading bugmail, use the needinfo flag!) 2011-12-02 12:19:34 PST
Comment on attachment 578235 [details] [diff] [review]
Part 1: Rework quota handling to use Windows instead of Databases

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

This looks good!

::: dom/indexedDB/IDBFactory.cpp
@@ +402,5 @@
>  
>    nsIScriptContext* context = sgo->GetContext();
>    NS_ENSURE_TRUE(context, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
>  
> +  nsCAutoString origin;

nsCString

::: dom/indexedDB/IndexedDatabaseManager.cpp
@@ +137,5 @@
>  
>  } // anonymous namespace
>  
>  IndexedDatabaseManager::IndexedDatabaseManager()
> +  : mQuotaHelperMutex("IndexedDatabaseManager.mQuotaHelperMutex")

Nit: remove indent

@@ +644,5 @@
>  }
>  
>  // static
> +bool
> +IndexedDatabaseManager::QuotaIsLifted()

Can you make a static inline in the header that calls Get() and then make this a regular method? That will avoid the mgr-> everywhere.

@@ +646,5 @@
>  // static
> +bool
> +IndexedDatabaseManager::QuotaIsLifted()
> +{
> +  IndexedDatabaseManager *mgr = IndexedDatabaseManager::Get();

Nit: * on the left

@@ +697,5 @@
> +}
> +
> +// static
> +void
> +IndexedDatabaseManager::CancelPromptsForWindow(nsPIDOMWindow* aWindow)

Static inline thing again.

@@ +701,5 @@
> +IndexedDatabaseManager::CancelPromptsForWindow(nsPIDOMWindow* aWindow)
> +{
> +  NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
> +
> +  IndexedDatabaseManager *mgr = IndexedDatabaseManager::Get();

* again.
Comment 7 Ben Turner (not reading bugmail, use the needinfo flag!) 2011-12-02 12:20:12 PST
Comment on attachment 578233 [details] [diff] [review]
Part 0: Add TLS wrapper so that I don't have to burn my eyes looking at NSPR calls

Let's skip this like we discussed ;)
Comment 8 Ben Turner (not reading bugmail, use the needinfo flag!) 2011-12-02 12:22:52 PST
Comment on attachment 577221 [details] [diff] [review]
Part 2: Make database creation hook into the quota system.

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

::: dom/indexedDB/IndexedDatabaseManager.h
@@ +318,5 @@
>    // thread.
>    nsTArray<nsCString> mTrackedQuotaPaths;
>  };
>  
> +class AutoEnterWindow {

Nit: { on next line

::: dom/indexedDB/OpenDatabaseHelper.cpp
@@ +1091,5 @@
>    }
>  
> +  NS_ASSERTION(mOpenDBRequest, "This should never be null!");
> +
> +  // XXXkhuey after gabor's stuff this assertion will be bogus.

Fix this comment (and the one in the other patch) to not be XXX and to simply say that this won't be true once we allow indexeddb outside of windows.
Comment 9 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-12-03 09:19:54 PST
https://hg.mozilla.org/projects/build-system/rev/ffdf85abb789
https://hg.mozilla.org/projects/build-system/rev/3838d56a1eaf
Comment 10 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-12-03 13:32:23 PST
https://hg.mozilla.org/mozilla-central/rev/ffdf85abb789
https://hg.mozilla.org/mozilla-central/rev/3838d56a1eaf

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