Closed Bug 173938 Opened 22 years ago Closed 22 years ago

full-page plugins inside an IFRAME cause gdk error / crash because the plugin window is destroyed after the iframe one

Categories

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

defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.3alpha

People

(Reporter: sam, Assigned: peterl-bugs)

References

()

Details

(Keywords: crash, regression, testcase, Whiteboard: [PL2:NA])

Attachments

(4 files, 2 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.1) Gecko/20020827
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.1) Gecko/20020827

http:joninfo.it

The site come up with all bells and whistles, and crashes befor I have a chanve
to use it

Reproducible: Always

Steps to Reproduce:
1.go to the site
2.
3.

Actual Results:  
"Exit 1"

Expected Results:  
allowed me to interacct with the page, duh...
do you crash with latest nightly build ? please mention Flash and Java plug-in
versions, are you using sound w/ Mozilla or using Mozilla remotely ?
Any chance to post GDB stack trace or Talkback ID
'mozilla/bin/components/talkback/talkback' for this crash ?
Keywords: crash, stackwanted
Shockwave Flash 4.0 r12
Java(TM) Plug-in 1.4.0-b92 -  File name: libjavaplugin_oji140.so

Plugger version 4.0

Also, BannerBlind.

Can I switch something to make a talkback ? I got this as an RPM, god knows
where from, probably either mozilla.org of redhat rawhide.
Neither the URL in the bug nor the comment is valid for me.  Reporter: Are you
sure it's correct?
Works perfectly for me on Linux with today cvs 20021011.

P.s. The url is http://www.jobinfo.it
are you running on a remote or multi-head display?  Both will cause a crash on
any page with flash.

If you use an RPM build, you can use gdb to grab the stacktrace yourself.
When I closed Mozilla, it died with a Gdk error.  Investigating...
Attached file stacktrace
not a very useful stack...  :(
Attached file testcase
iframe + flash = crash
it crashes when you reload, leave the page or close.
marking NEW

this was fixed in bug 152429, so must be a regression

==> plugins
Assignee: asa → serge
Status: UNCONFIRMED → NEW
Ever confirmed: true
QA Contact: asa → shrir
Summary: The site come up with all bells and whistles, and crashes befor I have a chanve to use it → iframe + flash = gdk error / crash
actually, this bug is older than bug 152429.  that bug is still fixed.
this bug regressed between trunk builds 2002041617 and 2002041707
bug 52334 appears to be the most likely culprit
this site wfm on 1014 trunk on linux.
oops
==> plugins
Component: Browser-General → Plug-ins
*** Bug 176898 has been marked as a duplicate of this bug. ***
*** Bug 176966 has been marked as a duplicate of this bug. ***
This happens on Windows too. We are destroying the IFRAME widget before the
plugin one and before the instance is destroyed.. For an explaination and
quickie patch that stops the crash by removing the check for mOwnsFrameLoader, see:
http://bugzilla.mozilla.org/show_bug.cgi?id=157157#c72

cc:ing jst, owner of bug 52334.
OS: Linux → All
Hardware: PC → All
Priority: -- → P2
Whiteboard: [PL2:NA]
Target Milestone: --- → mozilla1.3beta
*** Bug 177718 has been marked as a duplicate of this bug. ***
Summary: iframe + flash = gdk error / crash → iframe + flash = gdk error / crash [IFRAME window destroyed before plugin]
--->peterl

jst gave me a clue as to how to fix this without hacking up all of plugin code:
nsIContentViewer::Hide is called by the nsFrameFrame before going away so we
should take care of destroying the plugin there for now until bug 90268 and bug
bug 90256 are fixed.
Assignee: serge → peterl
Summary: iframe + flash = gdk error / crash [IFRAME window destroyed before plugin] → full-page plugins inside an IFRAME cause gdk error / crash because the plugin window is destroyed after the iframe one
Attached patch patch v.1 (obsolete) — Splinter Review
So I tired to use nsIContentViewer::Hide to either reparent the plugin window
or destroy and re-create it. Both attempts failed because I either had problems
getting to my parent widget or I didn't have access to a new parent to
correctly place myself when shown.

Instead, this patch changes the document destruction in nsFrameFrame so that
full-page plugins are always destroyed before the frame, like was done before
the fix to bug 52334. When full-page plugins are moved to use the same code
path as embedded in bug 90256 and plugins can survive a display: none in bug
90268; this code can probably be removed.

In addition to the special casing, I also noticed that we were creating the
next full-page plugin before the current one is destoryed when the testcase was
reloaded. The old plugin instance always needs to be destroyed before the new
one is created. This patch also ensure the previous content viewer is destroyed
by telling the full-page plugin's parent content viewer to show itself (which
has the side-effect of destroying the previous document's frames).
Attachment #106031 - Flags: superreview?(jst)
Attachment #106031 - Flags: review?(av)
Status: NEW → ASSIGNED
Keywords: patch, review
Target Milestone: mozilla1.3beta → mozilla1.3alpha
Comment on attachment 106031 [details] [diff] [review]
patch v.1

r=av
Attachment #106031 - Flags: review?(av) → review+
Comment on attachment 106031 [details] [diff] [review]
patch v.1

sr=jst, assuming this doesn't break repeated showing and hiding of an iframe
containing a full page plugin.
Attachment #106031 - Flags: superreview?(jst) → superreview+
ok, that last patch does break showing/hiding the plugin with display: none.
When the nsFrameFrame tries to show the plugin for a second time, it crashes
because Destroy was called on the nsIFrameLoader and it hasn't been setup for a
new document. Here's a testcase.
Attachment #106031 - Attachment is obsolete: true
Attached patch patch v.2 (obsolete) — Splinter Review
Here's another patch that adds to the last by clearing out the nsIFrameLoader
owned by content. The next time the frame is created, a new frame loader we be
made which will re-load our full-page plugin.
Attached patch patch v.2.1Splinter Review
oops, last attachment has wrong patch, doesn't check null on mFrameLoader.
Here's the right one.
Attachment #106312 - Attachment is obsolete: true
Attachment #106313 - Flags: superreview?(jst)
Attachment #106313 - Flags: review?(av)
Comment on attachment 106313 [details] [diff] [review]
patch v.2.1

sr=jst, but I do hope this could be backed out once the full page plugin code
is fixed up once n' for all.
Attachment #106313 - Flags: superreview?(jst) → superreview+
Comment on attachment 106313 [details] [diff] [review]
patch v.2.1

r=av, maybe it would make sense to file a reminder bug to remove this code when
it is no longer needed.
Attachment #106313 - Flags: review?(av) → review+
Yeah, please do file a bug about removing this once it's not needed...
patch was checked in last night, marking FIXED.

Bug 180802 was opend about removing this code after bug 90256 and bug 90268 are
fixed (will probably need to be touched anyway in removing nsIPluginViewer
interface).
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Keywords: review
Resolution: --- → FIXED
tested using a trunk build from 20021120
win 98
Was able to navigate through the site without incident
Status: RESOLVED → VERIFIED
*** Bug 180030 has been marked as a duplicate of this bug. ***
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: