Closed Bug 101769 Opened 23 years ago Closed 22 years ago

Stop loading plugins on startup


(Core Graveyard :: Plug-ins, defect, P2)



(Not tracked)



(Reporter: dp, Assigned: dp)



(Keywords: perf)


(1 file, 4 obsolete files)

Linux and mac load all plugins on startup. We should store the plugin info from
registry and read it from there unless the plugin has changed (filetime check)
This is a startup perf issue.
Blocks: 7251
Keywords: perf
It's also very annoying. The Macromedia Flash plugin seems to initialise the
sound driver which fails on my system because KDE's artsd is running. Flash
blocks until I kill off artsd.

Strange thing: the same plugin does work on Navigator 4.7.
I tried to fix this in the Mac code a while back, but there is a bug which
prevents a subsequent attempt to load a Mac plugin from working.
Issues with patch:
- We can optimize the two create of registry objects happening.
- What happens when we do reload plugins ? I guess we should do a sync our
  cache after reload.
- cache doesn't affect xpcom plugins (plugins detected by LoadXPCOMPlugisn()).
  I guess that is ok since they dont cause a load of plugin; Plus Patrick tells
  me that we should OBSOLETE and possibly even rip it out.
- Still need to add code to detect timestamp change on plugin dll and disable
  cache of that plugin
- Cache use filename as a string. Is that ok for the mac ?

av, if you think this is heading in the right direction, we can talk and resolve
all the above.

On reload plugins we should probably do the complete rescan of the plugin 

Another idea: could be benefitial for Windows too, why not to accept this 
approach for all platforms?
Oh Yeah! The patch does it for all platforms.
Ccing conrad for any possible embedding issues.
This should be fine for embedding apps. They will have the registry referred to
by nsIRegistry::ApplicationRegistry. If they do not pass a directory service
provider to NS_InitEmbedding, it will be the same registry as used by mozilla/NS.

As far as using strings for the paths to the cached plugins, Mac users are prone
to change the name of their drives at any time and then the hard path is then
invalid. Also, paths aren't unique on the Mac. There is a pretty easy way around
that though. You can use the PersistentDescriptor methods of nsIFile (nsFileSpec
has something like this as well). It will work XP but do something better on the
Taking over bug since I am working on this one.
Assignee: av → dp
Attachment #51087 - Attachment is obsolete: true
Ok this new patch fixes all issues above + implements versioning of the
nsplugins info in the registry and adds useful prlog stmts.

Conrad: This patch still doesn't address using the persistentfiledescriptor.
Plugins code uses a mismash of nsIFile and nsfilespec. Didnt know how to get
persistentfiledescriptor from an nsIFile. Anyone ?

r=/sr= please

Testing: I copied the into my plugins directory and tested
that it doesnt get loaded after caching, gets loaded if I touch it and saw flash
work in all cases in my browser. Did about:plugins to see the plugins info.
av: This patch is final enough that you can try it on your windows
buildconrad/bread: any help with mac ?
From nsIFileSpec:
and from nsIFile, QI to an nsILocalFile and then:

The data returned by each class is interchangeable with the other.

And, for a good essay on not using paths to specify files, ;-)
Thanks Conrad. I looked into doing the persistentDescriptor changes too. But
those changes are a little bit more pervasive (apis needs to change). I think
the right thing to do is to get this in and do that next along with conversion
to nsIFile from nsFileSpec.
Priority: -- → P2
Target Milestone: --- → mozilla0.9.5
Sounds fine. Since you're using registry APIs, you can just define a new key
that gets stored when the change to persistent descs gets made. If the new key
isn't there, we use the old key/value. I don't think it will be hard to have a
migration path, so go ahead and use paths for now.
We can even do better than that. There is versioning of the registry hierarchy
in the patch. If version numbers dont match, the registry hierarchy is thrown
and recreated.

