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)
Core Graveyard
Plug-ins
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: jwatt, Assigned: jst)
References
Details
(Keywords: fixed1.9.1, verified1.9.0.6, Whiteboard: [needs 445520 fixed])
Attachments
(2 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
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.
Reporter | ||
Comment 1•16 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.
Comment 3•16 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•16 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•16 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•16 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•16 years ago
|
||
Comment 8•16 years ago
|
||
See also bug 441424 which sounds like this problem too.
Updated•16 years ago
|
Flags: wanted1.9.0.x?
Flags: blocking1.9.0.1?
Flags: blocking1.9.0.1-
Comment 9•16 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•16 years ago
|
||
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•16 years ago
|
Assignee: nobody → jst
Comment 12•16 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•16 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•16 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•16 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•16 years ago
|
||
I just tried the patch, and it looks good to me -- it does fix the problems I was seeing.
Assignee | ||
Comment 18•16 years ago
|
||
Awesome, thanks! This was just checked in, closing bug.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 19•16 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•16 years ago
|
||
another potential dup in bug 442479
Comment 22•16 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.*?
Comment 23•16 years ago
|
||
(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•16 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...
Updated•16 years ago
|
Attachment #326964 -
Flags: approval1.9.0.1? → approval1.9.0.2?
Comment 25•16 years ago
|
||
Did this ever get checked in on 1.9.0.1?
Comment 26•16 years ago
|
||
(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•16 years ago
|
||
Anyway we can get a test for this? Maybe from the work in bug 441424?
Flags: in-testsuite?
Comment 28•16 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•16 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!
Comment 30•16 years ago
|
||
Mike, this will get approved when we have a resolution to bug 445520, which appears to be a regression from this bug.
Comment 31•16 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 :-(
Updated•16 years ago
|
Whiteboard: [needs 445520 fixed]
Comment 32•16 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•16 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 34•16 years ago
|
||
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+
Comment 37•16 years ago
|
||
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•16 years ago
|
||
Shouldn't this be reopened then if it was backed out?
Comment 39•16 years ago
|
||
It wasn't backed out on trunk - only on 1.9 branch. So technically this is still FIXED.
Assignee | ||
Comment 40•16 years ago
|
||
:(
Updated•16 years ago
|
Flags: blocking1.9.0.3?
Comment 41•16 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•16 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•16 years ago
|
||
Sorry, bug 450949 is the one blocking that right now.
Comment 44•16 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•16 years ago
|
||
This is unrelated to the problem in bug 451816.
Comment 46•16 years ago
|
||
Was this one included in 3.0.2 or will it be shipped in 3.0.3?
Comment 47•16 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•16 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)
Comment 49•16 years ago
|
||
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?
Updated•16 years ago
|
Attachment #326964 -
Flags: approval1.9.0.4? → approval1.9.0.4-
Comment 50•16 years ago
|
||
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•16 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•16 years ago
|
||
This one is creating a lot of headache for us to say the least.
Comment 55•16 years ago
|
||
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•16 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•16 years ago
|
Whiteboard: [needs 445520 fixed] → [needs 445520 & 450949 fixed]
Updated•16 years ago
|
Keywords: fixed1.9.1
Comment 57•16 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.
Comment 58•16 years ago
|
||
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+
Updated•16 years ago
|
Whiteboard: [needs 445520 & 450949 fixed] → [needs 445520 fixed]
Comment 59•16 years ago
|
||
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•16 years ago
|
||
What is the status on this one? Implemented or getting ready to?
Updated•16 years ago
|
Attachment #326964 -
Flags: approval1.9.0.6? → approval1.9.0.6+
Comment 61•16 years ago
|
||
Comment on attachment 326964 [details] [diff] [review]
Stop double instantiation even earlier.
Approved for 1.9.0.6, a=dveditz for release-drivers
Comment 63•16 years ago
|
||
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
Comment 64•16 years ago
|
||
(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•16 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•16 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.
Comment 67•16 years ago
|
||
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•16 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.
Comment 69•16 years ago
|
||
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•16 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•16 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•16 years ago
|
||
Hello,
No body for my problem plz ?
Thx
Arnaud
Comment 73•16 years ago
|
||
Arnaud, Bugzilla is not a support forum. Please use the support forums at
http://forums.mozillazine.org/
for support questions.
Comment 74•15 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?
See Also: → https://launchpad.net/bugs/243520
Comment 76•13 years ago
|
||
I can still see this with Flowplayer in FF12. According to Firebug, flowplayer.swf is loaded twice per player creation.
Updated•3 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•