Closed Bug 729162 Opened 8 years ago Closed 7 years ago

Add per-docshell observer list for private browsing transitions

Categories

(Core :: Document Navigation, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: jdm, Assigned: jdm)

References

Details

Attachments

(1 file, 2 obsolete files)

To ensure that a DOMStorage clears its cache if its owning window switches privacy modes, we need some kind of immediate notification.
Attachment #599226 - Flags: review?(bzbarsky)
Comment on attachment 599226 [details] [diff] [review]
Add per-docshell privacy mode transition observers.

Why is this going on nsILoadContext and not on the docshell?
(And also, please use an observer array, so you don't have to make assumptions about the removal behavior, if any.)
Only because it seemed logical to group it with the attribute.(In reply to Boris Zbarsky (:bz) from comment #2)
> Comment on attachment 599226 [details] [diff] [review]
> Add per-docshell privacy mode transition observers.
> 
> Why is this going on nsILoadContext and not on the docshell?

Only because it seemed logical to group it with the attribute.
I don't think we should do that.  If we're trying to observe on a particular window, the API should be one window or docshell, not on the load context...
Attachment #599471 - Flags: review?(bzbarsky)
Attachment #599226 - Attachment is obsolete: true
Attachment #599226 - Flags: review?(bzbarsky)
Blocks: PBnGen
Whiteboard: [needs review]
Comment on attachment 599471 [details] [diff] [review]
Add per-docshell privacy mode transition observers.

>+++ b/docshell/base/nsDocShell.h
>+    nsTObserverArray<nsIWeakReference*> mPrivacyObservers;

I'm ... a little surprised that worked.  You're getting nsIWeakReference objects, putting this in the array, then dropping the only ref to them, which should by rights explode.

Please make this member nsTObserverArray<nsWeakPtr>.

>+++ b/docshell/base/nsIDocShell.idl

Rev the uuid.

>+++ b/docshell/base/nsIPrivacyTransitionObserver.idl
>\ No newline at end of file

Add one, please.

r=me with those issues fixed.  Please do NOT check in without fixing the ownership model bit!
Attachment #599471 - Flags: review?(bzbarsky) → review+
Whiteboard: [needs review] → [needs updated patch]
Blocks: 722942
Attachment #599471 - Attachment is obsolete: true
Assignee: nobody → josh
Whiteboard: [needs updated patch] → [needs landing]
https://hg.mozilla.org/projects/birch/rev/6e93febee902
Flags: in-testsuite+
Whiteboard: [needs landing]
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/6e93febee902
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.