Browser calls "Destroy" twice for some XPCOM plugins

VERIFIED FIXED in mozilla0.9.6

Status

Core Graveyard
Java: OJI
P3
normal
VERIFIED FIXED
17 years ago
8 years ago

People

(Reporter: Steven Katz, Assigned: Peter Lubczynski)

Tracking

({topembed})

Trunk
mozilla0.9.6
topembed

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

17 years ago
The browser appears to be making two requests to the JavaPlugin to "Destroy" a
given applet.

To reproduce:

1) Install JavaPlugin
2) Go to any applet
3) Press the back button

Note that an error will be reported in the Java Plugin Console.

I have looked at this in a debugger and it is the result of the Browser calling
our Destroy method twice.

Comment 1

17 years ago
Works for me on Linux RedHat 6.2.
Could it be solaris-specific problem?
(Reporter)

Comment 2

17 years ago
When you say that it "works for you" what do you mean?  Do you see the error
reported in the Java Console?  That is the only observable effect outside of a
debugger.

Comment 3

17 years ago
ok, let me clarify this. I do following
  1. start ./dist/bin/mozilla www.mozilla.org
  2. open Java console and set it's debug level to 5
  3. load http://java.sun.com (both applets on this page work)
  4. press Back

I see no errors in java console. Moreover only two instances of
 each type of DESTROY, DELETE or QUIT events are reported.

My plugin version is 1.3.0_01.

(Reporter)

Comment 4

17 years ago
Ah, let me clarify as well.

This problem is with the 1.4 release of the Plugin/JRE currently in beta.  This
version uses a different scheme (based on nsIModule) to connect with the
browser.  I believe this scheme causes the browser to exersise different code
paths then the previous scheme (based on NSGetFactory).
(Assignee)

Comment 5

17 years ago
I see this on Win32 1.4 JRE too. It looks like the plugin isn't setting
nsPluginInstanceVariable_DoCacheBool to true and therefore the second call to
nsIPluginInstance::Destroy(). If the plugin returns true for this variable, is
there a second call to destroy?

Andrei, this may be a bug on our side that effects all XPCOM plugins in general.
It may be helpful to see what's going on in the sample.
Assignee: edburns → joe.chou
(Reporter)

Comment 6

17 years ago
What does this variable do?  When would we (the plugin) want to set it to true?

Comment 7

17 years ago
If I remember correctly it was introduced earlier by Sun people. The thing is 
that plugins may stay in memory after the page is left. This is by default for 
XPCOM plugins, right Peter? And this may be controlled by responding when 
browser calls NPP_GetValue asking for this variable.
(Assignee)

Comment 8

17 years ago
The extra nsIPluginInstance::Destroy() is in ~nsActivePlugin(). Should we be
calling this here?

Comment 9

17 years ago
And the latter comes when we remove item from the nsActivePlugin list. This can 
happen without nsObjectFrame::Destroy so we still need instance::Destroy there. 
The same situation can also happen in nsPluginHostImpl::Destroy() when we do 
this in nsActivePluginList.stopRunning() if some conditions are met (those 
obscure variables are set) and right after this in nsActivePluginList.shut() 
which causes ~nsActivePlugin. So, looks like a bug on our side. Don't see now 
what the best way to fix it would be. Flag indicating the active instance has 
already been destroyed?
(Assignee)

Comment 10

17 years ago
Hm....I have an idea. Would it work if we moved the call to
nsIPluginInstance::Destroy() from ~nsActivePlugin to the 'else' in stopRunning()
so it looks more like the same code segment in nsObjectFrame::Destroy()? It
should then be skipped when stopRunning() is called because mStopped would have
been set by StopPluginInstance() which is called right after first call to
nsIPluginInstance::Destroy() in nsObjectFrame.cpp. 

Actually, the single nsIPluginInstance::Destroy() in ~nsActivePlugin may be for
plugins that desire to be cached. If the above idea worked and a plugin
requested to be cached, the code path through nsObjectFrame::Destroy() would
cause us to never call nsIPluginInstance::Destroy(). Maybe in addition to the
above idea, also check nsPluginInstanceVariable_DoCacheBool and call
nsIPluginInstance::Destroy() from ~nsActivePlugin if set?
(Assignee)

Comment 11

17 years ago
mine
Assignee: joe.chou → peterlubczynski
OS: Solaris → All
Priority: -- → P3
Hardware: Sun → All
Summary: Browser calls "Destroy" twice for Applets → Browser calls "Destroy" twice for some XPCOM plugins
Target Milestone: --- → mozilla0.9.6
(Assignee)

Comment 12

17 years ago
Created attachment 55596 [details] [diff] [review]
possible patch
(Assignee)

Comment 13

17 years ago
Here's a possible patch to fix this, but I havn't tired it yet, especially with
1.3. Basically, we only call the Destroy() in the ActivePlugin destructor if
it's a cached plugin. In all other cases, we should have already done this
either in ObjectFrame.cpp if the plugin was made through layout or in remove()
if it's not.

Please test.
Status: NEW → ASSIGNED
Keywords: patch
(Assignee)

Comment 14

17 years ago
Comment on attachment 55596 [details] [diff] [review]
possible patch

wrong patch :-/
Attachment #55596 - Attachment is obsolete: true
(Assignee)

Comment 15

17 years ago
Created attachment 55597 [details] [diff] [review]
possible fix
(Assignee)

Comment 16

17 years ago
*** Bug 83742 has been marked as a duplicate of this bug. ***

Comment 17

17 years ago
Comment on attachment 55597 [details] [diff] [review]
possible fix

r=av, I have tested with Real which also displayed the problem.

Updated

17 years ago
Attachment #55597 - Flags: review+

Comment 18

17 years ago
Comment on attachment 55597 [details] [diff] [review]
possible fix

uh, OK. Seems a little kinky, but that's plugins! sr=attinasi
Attachment #55597 - Flags: superreview+
(Assignee)

Comment 19

17 years ago
patch checked in, marking FIXED.

I double checked this in the debugger with the JRE 1.3.0, 1.3.1, and 1.4. I set
breakpoints on the Destroy() call sites and the instance was always destroyed,
only once, when the frame went away.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Keywords: edt0.9.4
Resolution: --- → FIXED

Comment 20

17 years ago
Verified on trunk build (2001-11-09-08-trunk). There's no error in Java Console 
window.
Status: RESOLVED → VERIFIED

Comment 21

17 years ago
brent, can you verify this (installing 1.3, 1.3.1, and 1.4b3)?

Comment 22

17 years ago
minusing for 0.9.4.

Steve, are you seeing this in the 1.4 windows plugin too?
Keywords: edt0.9.4 → edt0.9.4-
(Reporter)

Comment 23

17 years ago
Yes.

Ref: 10-10-2001 posting by me

Updated

17 years ago
Keywords: topembed

Updated

8 years ago
Component: Java: OJI → Java: OJI
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.