Closed Bug 438830 Opened 16 years ago Closed 16 years ago

Plugins can be instantiated, killed, then reinstantiated when a page loads

Categories

(Core Graveyard :: Plug-ins, defect)

Product:
Core Graveyard
Component:
Plug-ins
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: jwatt, Assigned: jst)

References

Details

(Keywords: fixed1.9.1, verified1.9.0.6, Whiteboard: [needs 445520 fixed])

Attachments

(2 files)

double-instantiate.patch
1.24 KB, patch
Details | Diff | Splinter Review
Stop double instantiation even earlier.
2.54 KB, patch
sicking
: review+
sicking
: superreview+
samuel.sidler+old
: approval1.9.0.2-
dveditz
: approval1.9.0.4-
dveditz
: approval1.9.0.6+
Details | Diff | Splinter Review 
When a page embeds content that is handled by a plugin, it is possible for the plugin to be instantiated, destroyed, then instantiated again, all during the page load. This can mean that scripted commands to the plugin during the page load can be wiped out without the script noticing.

This problem can occur if script in the page accesses the plugin before the first reflow of the embedding element. This is because we synchronously instantiate the plugin if script tries to access it through the DOM before it has been instantiated. Then, regardless of whether the plugin has been instantiated or not, when the embedding element gets its first reflow, it posts an event to instantiate the plugin to the event loop. When this event is run, the "old" plugin instance is shut down and then a new instance is reinstantiated.
To give slightly more technical detail, it's nsObjectLoadingContent::HasNewFrame that posts the nsAsyncInstantiateEvent to the event loop when the embedding element is reflowed (at least in the case when the embedding element is an <object>). HasNewFrame is currently only called from nsObjectFrame::DidReflow.
I have a patch in bug 439146 that fixes this for us, just not sure if it's the right way to fix this issue.  It checks to see if the plugin instance exists, if if it does, prevents the second instantiation which causes the unload/reload.
Once the dust settles (fx 3 release) I'd like to get some feedback on the patch.
Flags: blocking1.9.1?
Flags: blocking1.9.0.1?
Shane, can you show a stack of how this double-instantiation happens? I.e. break in your patch where you return early due to an instance already existing in the frame. Also, pelase attach that patch to this bug as well, makes it easier to talk about...
Here's a traceback from placing a call to Debugger where I was returning if a plugin instance existed.

Breakpoint 1, 0x95c7d86d in Debugger ()
(gdb) bt
#0  0x95c7d86d in Debugger ()
#1  0x09e6b54b in nsObjectLoadingContent::Instantiate ()
#2  0x09e6b79e in nsAsyncInstantiateEvent::Run ()
#3  0x001bd6da in nsThread::ProcessNextEvent ()
#4  0x0017dcb7 in NS_ProcessPendingEvents_P ()
#5  0x007bbd22 in nsBaseAppShell::NativeEventCallback ()
#6  0x0078c99a in nsAppShell::ProcessGeckoEvents ()
#7  0x930a462e in CFRunLoopRunSpecific ()
#8  0x930a4d18 in CFRunLoopRunInMode ()
#9  0x961976a0 in RunCurrentEventLoopInMode ()
#10 0x961973f2 in ReceiveNextEventCommon ()
#11 0x9619732d in BlockUntilNextEventMatchingListInMode ()
#12 0x920ae7d9 in _DPSNextEvent ()
#13 0x920ae08e in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#14 0x920a70c5 in -[NSApplication run] ()
#15 0x0078bcfa in nsAppShell::Run ()
#16 0x005c8367 in nsAppStartup::Run ()
#17 0x0004ce15 in XRE_main ()
#18 0x00005c04 in main ()

See also bug 441424 which sounds like this problem too.
Flags: wanted1.9.0.x?
Flags: blocking1.9.0.1?
Flags: blocking1.9.0.1-
Hi, I can confirm this bug and also that Shane's patch works. I actually independently made a near-identical patch and put it on bug 90268. :)

