Closed Bug 722942 Opened 8 years ago Closed 8 years ago

nsPluginHost and nsNPAPIPluginInstance use Private Browsing global state to set plugin properties

Categories

(Core :: Plug-ins, defect)

x86
macOS
defect
Not set

Tracking

()

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.
This could probably cover nsNPAPIPlugin's use of the service for getting the value of NPNVprivateModeBool as well.
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.
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?
nsPluginHost can switch to using the observers from bug 729162 and can pass the value to nsNPAPIPluginInstance.
Depends on: 729162
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: nobody → josh
test_privatemode.xul continues to pass with these changes.
Attachment #617401 - Flags: review?(joshmoz)
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-
Attachment #617401 - Attachment is obsolete: true
Attachment #617779 - Flags: review?(joshmoz) → review+
Whiteboard: [needs review] → [needs landing]
https://hg.mozilla.org/projects/birch/rev/e18846bc6b20
Whiteboard: [needs landing]
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/e18846bc6b20
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
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.
Ah, thanks. I'll file a followup.
Depends on: 748752
You need to log in before you can comment on or make changes to this bug.