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

RESOLVED FIXED

Status

()

Core
Plug-ins
RESOLVED FIXED
9 years ago
5 years ago

People

(Reporter: jwatt, Assigned: jst)

Tracking

(Blocks: 1 bug, {fixed1.9.1, verified1.9.0.6})

Trunk
fixed1.9.1, verified1.9.0.6
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 +
blocking1.9.0.1 -
blocking1.9.0.6 +
wanted1.9.0.x +
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [needs 445520 fixed])

Attachments

(2 attachments)

(Reporter)

Description

9 years ago
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.
(Reporter)

Comment 1

9 years ago
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.

Updated

9 years ago
Duplicate of this bug: 439146

Comment 3

9 years ago
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

9 years ago
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?
(Assignee)

Comment 5

9 years ago
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

9 years ago
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

9 years ago
Created attachment 325668 [details] [diff] [review]
double-instantiate.patch

Comment 8

9 years ago
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-

Comment 9

9 years ago
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.
(Assignee)

Comment 10

9 years ago
Created attachment 326964 [details] [diff] [review]
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.
Attachment #326964 - Flags: superreview?(jonas)
Attachment #326964 - Flags: review?(jonas)
(Assignee)

Updated

9 years ago
Duplicate of this bug: 441424
(Assignee)

Updated

9 years ago
Assignee: nobody → jst

Comment 12

9 years ago
(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

9 years ago
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
(Assignee)

Comment 14

9 years ago
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+
(Assignee)

Comment 16

9 years ago
(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

9 years ago
I just tried the patch, and it looks good to me -- it does fix the problems I was seeing.
(Assignee)

Comment 18

9 years ago
Awesome, thanks! This was just checked in, closing bug.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Assignee)

Comment 19

9 years ago
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?

Comment 20

9 years ago
another potential dup in bug 442479

Updated

9 years ago
Duplicate of this bug: 442666

Comment 22

9 years ago
(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.

Comment 24

9 years ago
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?
Depends on: 445520

Comment 25

9 years ago
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?

Comment 28

9 years ago
> 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

9 years ago
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.

Comment 31

9 years ago
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]

Comment 32

9 years ago
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

9 years ago
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+
(Assignee)

Comment 35

9 years ago
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+

Comment 38

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

Comment 39

9 years ago
It wasn't backed out on trunk - only on 1.9 branch. So technically this is still FIXED.
(Assignee)

Comment 40

9 years ago
:(
Flags: blocking1.9.0.3?

Comment 41

9 years ago
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

9 years ago
(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

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

Comment 44

9 years ago
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.
(Assignee)

Comment 45

9 years ago
This is unrelated to the problem in bug 451816.

Comment 46

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

Comment 47

9 years ago
(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

9 years ago
(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
Duplicate of this bug: 445599
Duplicate of this bug: 465374

Comment 53

9 years ago
What is holding this one? Is it 450949?

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

Comment 54

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

Updated

9 years ago
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?

Comment 56

9 years ago
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."

Updated

9 years ago
Blocks: 467520
Whiteboard: [needs 445520 fixed] → [needs 445520 & 450949 fixed]
Keywords: fixed1.9.1

Comment 57

9 years ago
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?

Comment 60

8 years ago
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
(Assignee)

Comment 62

8 years ago
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

Updated

8 years ago
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!

Comment 65

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

Comment 66

8 years ago
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!

Comment 68

8 years ago
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.

Comment 70

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

Comment 71

8 years ago
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

8 years ago
Hello,

No body for my problem plz ?

Thx

Arnaud

Comment 73

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

http://forums.mozillazine.org/

for support questions.

Comment 74

8 years ago
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?

Updated

7 years ago

Comment 76

5 years ago
I can still see this with Flowplayer in FF12. According to Firebug, flowplayer.swf is loaded twice per player creation.
You need to log in before you can comment on or make changes to this bug.