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

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Jonathan Watt [:jwatt]]

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.

Comment 1

[Jonathan Watt [:jwatt]]

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.

Comment 3

[Shane Caraveo]

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.

Comment 4

[Shane Caraveo]

Once the dust settles (fx 3 release) I'd like to get some feedback on the patch.

Comment 5

[Johnny Stenback (:jst)]

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...

Comment 6

[Shane Caraveo]

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 ()

Comment 7

[Shane Caraveo]

Attachment: double-instantiate.patch

Comment 8

[Brian Polidoro]

See also bug 441424 which sounds like this problem too. 

Comment 9

[Tristan Schmelcher]

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.

Comment 10

[Johnny Stenback (:jst)]

Attachment: Stop double instantiation even earlier.

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.

Comment 12

[Shane Caraveo]

(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.

Comment 13

[Wladimir Palant]

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

Comment 14

[Johnny Stenback (:jst)]

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).

Comment 15

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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

Comment 16

[Johnny Stenback (:jst)]

(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.

Comment 17

[Mike Morearty]

I just tried the patch, and it looks good to me -- it does fix the problems I was seeing.

Comment 18

[Johnny Stenback (:jst)]

Awesome, thanks! This was just checked in, closing bug.

Comment 19

[Johnny Stenback (:jst)]

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.

Comment 20

[Shane Caraveo]

another potential dup in bug 442479

Comment 22

[Shane Caraveo]

(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.*?

Comment 23

[Samuel Sidler (old account; do not CC)]

(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.

Comment 24

[Tom Finegan]

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...

Comment 25

[Philip Chee]

Did this ever get checked in on 1.9.0.1?

Comment 26

[Samuel Sidler (old account; do not CC)]

(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.

Comment 27

[Samuel Sidler (old account; do not CC)]

Anyway we can get a test for this? Maybe from the work in bug 441424?

Comment 28

[Philip Chee]

> 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.

Comment 29

[Mike Morearty]

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!

Comment 30

[Samuel Sidler (old account; do not CC)]

Mike, this will get approved when we have a resolution to bug 445520, which appears to be a regression from this bug.

Comment 31

[Wladimir Palant]

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 :-(

Comment 32

[Robert Burke]

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.

Comment 33

[Robert Burke]

Correction to previous post:

swfobject 1.5 embeds flash with an embed tag

swfobject 2.1 embeds flash with an object tag

Comment 34

[Samuel Sidler (old account; do not CC)]

Comment on attachment 326964 [details] [diff] [review]
Stop double instantiation even earlier.

Approved for 1.9.0.2. Please land in CVS. a=ss

Comment 35

[Johnny Stenback (:jst)]

Fix landed in CVS.

Comment 36

[Reed Loden [:reed]]

Backed out on CVS HEAD.

Comment 37

[Samuel Sidler (old account; do not CC)]

Comment on attachment 326964 [details] [diff] [review]
Stop double instantiation even earlier.

Pushing this out to 1.9.0.3.

Comment 38

[u88484]

Shouldn't this be reopened then if it was backed out?

Comment 39

[Wladimir Palant]

It wasn't backed out on trunk - only on 1.9 branch. So technically this is still FIXED.

Comment 40

[Johnny Stenback (:jst)]

:(

Comment 41

[Magnus Alström]

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.

Comment 42

[Wladimir Palant]

(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.

Comment 43

[Wladimir Palant]

Sorry, bug 450949 is the one blocking that right now.

Comment 44

[Phillip Whelan]

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.

Comment 45

[Johnny Stenback (:jst)]

This is unrelated to the problem in bug 451816.

Comment 46

[Michael Olofsson]

Was this one included in 3.0.2 or will it be shipped in 3.0.3?

Comment 47

[DB Cooper]

(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

Comment 48

[u88484]

(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)

Comment 49

[Samuel Sidler (old account; do not CC)]

We won't block on this but will look at approval when the regression from bug 445520 (bug 450949) is fixed.

Comment 50

[Daniel Veditz [:dveditz]]

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

Comment 53

[Michael Olofsson]

What is holding this one? Is it 450949?

This one is creating a lot of headache for us to say the least.

Comment 54

[John Allberg]

This one is creating a lot of headache for us to say the least.

Comment 55

[Daniel Veditz [:dveditz]]

It's way too late for 1.9.0.5, moving out request. But really, it's bug 450949 you need on the list.

Comment 56

[Magnus Alström]

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."

Comment 57

[John Allberg]

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.

Comment 58

[Daniel Veditz [:dveditz]]

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.

Comment 59

[Daniel Veditz [:dveditz]]

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.

Comment 60

[Michael Olofsson]

What is the status on this one? Implemented or getting ready to?

Comment 61

[Daniel Veditz [:dveditz]]

Comment on attachment 326964 [details] [diff] [review]
Stop double instantiation even earlier.

Approved for 1.9.0.6, a=dveditz for release-drivers

Comment 62

[Johnny Stenback (:jst)]

Fix landed in CVS again.

Comment 63

[Al Billings [:abillings - ex-MoCo]]

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.

Comment 64

[David Tenser [:djst]]

(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!

Comment 65

[micaela]

 Bug 438830 plz help me...remove this bug...i cant logg on in my legitimation fr school because of it....what to do???

Comment 66

[Johnny Stenback (:jst)]

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.

Comment 67

[David Tenser [:djst]]

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!

Comment 68

[Andreas Järliden]

It is Telia (www.telia.se), a swedish telephone company.  The support number is [phone] for the electronic id service that claims that this bug prevents them from supporting firefox 3.

Comment 69

[David Tenser [:djst]]

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.

Comment 70

[Chris Lawson (gone)]

Can someone please open a TE bug so that we don't have to continue tracking this in a FIXED bug?

Comment 71

[Arnaud]

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

Comment 72

[Arnaud]

Hello,

No body for my problem plz ?

Thx

Arnaud

Comment 73

[Chris Lawson (gone)]

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

http://forums.mozillazine.org/

for support questions.

Comment 74

[Terhi]

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

Comment 75

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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?

Comment 76

[kverlin]

I can still see this with Flowplayer in FF12. According to Firebug, flowplayer.swf is loaded twice per player creation.