Closed Bug 1119462 Opened 8 years ago Closed 8 years ago

Allow unlimited quota for explicit persistent storage


(Core :: Storage: IndexedDB, defect)

Not set



Tracking Status
firefox36 --- fixed
firefox37 --- fixed


(Reporter: janv, Assigned: janv)




(1 file, 2 obsolete files)

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.
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → Jan.Varga
Attachment #8546130 - Flags: review?(bent.mozilla)
Comment on attachment 8546130 [details] [diff] [review]

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.
Attachment #8546130 - Flags: review?(bent.mozilla) → review+
Attached patch patch v2 (obsolete) — Splinter Review
Attachment #8546130 - Attachment is obsolete: true
Attachment #8546239 - Flags: review?(bent.mozilla)
Comment on attachment 8546239 [details] [diff] [review]
patch v2

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

Attachment #8546239 - Flags: review?(bent.mozilla) → review+
Sorry, I built only on mac.

Fixed and landed again:
Flags: needinfo?(Jan.Varga)
Closed: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Blocks: 1083927
Attached patch patch v3Splinter Review
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.
Attachment #8546239 - Attachment is obsolete: true
Attachment #8547099 - Flags: review+
Attachment #8547099 - Flags: approval-mozilla-aurora?
Attachment #8547099 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.