Correctly handle unsupported schema versions of localStorage

REOPENED
Unassigned

Status

()

Core
DOM
P3
normal
REOPENED
a year ago
8 months ago

People

(Reporter: wcpan, Unassigned)

Tracking

52 Branch
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

If a profile was opened by Nightly, the schema version will be bumped to 2, then if we open the same profile again with Firefox 52, it will crash in debug mode.

It should be handled (e.g. prompt a warning or delete the storage), instead of crash in debug mode and silently ignored in release mode.
Sounds like a dup of bug 1246615?
I guess so, maybe you can set a block flag or just mark as duplicate.
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1246615

Comment 4

a year ago
Well we shouldn't crash even in 52. We should get a warning instead:
"Unable to initialize storage, version is too high!"
and then IndexedDB/DOM cache should just error out.

Do you have stack trace for this crash. I can also try to build 52 myself and see what happens.
I was crashed at here:
https://dxr.mozilla.org/mozilla-release/rev/dbb35200d46931343fd853cbe1af7688794b0940/dom/storage/DOMStorageDBUpdater.cpp#397

I'm still building on my laptop, once it's done I'll post the whole stack.

Comment 6

a year ago
So, this is actually *Local Storage* bug, not Quota manager.
Local Storage is supposed to be downgrade compatible.
CCing Kris who implemented an upgrade for Local Storage recently.
Thanks Jan for jumping in and correcting my guess.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---

Comment 8

a year ago
Here we go, downgrade-compatible changes :)
I'm afraid that we need to patch Update() in StorageDBUpdater.cpp on all supported branches (aurora, beta, esr).

Comment 9

a year ago
and maybe even current release
(In reply to Jan Varga [:janv] from comment #8)
> Here we go, downgrade-compatible changes :)

Oh, does that mean it's a regression since 52?

> I'm afraid that we need to patch Update() in StorageDBUpdater.cpp on all
> supported branches (aurora, beta, esr).

Does that mean you suggest we need to prioritize this, that said P1? Thank you.
Flags: needinfo?(jvarga)

Comment 11

a year ago
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #10)
> (In reply to Jan Varga [:janv] from comment #8)
> > Here we go, downgrade-compatible changes :)
> 
> Oh, does that mean it's a regression since 52?

No, I think bug 1314361 caused 52 to assert. The assertion ends up as a crash in debug version.

> 
> > I'm afraid that we need to patch Update() in StorageDBUpdater.cpp on all
> > supported branches (aurora, beta, esr).
> 
> Does that mean you suggest we need to prioritize this, that said P1? Thank
> you.

It only crashes in debug version, so I don't it's P1
Flags: needinfo?(jvarga)
Priority: -- → P3
See Also: → bug 1246615
Duplicate of this bug: 1379807
You need to log in before you can comment on or make changes to this bug.