Don't store PDF viewing prefs in the Firefox prefs file

RESOLVED FIXED in Firefox 31

Status

()

P3
normal
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: vladan, Unassigned)

Tracking

(Depends on: 1 bug)

unspecified
Firefox 31
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [pdfjs-c-integration][pdfjs-f-ux-wanted][pdfjs-f-fixed-upstream]https://github.com/mozilla/pdf.js/pull/4559)

(Reporter)

Description

6 years ago
My prefs.js file contains a ~3KB entry for "pdfjs.database". It looks like it consists of pdf viewing preferences. 

It doesn't look like the string contains any timestamps to be used to expire any of these values. Is this an infinitely growing string?

Also, we should not be using prefs as a general key-value store. It's a common practice for Mozilla devs & extension authors to do this, but large prefs file slow down startup when the file first gets read and they slow down pref flushes which are done on the main thread during shutdown & at runtime. Could pdf.js store this info asynchronusly in content-prefs.sqlite or in a separate file?
(In reply to Vladan Djeric (:vladan) from comment #0)

> It doesn't look like the string contains any timestamps to be used to expire
> any of these values. Is this an infinitely growing string?

It's limited to 4K (http://mxr.mozilla.org/mozilla-central/source/browser/extensions/pdfjs/components/PdfStreamConverter.js#35) and stores info only for about last 20 viewed files (http://mxr.mozilla.org/mozilla-central/source/browser/extensions/pdfjs/content/web/viewer.js#244).
(Reporter)

Comment 2

6 years ago
Thanks. We really should move away from using prefs for this kind of storage though
Depends on: 866238

Comment 3

6 years ago
Indexed DB is the correct API to use for this purpose, it seems.

Updated

6 years ago
Priority: -- → P3
Whiteboard: [pdfjs-c-integration][pdfjs-f-ux-wanted]
This is disposable data and there is no need to store it forever, it is LRU cache. We agree that storing it in prefs.js is bad idea, but looks like it was "forced" by bug 793282.

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #3)
> Indexed DB is the correct API to use for this purpose, it seems.

Indexed DB will not give anything suitable to our needs: quickly retrieve small chunk of data. There is nothing convenient in managing LRU cache in the database type storage -- we need to read/update all entries all the time. So the solution will be store blob of text data in one table with one record. Will that still be a better solution than prefs.js or localStorage?

Our use case is to quickly update cache data that stores ~20 small entries and discard it when user wants to clear history. I think localStorage sounds like a better solution/choice here. Does its initial read performance problem affect Firefox or only webapp that is using it?
Flags: needinfo?
See Also: → bug 793282
You really want bug 866238 (localStorage is sucky...), but that doesn't exist yet.
Flags: needinfo?
Why not a simple JSON file?
"simple JSON file" isn't really all that simple, at least not without something like bug 947694.

Updated

5 years ago
Whiteboard: [pdfjs-c-integration][pdfjs-f-ux-wanted] → [pdfjs-c-integration][pdfjs-f-ux-wanted][pdfjs-f-fixed-upstream]https://github.com/mozilla/pdf.js/pull/4559

Updated

5 years ago
Depends on: 995431
We switched to sessionStorage instead.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 31
You need to log in before you can comment on or make changes to this bug.