So when we do the persistentDescriptor I would just not store "filename" anymore
and change the key to "persistentDescriptor" and bump the version number
Attachment #51228 - Attachment is obsolete: true
Storing both would allow forward/backward compatibility.
After I tested it on windows: 
- CachePluginsInfo() never happens because of premature returns in NS_WIN only code.
- Unwanted plugins need to be cached and not loaded
- Unwanted plugins cause mSyncCachedPlugins to be set everytime

New patch in the works.
Summary: Linux & Mac load plugins on startup → Stop loading plugins on startup
Attachment #51335 - Attachment is obsolete: true
This latest patch covers all issues including versioning, filestamp check,
dealing with unwanted plugins.

Works on windows and unix. Checking it on the mac.
Attachment #51574 - Attachment is obsolete: true
The mac was using mFullPath and mFileName while other platforms were using only
mFileName. This is all ugly and converting to nsIFile and using persitent
descriptor would fix it. Filed bug 102700

So finally this works on all platforms.

Ccing waterson. sr= ?
av r= ?
dp, I am looking at it. Still need some time to fully understand it.
Let me know once you have reviewed it.
Target Milestone: mozilla0.9.5 → mozilla0.9.6
Comment on attachment 51699 [details] [diff] [review]
Patch after win,unix,mac testing

nice clean up, so far no problems with the patch, 
r=av, will file issues as they arise.
Attachment #51699 - Flags: review+
Don't need to check |if (foo)| to |delete foo;|, |delete| handles null pointers:

+      }
+      else {
+        if (pluginTag)
         delete pluginTag;

Linked list removal is less painful if you maintain a link pointer (in

  nsPluginTag *tag, **link;
  for (link = &mCachedPlugins, tag = *link;
       tag != nsnull; 
       link = &tag->mNext, tag = *link) {
    if (/* found the element to remove */) {
      *link = tag->mNext;
      return tag;

Otherwise, looks good. sr=waterson
You can consolidate the iterator-pointer assignment via

  nsPluginTag *tag, **link;
  for (link = &mCachedPlugins; (tag = *link) != nsnull; link = &tag->mNext) {
    if (/* found the element to remove */) {
      *link = tag->mNext;
      return tag;

and also fit the for-loop control on one line.  Bonus!

Thanks guys. Brenda, I will stick with the multiline for loop as I have a
preference for not having assignments as sideeffects.
Thats the second time i missed the 'n' out of brendan. I swear that wasnt
intentional. Now, got to figure out who brenda is ...
Fixed check in 0.9.6
Closed: 22 years ago
Resolution: --- → FIXED
(don't remind me of Beverley Hills 90210!)

side effects are all the for loop control is about, but choose your poison.  The
redudancy isn't the only objection to assigning in the init and update parts --
it's also visually busier, and the comma operators don't help.  Parting shot
from Brenda :-)

Buil ID: 2001101510

I installed this build on Linux, I started and closed mozilla a few times so
plugins get registered. Then I started mozilla (my start page is a blank page).
And without browsing any other page, I do on command line "cat
/proc/<MozillaPid>/maps" and I still see all the plugins in memory. (,,,, and ).

Am I doing anything wrong ?
I know. My problem is obvious. I have an old build. I downloaded today a nightly
build ( mozilla-gcc295-i686-pc-linux-gnu.tar.gz ) but it seems to be rather old.
Sorry for the spam. :-(
I think the branch would like this as well.
Keywords: edt0.9.4
I have the patch for bug 77231 almost merged into 0.9.4. I think it should be
able to go into the branch itself.
No longer blocks: 77231
Keywords: edt0.9.4
Blocks: 105935
Conrad, be sure you get the patch in bug 101769 which is really part of this bug.
This patch really won't help Windows at all and in fact has caused a few
regressions so I don't think anymore it should NOT be on the embedding branch.
I agree with Peter. There is no full understanding of some of the regressions. 
See, e.g., bug 112508.
fixed a while ago. verif!
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.