Closed
Bug 722942
Opened 12 years ago
Closed 12 years ago
nsPluginHost and nsNPAPIPluginInstance use Private Browsing global state to set plugin properties
Categories
(Core Graveyard :: Plug-ins, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla15
People
(Reporter: jdm, Assigned: jdm)
References
Details
Attachments
(1 file, 1 obsolete file)
The PB service is going away. Instead, we might be able to get away with setting the property based on the closest docshell when the plugin is created.
Assignee | ||
Comment 1•12 years ago
|
||
This could probably cover nsNPAPIPlugin's use of the service for getting the value of NPNVprivateModeBool as well.
Comment 2•12 years ago
|
||
Josh, do you know how we can get our hands on a window/document/docshell/loading context in these two classes?
(In reply to Ehsan Akhgari [:ehsan] from comment #2) > Josh, do you know how we can get our hands on a > window/document/docshell/loading context in these two classes? Not off the top of my head, I'd start by using mxr to find relevant class usage in "dom/plugins/base". Ping me on irc to talk this through if you need more help, the more specific the question the better.
Assignee | ||
Comment 4•12 years ago
|
||
Ehsan, it looks like we can obtain the plugin's element (http://mxr.mozilla.org/mozilla-central/source/dom/plugins/base/nsNPAPIPlugin.cpp#1299 or http://mxr.mozilla.org/mozilla-central/source/dom/plugins/base/nsNPAPIPlugin.cpp#2149), that suggests to me we can walk up to find the document and docshell, etc. Josh, does that sounds workable?
Assignee | ||
Comment 5•12 years ago
|
||
nsPluginHost can switch to using the observers from bug 729162 and can pass the value to nsNPAPIPluginInstance.
Depends on: 729162
Assignee | ||
Comment 6•12 years ago
|
||
Actually it's not quite that easy - the plugin host can't be in charge of being the observer; each instance has to be instead. We can probably get the plugin instance owner, get the document from that, and get a docshell from that.
(In reply to Josh Matthews [:jdm] from comment #6) > Actually it's not quite that easy - the plugin host can't be in charge of > being the observer; each instance has to be instead. We can probably get the > plugin instance owner, get the document from that, and get a docshell from > that. Sounds reasonable.
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → josh
Assignee | ||
Comment 8•12 years ago
|
||
test_privatemode.xul continues to pass with these changes.
Attachment #617401 -
Flags: review?(joshmoz)
Assignee | ||
Updated•12 years ago
|
Whiteboard: [needs review]
Comment on attachment 617401 [details] [diff] [review] Obtain private browsing status from document of plugin owner, and watch private mode transitions on a per-instance basis. Review of attachment 617401 [details] [diff] [review]: ----------------------------------------------------------------- I was hoping to fully decomtaminate nsNPAPIPluginInstance some day (it's down to just nsISupports now), this is a step in the other direction. We typically use a peer xpcom object (nsPluginInstanceOwner) to handle events and notifications like this. Can you consider making the nsPluginInstanceOwner the listener instead of nsNPAPIPluginInstance? That should also remove some ugly casts from the patch. Minusing for the move to nsPluginInstanceOwner, happy to hear objections and reconsider though. (FYI, I'm sort of hoping to combine nsPluginInstanceOwner and nsNPAPIPluginInstance some day, but for now I want to try to decom nsNPAPIPluginInstance to simplify things.) ::: dom/plugins/base/nsNPAPIPluginInstance.cpp @@ +157,5 @@ > > + nsCOMPtr<nsPIDOMWindow> domWindow = GetDOMWindow(); > + NS_ENSURE_STATE(domWindow); > + nsCOMPtr<nsIDocShell> docShell = domWindow->GetDocShell(); > + docShell->AddWeakPrivacyTransitionObserver(this); Is docShell guaranteed to be non-null here?
Attachment #617401 -
Flags: review?(joshmoz) → review-
Assignee | ||
Comment 10•12 years ago
|
||
Attachment #617779 -
Flags: review?(joshmoz)
Assignee | ||
Updated•12 years ago
|
Attachment #617401 -
Attachment is obsolete: true
Attachment #617779 -
Flags: review?(joshmoz) → review+
Assignee | ||
Updated•12 years ago
|
Whiteboard: [needs review] → [needs landing]
Comment 11•12 years ago
|
||
https://hg.mozilla.org/projects/birch/rev/e18846bc6b20
Whiteboard: [needs landing]
Target Milestone: --- → mozilla15
Comment 12•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e18846bc6b20
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 13•12 years ago
|
||
Comment on attachment 617779 [details] [diff] [review] Obtain private browsing status from document of plugin owner, and watch private mode transitions on a per-instance basis. Review of attachment 617779 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/plugins/base/nsNPAPIPlugin.cpp @@ +2136,5 @@ > } > > case NPNVprivateModeBool: { > + nsCOMPtr<nsIDocument> doc = GetDocumentFromNPP(npp); > + nsCOMPtr<nsPIDOMWindow> domwindow = do_QueryInterface(doc); This will always be null.
Assignee | ||
Comment 14•12 years ago
|
||
Ah, thanks. I'll file a followup.
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•