880079 - Clearing Browser Data After Writing a Large String to Local Storage & Trying to Write that String Again fails with a DOM Exceeded Quota Error

Status: RESOLVED WORKSFORME

(Core :: Storage: localStorage & sessionStorage, defect, P5)

Description

[Jason Smith [:jsmith]]

Build: 1.01 6/5/2013 Inari

STR

1. Go to http://jds2501.github.io/webapi-permissions-tests/largelocalstorage.html
2. Select write local storage
3. Reload - notice the string is present
4. Clear browser data in the browser
5. Reload - notice the string is gone
6. Select write local storage

Expected

The string should be written - we've just freed all data for this origin.

Actual

The string fails to be written with the DOM Quota Exceeded.

Comment 1

[Jason Smith [:jsmith]]

DOM Quota Exceeded Error meant to say in actual results

Comment 2

[[:fabrice] Fabrice Desré]

Ben, this happens on 1.0,1 but not current trunk, so I guess the new quota manager somehow does not suffer from the same issue.

Comment 3

[Honza Bambas (:mayhemer)]

(In reply to Fabrice Desré [:fabrice] from comment #2)
> Ben, this happens on 1.0,1 but not current trunk, so I guess the new quota
> manager somehow does not suffer from the same issue.

On trunk (mozilla-central) is the new DOM storage code.  It, at this moment, has its own specific simple quota manager and that probably doesn't suffer from this bug.  The new quota manager - the global one, not the DOM storage specific - is not used by DOM storage right now (ask Jan Varga for more details).

Comment 4

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

Sounds like this isn't actually related to the new QuotaManager that Jan added.

Comment 5

[Jan Varga [:janv]]

Yeah, I don't think this is related to the new QuotaManager.

Comment 6

[[:fabrice] Fabrice Desré]

Attachment: patch

Comment 7

[Mounir Lamouri (:mounir)]

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

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

r=me with mCachedOwner being truncated.

::: dom/src/storage/nsDOMStoragePersistentDB.cpp
@@ +694,5 @@
>  
>    MarkAllScopesDirty();
>  
> +  // Reset mCachedUsage since it will be reused in GetUsageInternal.
> +  mCachedUsage = 0;

I think you want to do:
mCachedUsage = 0;
mCachedOwner.Truncate();

Comment 8

[[:fabrice] Fabrice Desré]

Attachment: patch with comments addressed

Comment 9

[Dietrich Ayala (:dietrich)]

* what's the user impact?

* what's the risk of this change?

* are there tests that ensure this works as expected, and cannot be regressed in the future?

Comment 10

[Jason Smith [:jsmith]]

(In reply to Dietrich Ayala (:dietrich) from comment #9)
> * what's the user impact?

More developer impacting than user impacting. In the developer case, they won't be able to write the same large data again after clearing stored data in the browser that they were previously able to write before data was cleared.

> 
> * what's the risk of this change?

Fabrice is out until 6/29, so I'll redirect to Mounir on this.

> 
> * are there tests that ensure this works as expected, and cannot be
> regressed in the future?

The patch doesn't appear to have tests.

Comment 11

[Mounir Lamouri (:mounir)]

(In reply to Dietrich Ayala (:dietrich) from comment #9)
> * what's the user impact?

What Jason said.

> * what's the risk of this change?

Pretty low. It is simply invalidating some cache when we clear apps data.

> * are there tests that ensure this works as expected, and cannot be
> regressed in the future?

This bug doesn't appear in m-c but we could definitively have a mochitest. Jason, is there any chance you could write a mochitest for this bug based on the STR you wrote in comment 0?

Comment 12

[Alex Keybl [:akeybl]]

Shipped in 1.0.1, no more critical for 1.1 than for that release. 1.7mb strings seem very unlikely. Let's fix in 1.2, if it's affected.

Comment 13

[Florian Bender]

Any update on this?

Comment 14

[Jens Stutte [:jstutte]]

:janv, can you please check if this is still an issue? Seems obsolete to me due to newer versions of QuotaManager.

Comment 15

[Jan Varga [:janv]]

I'll try to reproduce with both localStorage implementations.

Comment 16

[Jan Varga [:janv]]

This is not an issue anymore with LSNG (data for given site/origin can be easily deleted in Preferences - "Site Data Manager").
Clearing data when the old localStorage is enabled is more difficult, I had to go to "Clear Recent History..." and clear all "Offline Website Data".
Anyway, the bug was originally reported for Firefox OS which had a different way to clear browser data.