474866 - Plugins not instantiated upon second visit to page on Linux

Status: VERIFIED FIXED

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

Description

[Kenneth Russell]

On Linux, upon the second visit to a web page containing a plugin such as a Java applet, the plugin is not being instantiated by the browser. To reproduce:

Get JDK 6u12 from https://jdk6.dev.java.net/ (I used the self-extracting JRE jar file), symlink lib/i386/libnpjp2.so into the Firefox plugins directory, and go to http://java.sun.com/products/plugin/1.5.0/demos/applets.html . Click the Clock example. Once the page loads and the clock is running, click the back button. Then click the Clock link again. There will be a hole in the page where the applet should be. The Java Plug-In doesn't appear to be receiving an NPP_New call.

This is a blocker for us.

Comment 1

[Johnny Stenback (:jst)]

Yeah, this is bad. Regression from bug 472439.

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

<3 plugin code.

Comment 3

[Johnny Stenback (:jst)]

Attachment: Fix.

This fixes this bug by reverting part of the change that caused this. This makes us leave the peer in the instance while a plugin is cached in the active plugins list in the plugin host, but it makes the code break the reference from the peer to the owner when a plugin is inserted into that list. This fixes the problem in this bug, and does not appear to regress the leak fix that caused this.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 358527 [details] [diff] [review]
Fix.

Looks ok if we're really completely guaranteed that cast is safe.  Might be worth using a QI here (with some sort of QI-to-CID setup like nsStandardURL uses, say).

Comment 5

[Johnny Stenback (:jst)]

We rely on that cast being safe elsewhere already. At least at http://mxr.mozilla.org/mozilla-central/source/modules/plugin/base/src/nsPluginHostImpl.cpp#3530, so we should be no less safe now. The interfaces for this stuff should probably just go away for 3.2 anyways, so I'd rather not add more to the mess unless it's needed here.

Comment 6

[Johnny Stenback (:jst)]

So this landed this morning but had to be backed out due to tinderbox showing leaks again, even if I didn't see any increase in leaks here locally. I've got a new patch coming that for me now for the first time shows no leaks when running the test content/base/test/test_bug391728.html, so I think the next patch is good to go. The new patch does what bent suggested as an option when fixing the plugin test leaks that caused this, which was to make the nsPluginInstancePeer have a weak reference to its owner.

Comment 7

[Johnny Stenback (:jst)]

Attachment: Better fix, that really doesn't leak locally.

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 358803 [details] [diff] [review]
Better fix, that really doesn't leak locally.

jst explained what's going on here over IRC to me.

Comment 9

[Johnny Stenback (:jst)]

Pushed as http://hg.mozilla.org/mozilla-central/rev/0932e7c4d976 and http://hg.mozilla.org/releases/mozilla-1.9.1/rev/bdd9bf909dd7

Comment 10

[Steve England [:stevee]]

It looks like this caused bug 475646

Comment 11

[Tony Chung [:tchung]]

Hi Murali, when you do Linux plugin testing on beta 3, can you also verify this bug is fixed at the same time?   Thanks, Tony

Comment 12

[Murali Nandigama [:murali]]

Verified as fixed

Comment 13

[Murali Nandigama [:murali]]

Verified on the branch and not on trunk