1119462 - Allow unlimited quota for explicit persistent storage

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect)

Description

[Jan Varga [:janv]]

Explicit persistent storage is now protected by the first prompt. However, explicit persistent storage currently allows to store 50 MB only. We used to have a second (quota) prompt at 50 MB, but that hasn't been updated for PBackground and we just pretend that the user didn't allow to store more.

We decided to remove the second prompt, essentially allowing unlimited quota after the first prompt succeeds and also disallow explicit persistent storage on mobile for normal content. Chrome and apps on mobile will get unlimited quota too.

Since we aim to land this also on aurora, the second prompt will be removed in two steps:
1. Do the minimum to achieve the desired behavior
2. Remove all code that relates to the second prompt. This involves many other modules and the patch is around 120k big

This bug will address 1.

Comment 1

[Jan Varga [:janv]]

Attachment: patch

Comment 2

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

Comment on attachment 8546130 [details] [diff] [review]
patch

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

r=me with a better name, below.

::: dom/quota/QuotaManager.cpp
@@ +2722,5 @@
> +QuotaManager::IsStorageAllowed(PersistenceType aPersistenceType,
> +                               const nsACString& aOrigin,
> +                               bool aIsApp)
> +{
> +#if defined(MOZ_WIDGET_ANDROID) || defined(MOZ_WIDGET_GONK)

Hm, this will make Mulet behave differently than B2G... But for just this one branch I guess I don't care...

::: dom/quota/QuotaManager.h
@@ +315,5 @@
>                     bool* aIsApp,
>                     bool* aHasUnlimStoragePerm);
>  
>    static bool
> +  IsStorageAllowed(PersistenceType aPersistenceType,

This needs a better name... IsExplicitPersistenceAllowed? Something like that.

Comment 3

[Jan Varga [:janv]]

Attachment: patch v2

Comment 4

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

Comment on attachment 8546239 [details] [diff] [review]
patch v2

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

Thanks!

Comment 5

[Jan Varga [:janv]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/977aab16aad3

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out for Android build bustage:

https://hg.mozilla.org/integration/mozilla-inbound/rev/536e7f7a6d27
https://treeherder.mozilla.org/logviewer.html#?job_id=5223326&repo=mozilla-inbound

Comment 7

[Jan Varga [:janv]]

Sorry, I built only on mac.

Fixed and landed again:
https://hg.mozilla.org/integration/mozilla-inbound/rev/15830cc2b55b

Comment 8

[Nigel Babu [:nigelb]]

Backed out for B2G M5 bustage
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee220d48eab7

Comment 9

[Jan Varga [:janv]]

one more try:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1b2da9260e1f

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1b2da9260e1f

Comment 11

[Jan Varga [:janv]]

Attachment: patch v3

This is the patch which actually landed on m-c

Approval Request Comment

[Feature/regressing bug #]: This is a followup fix for bug 1083927.

[User impact if declined]: Users of explicit persistent storage won't be able to store more than 50 MB of data.

[Describe test coverage new/current, TBPL]: We have a test that checks that explicit persistent storage (which means unlimited quota after the first prompt succeeds) is not allowed on mobile.

[Risks and why]: The patch is quite simple. We intentionally made it simple for aurora. 

[String/UUID change made/needed]: No string/UUID changes.

Comment 12

[Jan Varga [:janv]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f48c43ef6a37