FYI, if you're a webpage developer then an easy workaround to get your page working with FF3 is to delay accessing any plugins until after your page has fully loaded. An easy way to do this is to schedule your plugin scripting code to run after all pending browser events using window.setTimeout with a timeout of zero.

Alternatively, if you are an NPAPI plugin developer and your plugin gets given a callback function by the webpage, then you get your plugin working with FF3 on broken webpages by invoking some callback that will cause the webpage to immediately access your plugin. When it accesses your plugin, that runs nsObjectLoadingContent::EnsureInstantiation, which cancels any pending nsAsyncInstantiateEvents, even when already instantiated. Hence if you invoke such a callback at just the right time then it will cancel the pending unload/reload. It's hard to get the timing right though.
This is based on Shane's earlier fix, but moves it even earlier in the process. The problem is that we end up in some cases instantiating the plugin from JS (due to JS content policy implementations or actions by the page etc), and then once the plugin frame is reflown, we don't notice that its plugin has already been instantiated so we re-instantiate (asynchronously). This patch makes us detect that the new frame already has a plugin in it, and doesn't even post the event to asynchronously instantiate the (already instantiated) plugin.

Bug 441424 has a good testcase for this problem, using an extension that contains a JS content policy implementation.

Shane, if you can test this patch as well (and Tristan too, if you have time to test yet another patch!), I'd really appreciate it.
Attachment #326964 - Flags: superreview?(jonas)
Attachment #326964 - Flags: review?(jonas)
Assignee: nobody → jst
(In reply to comment #10)
> Shane, if you can test this patch as well (and Tristan too, if you have time to
> test yet another patch!), I'd really appreciate it.
> 

It seems to work fine, I'll have a couple other people here test it as well.
Note that I have received a large number of bug reports after Firefox 3 release - those might very well be duplicates of this bug or not but I simply don't have the time to investigate all of them. Examples:

https://www.mozdev.org/bugs/show_bug.cgi?id=19414
https://www.mozdev.org/bugs/show_bug.cgi?id=19304
http://adblockplus.org/forum/viewtopic.php?t=2586
http://adblockplus.org/forum/viewtopic.php?t=2539
http://adblockplus.org/forum/viewtopic.php?t=2574
This is definitely wanted for 1.9.0.1, it's a regression and it's hurting plugin content developers especially hard (per bug 441424).
Status: NEW → ASSIGNED
Flags: wanted1.9.0.x?
Flags: wanted1.9.0.x+
Flags: blocking1.9.1?
Flags: blocking1.9.1+
Comment on attachment 326964 [details] [diff] [review]
Stop double instantiation even earlier.

Are you sure aFrame is never null when nsObjectLoadingContent::HasNewFrame is called?

r/sr=me if so
Attachment #326964 - Flags: superreview?(jonas)
Attachment #326964 - Flags: superreview+
Attachment #326964 - Flags: review?(jonas)
Attachment #326964 - Flags: review+
(In reply to comment #15)
> (From update of attachment 326964 [details] [diff] [review])
> Are you sure aFrame is never null when nsObjectLoadingContent::HasNewFrame is
> called?

Yes, the only caller:

http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsObjectFrame.cpp#1060

always passes in |this|, and passing no frame to HasNewFrame() makes no sense to me.
I just tried the patch, and it looks good to me -- it does fix the problems I was seeing.
Awesome, thanks! This was just checked in, closing bug.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment on attachment 326964 [details] [diff] [review]
Stop double instantiation even earlier.

I've heard several times now in bugzilla and/or private email that this is causing lots of trouble in many plugins. I think it's worth taking on the branch.
Attachment #326964 - Flags: approval1.9.0.1?
another potential dup in bug 442479
(In reply to comment #18)
> Awesome, thanks! This was just checked in, closing bug.
> 

Where was this checked in?  I don't see it in CVS trunk.  Can we get it for 1.9.0.*?
(In reply to comment #22)
> Where was this checked in?  I don't see it in CVS trunk.  Can we get it for
> 1.9.0.*?

It was checked in on mozilla-central. There's an approval request for the patch to land in 1.9.0.1.
The 20080628 Minefield build appeared to have the patch-- at least, the double instantiation behavior had stopped.  The build still leaks instances of plugIn interface objects.  

I'll update bug 434547 re the leaks as they don't really seem to fit in this bug...
Attachment #326964 - Flags: approval1.9.0.1? → approval1.9.0.2?
Did this ever get checked in on 1.9.0.1?
(In reply to comment #25)
> Did this ever get checked in on 1.9.0.1?

No. It got checked into mozilla-central for 1.9.1 and has an approval requested (approval1.9.0.2?) for landing on the 1.9 branch.
Anyway we can get a test for this? Maybe from the work in bug 441424?
Flags: in-testsuite?
> Anyway we can get a test for this? Maybe from the work in bug 441424?

Well the test case there involves an extension (greasemonkey) or some way to access nsIContentPolicy and then setting a breakpoint in the debugger, a breakpoint that would bit rot as the code changes.

This bug also caused the regression bug 445520. So perhaps we should wait to see how that is resolved.
Just wanted to ping this.  At last report (7/19/08), this bug has an approval requested for 1.9.0.2.

I am not really familiar with Mozilla version numbers -- should I think of "1.9.0.2" as roughly corresponding to Firefox 3.0.2?

Also, I take it the approval for 1.9.0.2 is still pending?  I guess basically my point is, I'd like to keep an eye on this bug to make sure it makes it into Firefox 3.0.2, but I don't really understand how I can do that -- e.g. when and where does this approval request get considered.  Thanks!
Mike, this will get approved when we have a resolution to bug 445520, which appears to be a regression from this bug.
Both is correct - Gecko 1.9.0.2 corresponds with Firefox 3.0.2, and this patch is still waiting for approval to land before Firefox 3.0.2 release :-(
Whiteboard: [needs 445520 fixed]
Please see Bug 445599 (apparently a dup of this bug) for another test case around *this* bug.  I also have a working demo of the bug here: http://homepages.rootsweb.ancestry.com/~reburke/work/Firefox3Bug/ff3buginfo.html

Note that in bug 445520 (related to this bug), swfobject 1.5 is mentioned.  I found that upgrading swfobject to 2.1 fixes the problem for me described in *this* bug report.  The basic difference is that swfobject 2.1 embeds flash with an embed tag, whereas swfobject 2.1 embeds flash with an object tag (with nested param tags).  In FF 3.0.1, if you embed flash with an embed tag, and you have certain add-ons such as Greasemonkey, the flash runtime initializes twice due to plugin being initialized twice (see my NSPR logging in Bug 445599).

Anyhow, I think the clue here may be the way in which FF 3.0.1 handles the embed tag, as opposed to how FF 2 handled it.
Correction to previous post:

swfobject 1.5 embeds flash with an embed tag

swfobject 2.1 embeds flash with an object tag
Comment on attachment 326964 [details] [diff] [review]
Stop double instantiation even earlier.

Approved for 1.9.0.2. Please land in CVS. a=ss
Attachment #326964 - Flags: approval1.9.0.2? → approval1.9.0.2+
Fix landed in CVS.
Keywords: fixed1.9.0.2
Backed out on CVS HEAD.
Keywords: fixed1.9.0.2
Comment on attachment 326964 [details] [diff] [review]
Stop double instantiation even earlier.

Pushing this out to 1.9.0.3.
Attachment #326964 - Flags: approval1.9.0.3?
Attachment #326964 - Flags: approval1.9.0.2-
Attachment #326964 - Flags: approval1.9.0.2+
Shouldn't this be reopened then if it was backed out?
It wasn't backed out on trunk - only on 1.9 branch. So technically this is still FIXED.
:(
Flags: blocking1.9.0.3?
Since this will not be solved until "earliest" 1.9.0.3, we need to update our customers about the progress. 

1. Why is the solution backed out? 
2. What is the time schedule for 1.9.0.3? Q3/Q4-2008 or Q1 2009, any prediction would be nice. 

Currently our solution is to downgrade our end-users to FireFox 2.0, but have no idea how long this will be accepted.
(In reply to comment #41)
> 1. Why is the solution backed out? 

Bug 445520

> 2. What is the time schedule for 1.9.0.3? Q3/Q4-2008 or Q1 2009, any
> prediction would be nice.

Minor releases appear roughly once a month - meaning October.
Sorry, bug 450949 is the one blocking that right now.
Can anyone confirm if this is the same issue as Bug 451816, that bug is still in the unconfirmed stage, it doesn't seem to be related to adblock extensions or anything like that.

This is causing endless amounts of problems for me.
This is unrelated to the problem in bug 451816.
Was this one included in 3.0.2 or will it be shipped in 3.0.3?
(In reply to comment #46)
> Was this one included in 3.0.2 or will it be shipped in 3.0.3?

I reproduced it last night during browsing (on 3.0.3), so I would say no ... 

Would be nice if it was fixed in 3.0.4
(In reply to comment #47)
> (In reply to comment #46)
> > Was this one included in 3.0.2 or will it be shipped in 3.0.3?
> I reproduced it last night during browsing (on 3.0.3), so I would say no ... 
> Would be nice if it was fixed in 3.0.4

The flag for the patch has "samuel.sidler: approval1.9.0.4?" So they are trying to get approval to land this for Gecko 1.9.0.4 (Firefox 3.0.4)
We won't block on this but will look at approval when the regression from bug 445520 (bug 450949) is fixed.
Flags: blocking1.9.0.4?
Attachment #326964 - Flags: approval1.9.0.4? → approval1.9.0.4-
Comment on attachment 326964 [details] [diff] [review]
Stop double instantiation even earlier.

Can't approve this with open regressions. When the regressions are fixed please create a roll-up patch if you're going to request approval
What is holding this one? Is it 450949?

This one is creating a lot of headache for us to say the least.
This one is creating a lot of headache for us to say the least.
Flags: blocking1.9.0.5?
It's way too late for 1.9.0.5, moving out request. But really, it's bug 450949 you need on the list.
Flags: blocking1.9.0.5? → blocking1.9.0.6?
How is bug fixing actually supposed to work for this software? This bug was fixed 5 months ago, and have been moved forward a lot of times... Are we supposed to vote for the "blocking" bug or are we supposed to update our support page with a link to this bug, so our customers starts to vote for this bug each time we tell them to downgrade to FireFox 2.0! 

Our problem is that we have quite some installed base on different goverment services using our plugin. Goverment means they are very slow on any type of updates for there web pages, so the only possible workaround is to downgrade the web browser. 

I agree: "This one is creating a lot of headache for us to say the least."
Blocks: abp
Whiteboard: [needs 445520 fixed] → [needs 445520 & 450949 fixed]
In the next release of our services we will not accept FF3 users and we will point them to this bug and recommend them to vote.
Blocking on this assuming we can take the regression bug 450949 in stride. Will need to take the regression fix in bug 445520 as well.
Flags: blocking1.9.0.6? → blocking1.9.0.6+
Whiteboard: [needs 445520 & 450949 fixed] → [needs 445520 fixed]
Comment on attachment 326964 [details] [diff] [review]
Stop double instantiation even earlier.

bug 450949 is now resolved by a newer Shockwave, lets try to get this and bug 445520 in.
Attachment #326964 - Flags: approval1.9.0.6?
What is the status on this one? Implemented or getting ready to?
Attachment #326964 - Flags: approval1.9.0.6? → approval1.9.0.6+
Comment on attachment 326964 [details] [diff] [review]
Stop double instantiation even earlier.

Approved for 1.9.0.6, a=dveditz for release-drivers
Fix landed in CVS again.
Keywords: fixed1.9.0.6
Verified fixed in 1.9.0.6 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.6pre) Gecko/2009011304 GranParadiso/3.0.6pre.
Keywords: fixed1.9.0.6 → verified1.9.0.6
Depends on: 474022
(In reply to comment #57)
> In the next release of our services we will not accept FF3 users and we will
> point them to this bug and recommend them to vote.

From Jonas Sicking: 

"So there is good news and bad. This bug has been
fixed since Firefox 3.0.6. I just attempted using Firefox 3.0.7 with a
spoofed UA string to look like Firefox 2, and things mostly works. Was
able to install the e-legitimation and use it to log in to the test
site. I was not however able to the e-legitimation to sign something.
The dialog for signing comes up, but I can't type in it or click any
buttons. I just tested in firefox 2 and things work correctly there."

So something is still wrong, but it's no longer this particular bug. Jonas, since it works in Firefox 2, it sounds like we have an opportunity to compare what's going on. Is any help needed from e.g. TeliaSonera at this point?

John Allberg: are you, or could you get us in contact with one of your developers in case we need help finding the cause of this (new) problem? Note: I'm asking on behalf of Jonas Sicking. I'm not a developer myself, but I work from Sweden and you can certainly use my e-mail if you want to get in touch. Also, do you have the link to the error page where you tell customers to vote for this bug? I think that should be replaced with something else as it's probably just confusing/frustrating to customers.

Thanks!
 Bug 438830 plz help me...remove this bug...i cant logg on in my legitimation fr school because of it....what to do???
michaela, we'd love to help you, but whoever told you that the reason for not being able to log in because of this bug is wrong because this bug has already been fixed (Status RESOLVED FIXED). Please inform whomever pointed you at this bug that they are referring to the wrong bug, and we need help figuring out what is not working and why with regards to e-legitimation websites. Please help us help you, this bug is NOT the problem.
Even better, Michaela, could you point out the page that instructed you to visit this bug report? That would be very helpful for us in order to know who we should get in touch with to fix the underlying problem.

Thanks!
It is Telia (www.telia.se), a swedish telephone company.  The support number is +46-771-323262 for the electronic id service that claims that this bug prevents them from supporting firefox 3.
Thanks to Andreas, I finally found the page where Telia links to this bug: https://cve.trust.telia.com/testa/Step2CheckOsBrowser.aspx.

Interestingly, they don't seem to support Safari either, making the choice very limited for users (basically Firefox 2.0!). 

Jonas, Johnny: if you don't disagree, I can call Telia up first thing tomorrow and try to get in touch with one of their developers so we can work together on getting the real problem solved.
Can someone please open a TE bug so that we don't have to continue tracking this in a FIXED bug?
Hello,
The problem seems to be fixed for you but I have allways the problem.

Here is my test page :
http://www.bauret.be/flash3/readme.html

The first time you open the page you need to click to see the video load in autoplay :-(

Is it the same bug ?
How can I fix it ?

Thx

Arnaud
Hello,

No body for my problem plz ?

Thx

Arnaud
Arnaud, Bugzilla is not a support forum. Please use the support forums at

http://forums.mozillazine.org/

for support questions.
2009-03-19  David wrote:

"Jonas, Johnny: if you don't disagree, I can call Telia up first thing tomorrow
and try to get in touch with one of their developers so we can work together on
getting the real problem solved."

Did you call them?  They still ask you to use FF 2 when applying for E-legitimation. I tried today.
My bank SEB, called Telia today and they said the problem remains. Telia still links to this bug http://tinyurl.com/kt9gcl. Maybe there is an another one?

SEB gave me a phone no to call if I wanted:  0771-323262 (Telia, Sweden).

/ Terhi
I've talked with people at Telia. The remaining issues are unrelated to this bug, but rather related to changes made to use cocoa widgets in Firefox 3 on Mac. The issue appears to be known on their end, but I don't know what the timeline for fixing it is.

I also asked them to remove the reference to this bug, though maybe that hasn't happened yet?
I can still see this with Flowplayer in FF12. According to Firebug, flowplayer.swf is loaded twice per player creation.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: