Closed
Bug 104038
Opened 23 years ago
Closed 23 years ago
Browser calls "Destroy" twice for some XPCOM plugins
Categories
(Core Graveyard :: Java: OJI, defect, P3)
Core Graveyard
Java: OJI
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla0.9.6
People
(Reporter: steve.katz, Assigned: peterlubczynski-bugs)
Details
(Keywords: topembed)
Attachments
(1 file, 1 obsolete file)
1.85 KB,
patch
|
serhunt
:
review+
attinasi
:
superreview+
|
Details | Diff | Splinter Review |
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•23 years ago
|
||
Works for me on Linux RedHat 6.2. Could it be solaris-specific problem?
Reporter | ||
Comment 2•23 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•23 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•23 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•23 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•23 years ago
|
||
What does this variable do? When would we (the plugin) want to set it to true?
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•23 years ago
|
||
The extra nsIPluginInstance::Destroy() is in ~nsActivePlugin(). Should we be calling this here?
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•23 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•23 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•23 years ago
|
||
Assignee | ||
Comment 13•23 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•23 years ago
|
||
Comment on attachment 55596 [details] [diff] [review] possible patch wrong patch :-/
Attachment #55596 -
Attachment is obsolete: true
Assignee | ||
Comment 15•23 years ago
|
||
Assignee | ||
Comment 16•23 years ago
|
||
*** Bug 83742 has been marked as a duplicate of this bug. ***
Comment 17•23 years ago
|
||
Comment on attachment 55597 [details] [diff] [review] possible fix r=av, I have tested with Real which also displayed the problem.
Attachment #55597 -
Flags: review+
Comment 18•23 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•23 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.
Comment 20•23 years ago
|
||
Verified on trunk build (2001-11-09-08-trunk). There's no error in Java Console window.
Status: RESOLVED → VERIFIED
Comment 21•23 years ago
|
||
brent, can you verify this (installing 1.3, 1.3.1, and 1.4b3)?
Comment 22•23 years ago
|
||
minusing for 0.9.4. Steve, are you seeing this in the 1.4 windows plugin too?
Reporter | ||
Comment 23•23 years ago
|
||
Yes. Ref: 10-10-2001 posting by me
You need to log in
before you can comment on or make changes to this bug.
Description
•