Open Bug 864944 Opened 11 years ago Updated 2 years ago

Make PStorage actors from children with the same principal share a parent DOMStorage object

Categories

(Core :: DOM: Core & HTML, defect, P5)

18 Branch
x86_64
Linux
defect

Tracking

()

blocking-b2g -
Tracking Status
b2g18 + ---

People

(Reporter: jdm, Unassigned)

References

Details

(Whiteboard: [MemShrink:P2] c=)

Attachments

(1 file)

Currently we create a brand new DOMStorageImpl for every StorageParent instance, causing us to re-cache everything on the first access. We should look into obtaining the relevant principal and using nsIDOMStorageManager to get a proper shared DOMStorage object in the parent instead.
Whiteboard: [MemShrink]
Version: unspecified → 18 Branch
blocking-b2g: --- → tef?
I've got code that I'm testing, but this appears harder than originally expected. We seem to be missing the code that ensures one storage object per origin.
Assignee: nobody → josh
Honza, the comment in nsIDOMStorageManager for getLocalStorageForPrincipal says "Returns instance of localStorage object for aURI's origin. This method ensures there is always only a single instance for a single origin.", which we assumed meant we could get a storage object that wouldn't require re-caching all of the keys and values on subsequent reloads. However, looking at the code, getLocalStorageForPrincipal has always created a brand new storage object since it was first introduced, and there does not appear to be any kind of origin -> storage object cache anywhere. Am I misunderstanding the purpose of the method?
Flags: needinfo?(honzab.moz)
(In reply to Josh Matthews [:jdm] from comment #2)
> Honza, the comment in nsIDOMStorageManager for getLocalStorageForPrincipal
> says "Returns instance of localStorage object for aURI's origin. This method
> ensures there is always only a single instance for a single origin.", which
> we assumed meant we could get a storage object that wouldn't require
> re-caching all of the keys and values on subsequent reloads. However,
> looking at the code, getLocalStorageForPrincipal has always created a brand
> new storage object since it was first introduced, and there does not appear
> to be any kind of origin -> storage object cache anywhere. Am I
> misunderstanding the purpose of the method?

Not sure I follow the question.  In this old code nsDOMStorage caches all its entries inside it self.  There is always new nsDOMStorage object for each windows.localStorage.  We also preload the data to an inmemory sqlite table.

This has changed in the new code - there is only one common cache for all objects.
Flags: needinfo?(honzab.moz)
(In reply to Honza Bambas (:mayhemer) from comment #4)
> In this old code nsDOMStorage caches all
> its entries inside it self.

In it's DOMStorageImpl to be exact.
If you really want to go the way of saring DOMStorageImpl, you may want to keep a hash table of them by principal code base URI.  The patch should be just used to prove the concept.
Given the amount of effort required to create a new per-origin cache and get the details of when to delete it correct, I don't think this should be the focus of our tef-blocking efforts. We should get as much bang for our buck from bug 864941 instead.
Whiteboard: [MemShrink] → [MemShrink] [tef-triage]
Leaving this nominated for now as we discuss bug 850175 and whether that will continue to block.
Whiteboard: [MemShrink] [tef-triage] → [MemShrink:P3] [tef-triage]
Whiteboard: [MemShrink:P3] [tef-triage] → [MemShrink:P2] [tef-triage]
Circling back to this, bug 850175 is taking it's course based on comment 7 this bug actually sounds like too much risk to take so late in v1.0.1 and our resources are better spent elsewhere.  Tracking and flagging for scrum backlog.
blocking-b2g: tef? → -
tracking-b2g18: --- → +
Whiteboard: [MemShrink:P2] [tef-triage] → [MemShrink:P2] c=
Assignee: josh → nobody
https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.
Priority: -- → P5
Component: DOM → DOM: Core & HTML
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: