Closed Bug 729162 Opened 8 years ago Closed 7 years ago
Add per-docshell observer list for private browsing transitions
7.44 KB, patch
|Details | Diff | Splinter Review|
To ensure that a DOMStorage clears its cache if its owning window switches privacy modes, we need some kind of immediate notification.
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...
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]
Attachment #599471 - Attachment is obsolete: true
Assignee: nobody → josh
Whiteboard: [needs updated patch] → [needs landing]
Whiteboard: [needs landing]
Target Milestone: --- → mozilla15
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.