128366 - OJI causes plugin DLL to load at startup, scan for plugins, and load Java DLL's

Status: RESOLVED FIXED

(Core Graveyard :: Java: OJI, defect, P3)

Description

[Peter Lubczynski]

To help see when plugin scanning first happens, run with this environment variable:
NSPR_LOG_MODULES=Plugin:5

Notice that when Java is disabled in the prefs at startup, the initial plugin
scan is delayed until plugins are actually needed. In fact, the host's
constructor isn't even called. Delaying the scan from startup may be a
performance win and save on some allocations.

From briefly looking at it, it looks like the JVM Manager needs the Java
plugin's factory. Here's the stack:

nsPluginHostImpl::LoadPlugins(nsPluginHostImpl * const 0x00f201fc) li
nsPluginHostImpl::GetPluginFactory(nsPluginHostImpl * const 0x00f201
nsJVMManager::StartupJVM() line 665 + 32 bytes
nsJVMManager::MaybeStartupLiveConnect() line 901 + 20 bytes
nsJVMManager::StartupLiveConnect(nsJVMManager * const 0x00f13ae
nsJSEnvironment::Init() line 1682 + 34 bytes
NS_CreateScriptContext(nsIScriptGlobalObject * 0x00f13158, nsIScriptC
nsDOMSOFactory::NewScriptContext(nsDOMSOFactory * const 0x00f1
nsDocShell::EnsureScriptEnvironment(nsDocShell * const 0x00f1f990) li
nsWebShell::GetInterface(nsWebShell * const 0x00f1f9b8, const nsID & 
nsGetInterface::operator()(const nsID & {...}, void * * 0x0012f6c8) line 5
nsCOMPtr<nsIDOMWindow>::assign_from_helper(const nsCOMPtr_he
nsCOMPtr<nsIDOMWindow>::nsCOMPtr<nsIDOMWindow>(const nsC
nsAppShellService::RegisterTopLevelWindow(nsAppShellService * con
nsAppShellService::CreateTopLevelWindow(nsAppShellService * const 
nsWindowCreator::CreateChromeWindow(nsWindowCreator * const 0x0
nsWindowWatcher::OpenWindowJS(nsWindowWatcher * const 0x00f1c
nsWindowWatcher::OpenWindow(nsWindowWatcher * const 0x00f1ce8
nsProfile::LoadDefaultProfileDir(nsCString & {...}, int 1) line 574 + 94 b
nsProfile::StartupWithArgs(nsProfile * const 0x00f1d4c8, nsICmdLineSe
nsAppShellService::DoProfileStartup(nsAppShellService * const 0x00f1a
InitializeProfileService(nsICmdLineService * 0x00c42218) line 940 + 31
main1(int 1, char * * 0x00337ec0, nsISupports * 0x00000000) line 1242

Comment 1

[Phil Schwartau]

Is this handled in the abstract LiveConnect Java <--> JavaScript API,
which I thought was unaware of the browser embedding? 

Not sure, reassigning to Patrick as a possible LiveConnect bug;
please reassign as necessary -

Comment 2

[Peter Lubczynski]

Attachment: disabling Java Support on Startup Time and Footprint metrics

From these numbers, it looks like caching plugin info in the application
registry already does a pretty good job of improving startup time.

Additional comments from Hong:
The plugins installed were all standard (Java plugins, Shockwave Flash).  On
Windows there are a number of plugins like the Client Detection Tool and HP
Printer Identifier which appear to be installed by the trunk installer.  It did
pick up the Shockwave Flash in the Communicator folder.  On the Mac I noticed
that Netscape was picking up Quicktime which was in the Apple system folder
under "Internet plugins".

Comment 3

[Peter Lubczynski]

--->peterl Patrick is on sabatical. I looked at this over the weekend and I have
a simple patch comming.

Comment 4

[Peter Lubczynski]

Attachment: patch to remove StartupJVM() call from nsJVMManager::MaybeStartupLiveConnect()

It appears that |StartupJVM| was being called at startup due to Liveconnect
initilizing through |MaybeStartupLiveConnect|. |StartupJVM| would cause us to
load plugins, scan (stat) them, and perhaps even load Java's DLLs. According to
listdlls.exe, NPOJI610.dll, jpins32.dll, jpishare.dll, gkplugin.dll are being
loaded at startup plus we always check timestamps of files in all plugins
folders  (application, user, and global).

Since it works to install Java without restarting and also works to start with
Java disabled and later enabled it in the prefs, the browser and the code
supports calling |StartupJVM| later, whenever needed when |GetRunningJVM| is
requested. This patch removes that call from |MaybeStartupLiveConnect| and the
startup sequence and defers it to when Java is needed for the frist time. This
also has the benefit of lazily loading plugins and not scanning until the first
time actually needed in a browsing session. 

I've run through all the OJI Liveconnect tests here:
http://www.mozilla.org/quality/browser/front-end/testcases/oji/liveconnecttest.html

I have only run on Windows but they all pass like they do without the patch.

Maybe only a little bit of time and memory would be gained at startup, but
hopefully every little bit helps.

Comment 5

[Phil Schwartau]

Since this turns out to involve OJI and not abstract LiveConnect,
setting component to OJI. This answers the question in Comment #1.

Comment 6

[gabriel]

.

Comment 7

[Peter Lubczynski]

*** Bug 160374 has been marked as a duplicate of this bug. ***

Comment 8

[Xiaobin Lu]

If we remove StartupJVM call, will JSJ_ConnectToJavaVM call still succeed? I 
remember some one add that code purposly to hack around liveconnect. 

Comment 9

[rubydoo123]

moving to future

Comment 10

[Serge Gautherie (:sgautherie)]

Comment on attachment 89788 [details] [diff] [review]
patch to remove StartupJVM() call from nsJVMManager::MaybeStartupLiveConnect()


'r=?': (see comment 8)
Who can test/answer your question ?

Comment 11

[Brian Ryner (not reading)]

Profiling and tinderbox tests show that this accounts for about half of startup
time on OS X.

(In reply to comment #8)
> If we remove StartupJVM call, will JSJ_ConnectToJavaVM call still succeed? I 
> remember some one add that code purposly to hack around liveconnect. 

If a java_vm_arg is passed into JSJ_ConnectToJavaVM, it will call the
attach_current_thread callback (attach_current_thread_impl), which calls
JVM_GetJNIEnv(), which calls GetRunningJVM(), which will start the JVM if it is
enabled but not yet started.  So, I think this should be ok.  I also tested on
windows and found that all the tests pass (at least, the ones that passed
before); I'll do some testing on other platforms as well.

Comment 12

[Brian Ryner (not reading)]

Comment on attachment 89788 [details] [diff] [review]
patch to remove StartupJVM() call from nsJVMManager::MaybeStartupLiveConnect()

r=bryner.  I tested on Windows and Linux and found that liveconnect tests
passed both with and without the patch.  On Mac, Liveconnect doesn't seem to
work even without this patch, so I don't think it's going to regress anything.

Comment 13

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 89788 [details] [diff] [review]
patch to remove StartupJVM() call from nsJVMManager::MaybeStartupLiveConnect()

Awesome. sr=shaver.  (though, really, do we need to keep the #if 0 section?  we
have CVS)

Comment 14

[timeless]

before changing mac java support, could we *please* cc the mrj plugin
replacement (jep)?

Comment 15

[Steven Michaud [:smichaud] (Retired)]

Is there a Mac OS X binary which contains the changes discussed here,
that I can test against (a nightly or something similar)?

I'm the author of something called the Java Embedding Plugin, which
allows other web browsers than Safari to use Java 1.4.X on OS X.

http://javaplugin.sourceforge.net/

(By the way, the Java Embedding Plugin (via the "MRJ Plugin JEP")
currently already starts the 1.4.X JVM as the browser is being
launched.  The main reason I did this is to be able to support
LiveConnect when used outside of any applet -- as most of the Mozilla
"OJI test cases" require.)

http://www.mozilla.org/quality/browser/front-end/testcases/oji/liveconnecttest.html

Comment 16

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #14)

Thanks for CCing me!

Comment 17

[Brian Ryner (not reading)]

checked in

Comment 18

[Steven Michaud [:smichaud] (Retired)]

> checked in

I suppose this means a binary has (or will be) generated.  Where do I
look for it?

Comment 19

[Steven Michaud [:smichaud] (Retired)]

> > checked in
>
> I suppose this means a binary has been (or will be) generated.
> Where do I look for it?

Well?

I notice that the "Target Milestone" (above) is "mozilla1.8alpha6".
Does that mean that Mozilla 1.8a6, when it comes out, will contain
this patch?

Comment 20

[Serge Gautherie (:sgautherie)]

(In reply to comment #19)
> > > checked in
> >
> > I suppose this means a binary has been (or will be) generated.
> > Where do I look for it?

All nighlies since the checkin should include it.
Learn more there: <http://www.mozilla.org/developer/>.

Comment 21

[Boris Zbarsky [:bzbarsky]]

This patch probably exacerbated bug 273785 a good bit for people who have Java
enabled...

Comment 22

[Jungshik Shin]

Steven, have you tested your plugin with a recent nightly/1.8a6? 
1.8a6 (with this patch) worked/s for me and I added the release notes of 1.8a6
that your plugin needs to be installed.
(http://www.mozilla.org/releases/mozilla1.8a6/installation-extras.html#extras_java)

Comment 23

[Steven Michaud [:smichaud] (Retired)]

Yes I did.  It works fine (and it should generally now work with even
the latest Mozilla, Camino and Firefox nightlies).

I made changes in JEP 0.8.9 that specifically address the changes that
were made here.  Here's item #1 in that version's Changes.txt:

> 1. Changed back to launching the version 1.4.X Java Virtual Machine on
>    demand (whenever Java is used for the first time in a browser
>    session).
>
>    From versions 0.8.3 to 0.8.8, the Java Embedding Plugin launched
>    the 1.4.X JVM as the browser started up.  The change was made to
>    accomodate using LiveConnect outside of any applet.  I've now
>    figured out how to do this without launching the JVM at browser
>    startup (for more information see maybeCreateJavaVM and
>    loadAWTLibraries in AppletView.m).
>
>    Furthermore, future releases of the Mozilla-family browsers will no
>    longer be able to launch the JVM at startup (because they will no
>    longer load the MRJ Plugin at startup).  This change has already
>    appeared in the nightlies created since 12-02-2004 (except for
>    those corresponding to the "1.7 branch").  For more information on
>    this change see:
>
>    https://bugzilla.mozilla.org/show_bug.cgi?id=128366