Closed Bug 545734 Opened 10 years ago Closed 10 years ago

need to hide the iframe used for submitting plugin crash reports

Categories

(Toolkit :: Crash Reporting, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.3a2
Tracking Status
status1.9.2 --- .4-fixed

People

(Reporter: Dolske, Assigned: Dolske)

References

Details

(Keywords: dogfood, Whiteboard: [fixed-lorentz])

Attachments

(1 file, 2 obsolete files)

Got a couple reports of a mysterious box appearing in the browser UI, containing text with a crashID... "there was a frame on the right, with a width of about 150-200px, white apart a crash ID into it. it was starting under bookmarks toolbar and going down till the statusbar. tabbar was on the left of this frame that was not closeable nor resizeable nor whatever."

I think what's happening is that the iframe added by CrashSubmit.jsm needs to hidden (display:none or make is 0x0). I don't see that being done anywhere. Mak also noted that this iframe seems to persist... Not sure if it was just really really slow to submit, or there's a case where it fails to get removed (looks like the code does at least attempt to remove the iframe after submission).
I don't think it's just a slow submission - I had it appear yesterday, and it persisted overnight. Unless it's a timeout, i guess.
Bleh, it ought to be removed, dunno why it wouldn't be. Also, I wish we had proper APIs so this didn't have to use an iframe.
Attached image attack of the iframes (obsolete) —
This is what one of my windows looks like right now, thanks to this bug and the #1 topcrash bug on Linux.
Heh. I wonder if the CrashSubmit code is failing here because it expects to be operating on HTML? about:crashes is XHTML, but clearly when used in browser.xul it's operating on XUL.

Assuming the webprogresslistener gets all the way to STATE_STOP, it should remove the iframe no matter what:
http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/CrashSubmit.jsm#268

Might be interesting to stick some extra debug statements in there to find out what's happening.
Is anyone seeing this on Windows?
Error: this.iframe.docShell.removeProgressListener is not a function
Source File: file:///builds/minefield-x86/firefox/modules/CrashSubmit.jsm
Line: 269

This also seemingly prevents the plugin-crash UI from actually showing.
Note, #testday participants also say that whenever the crash reporter is enabled on Linux they never see the plugin-crashed UI. The iframe being stuck visible is only an occasional problem beyond that.
more details: the first time I go to http://flashcrash.dempsky.org/ the iframe doesn't appear, but no plugin-crash UI. I hit refresh, and the iframe appears with the error console message about .removeProgressListener.
The code explicitly creates a xul:iframe:
http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/CrashSubmit.jsm#312

I don't know why that would fail.
(In reply to comment #5)
> Is anyone seeing this on Windows?

Yes, Windows 7 here.
I suspect some (all?) cases of the plugin-crashed UI not being shown are either bug 544550 or bug 545686.
Attached patch Patch v.1 (obsolete) — Splinter Review
I was able to reproduce the sticky iframe by unplugging my network cable, the form submission fails after a few seconds and the frame ends up holding an error page. [Note to self: also need to set MOZ_CRASHREPORTER=1 to trigger submission in own builds.]

I got other errors if I styled the iframe with display:none (presumably that's bad to do for an iframe with content you're trying to manipulate?). So, instead, I set width=0 plus a minWidth override for the default iframe style.

The error that was being thrown in some cases is fixed by adding a QI. I guess the error page results in a new docshell (which hasn't been QI'd to this interface yet?)...
Assignee: nobody → dolske
Attachment #426642 - Attachment is obsolete: true
Attachment #427047 - Flags: review?(ted.mielczarek)
Blocks: LorentzAlpha
Comment on attachment 427047 [details] [diff] [review]
Patch v.1

>diff --git a/toolkit/crashreporter/CrashSubmit.jsm b/toolkit/crashreporter/CrashSubmit.jsm
>--- a/toolkit/crashreporter/CrashSubmit.jsm
>+++ b/toolkit/crashreporter/CrashSubmit.jsm
>@@ -261,16 +261,17 @@ Submitter.prototype = {
>         aIID.equals(Ci.nsISupports))
>       return this;
>     throw Components.results.NS_NOINTERFACE;
>   },
> 
>   onStateChange: function(aWebProgress, aRequest, aFlag, aStatus)
>   {
>     if(aFlag & STATE_STOP) {
>+      this.iframe.docShell.QueryInterface(Ci.nsIWebProgress);
>       this.iframe.docShell.removeProgressListener(this);
> 
>       // check general request status first
>       if (!Components.isSuccessCode(aStatus)) {
>         this.element.removeChild(this.iframe);
>         if (this.errorCallback) {
>           this.errorCallback(this.id);
>         }
>@@ -306,16 +307,18 @@ Submitter.prototype = {
>     if (!dump.exists() || !extra.exists()) {
>       this.cleanup();
>       return false;
>     }
>     this.dump = dump;
>     this.extra = extra;
>     let iframe = this.document.createElementNS("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul", "iframe");
>     iframe.setAttribute("type", "content");
>+    iframe.style.width = 0;
>+    iframe.style.minWidth= 0;

nit: spaces around equals sign.

Looks fine otherwise, r=me
Attachment #427047 - Flags: review?(ted.mielczarek) → review+
Attached patch Patch v.2Splinter Review
Attachment #427047 - Attachment is obsolete: true
Pushed http://hg.mozilla.org/mozilla-central/rev/47994c0cb561
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a2
Blanket approval for Lorentz merge to mozilla-1.9.2
a=beltzner for 1.9.2.4 - please make sure to mark status1.9.2:.4-fixed
You need to log in before you can comment on or make changes to this bug.