1067885 - IndexedDB no longer honors the unlimited storage permission

Status: RESOLVED WORKSFORME

(Core :: Storage: IndexedDB, defect)

Description

[Chris Lord [:cwiiis]]

I'm currently getting these errors in the console, quite likely due to a Gecko change (but I don't know):

E/GeckoConsole( 6271): [JavaScript Error: "IndexedDB UnknownErr: ActorsParent.cpp:415"]
E/GeckoConsole( 6271): [JavaScript Error: "IndexedDB UnknownErr: ActorsParent.cpp:415"]
E/Music   ( 6983): [JavaScript Error: "UnknownError"]
E/Music   ( 6983): [JavaScript Error: "UnknownError"]
E/Music   ( 6983): Content JS ERROR at app://music.gaiamobile.org/gaia_build_defer_index.js:227 in MediaDB/openRequest.onerror: MediaDB(): UnknownError
E/Music   ( 6983): Content JS ERROR at app://music.gaiamobile.org/gaia_build_defer_index.js:388 in withStoreOnError: asyncStorage: can't open database: UnknownError

The app shows greyed out with a spinner that animates indefinitely, until I switch music tab, then it tells me I need to insert an SD card. The SD card is fine and works in other apps (such as the file manager, where I can see all my music).

While we should trace what this error is, I think the Music app needs to have more robust error handling. In this situation, it should try to rebuild the database, rather than leave me with no way to listen to music (other than trying/writing another app).

Comment 1

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

This doesn't happen for me.  Can you attach the music sqlite database from your device?

Comment 2

[Chris Lord [:cwiiis]]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #1)
> This doesn't happen for me.  Can you attach the music sqlite database from
> your device?

Here's the compressed contents of the 'idb' directory in the app's storage: https://www.dropbox.com/s/1vb3wru0ctsyest/musicdb.tar.bz2?dl=0

To be clear, reverting bug 994190 immediately fixes the issue for me (I wasn't just guessing :)).

Comment 3

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

There are two issues here:

1. We stopped honoring the unlimited storage permission.  Cwiiis's DB is too big for the default quota, so it fails to open.
2. When we fail to open a DB because it exceeds the quota we throw an unknown error instead of a quota error.

Comment 4

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

Attachment: Patch

Comment 5

[Jan Varga [:janv]]

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

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

::: dom/indexedDB/ActorsParent.cpp
@@ +10806,5 @@
>      }
> +
> +    if (mStoragePrivilege == mozilla::dom::quota::Chrome) {
> +      mEnforcingQuota = false;
> +    }

Hm, there's an assertion above:
MOZ_ASSERT(principalInfo.type() == PrincipalInfo::TContentPrincipalInfo);

So I think we should never get mozilla::dom::quota::Chrome here

@@ +10813,5 @@
> +  if (permission == PermissionRequestBase::kPermissionAllowed &&
> +      mEnforcingQuota)
> +  {
> +    // If we're running from a window then we should check the quota permission
> +    // as well.

Not sure if this comment is right.

Comment 6

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

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

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

::: dom/indexedDB/ActorsParent.cpp
@@ +39,5 @@
>  #include "mozilla/dom/indexedDB/PBackgroundIDBTransactionParent.h"
>  #include "mozilla/dom/indexedDB/PBackgroundIDBVersionChangeTransactionParent.h"
>  #include "mozilla/dom/indexedDB/PIndexedDBPermissionRequestParent.h"
>  #include "mozilla/dom/ipc/BlobParent.h"
> +#include "CheckQuotaHelper.h"

Please put this in alphabetical order!

Comment 7

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

Attachment: Patch

For checkin.

Comment 8

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

Bug 994190 was backed out, so this is technically WFM.

However, I'm going to land this on jamun and it will be merged back in once bug 994190 relands.