Note: There are a few cases of duplicates in user autocompletion which are being worked on.

simplify plugin instance creation and initialization

RESOLVED FIXED in mozilla14

Status

()

Core
Plug-ins
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Josh Aas, Assigned: Josh Aas)

Tracking

Trunk
mozilla14
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Created attachment 611321 [details] [diff] [review]
fix v1.0

- Better initialization method names/signatures.

- Remove unnecessary current directory initialization hack for Windows (doesn't do anything any more, were around for OJI I believe).
Attachment #611321 - Flags: review?(jst)
(Assignee)

Comment 1

5 years ago
try server run:

https://tbpl.mozilla.org/?tree=Try&rev=0d7c620a14fb
Comment on attachment 611321 [details] [diff] [review]
fix v1.0

- In nsPluginHost::TrySetUpPluginInstance():

>-#if defined(XP_WIN)
>-  static BOOL firstJavaPlugin = FALSE;
>-  BOOL restoreOrigDir = FALSE;
>-  WCHAR origDir[_MAX_PATH];
>-  if (pluginTag->mIsJavaPlugin && !firstJavaPlugin) {
>-    DWORD dw = GetCurrentDirectoryW(_MAX_PATH, origDir);
>-    NS_ASSERTION(dw <= _MAX_PATH, "Failed to obtain the current directory, which may lead to incorrect class loading");
>-    nsCOMPtr<nsIFile> binDirectory;
>-    nsresult rv = NS_GetSpecialDirectory(NS_XPCOM_CURRENT_PROCESS_DIR,
>-                                         getter_AddRefs(binDirectory));
>-
>-    if (NS_SUCCEEDED(rv)) {
>-      nsAutoString path;
>-      binDirectory->GetPath(path);
>-      restoreOrigDir = SetCurrentDirectoryW(path.get());
>-    }
>-  }
>-#endif

I suspect this code is still important in subtle ways to Java, and I think it's in our interest to leave this in place. IIRC this is important from a security point of view inside the Java plugin.

r=jst if you leave the above code, plus its counter part, in place.
Attachment #611321 - Flags: review?(jst) → review+
(Assignee)

Comment 3

5 years ago
(In reply to Johnny Stenback (:jst, jst@mozilla.com) from comment #2)

> I suspect this code is still important in subtle ways to Java, and I think
> it's in our interest to leave this in place. IIRC this is important from a
> security point of view inside the Java plugin.

The only thing that happens when we have the current directory switched is:

nsRefPtr<nsNPAPIPluginInstance> instance = new nsNPAPIPluginInstance(plugin.get());

The nsNPAPIPluginInstance constructor doesn't do anything except set some member variables to basic values - it doesn't call into the plugin at all. I'm not seeing how the current directory could matter for any of that. When I looked up the history of the code it seemed to be there for some old Java XPCOM/OJI plugin initialization that has been since removed.
Yup, you're right, this is no longer relevant, so remove away...
(Assignee)

Comment 5

5 years ago
pushed to mozilla-inbound

http://hg.mozilla.org/integration/mozilla-inbound/rev/029a32f787e9
https://hg.mozilla.org/mozilla-central/rev/029a32f787e9
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
You need to log in before you can comment on or make changes to this bug.