Closed Bug 508057 Opened 10 years ago Closed 10 years ago

crash [@ imgRequestProxy::OnStopRequest(nsIRequest*, nsISupports*, unsigned int, int) ] Classic Compact theme

Categories

(Core :: ImageLib, defect, critical)

defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla1.9.3a1
Tracking Status
status1.9.2 --- beta1-fixed
blocking1.9.1 --- .4+
status1.9.1 --- .4-fixed

People

(Reporter: kbrosnan, Assigned: joe)

References

()

Details

(4 keywords, Whiteboard: [sg:critical?])

Crash Data

Attachments

(2 files)

The extension developer reported that his theme is causing crashes in Firefox. BZ asked me to file a bug so this would not get lost. http://forums.mozillazine.org/viewtopic.php?f=18&t=1395115

Install the classic compact theme and Classic Compact Options

https://addons.mozilla.org/en-US/firefox/addons/versions/6969
https://addons.mozilla.org/en-US/firefox/addons/versions/3699
Use the Classic Compact Options to set the "background style" to "flat"
bookmarks > organize bookmarks will often crash Firefox

Crash stats link in the url ~75 crashes a week.
Group: core-security
Blocks: 393936
blocking1.9.1: --- → ?
Flags: blocking1.9.2?
Attached patch Proposed patchSplinter Review
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #392273 - Flags: review?(joe)
Just to be clear, this is much like bug 493240 in terms of branch/security stuff.
Most crashes in past week caused by me trying to figure out what is going on. I am getting scattered reports from other users of the crash issue.  Only crashes when Classic Compact Options "background style" option is set to either "flat" or "flat except tabs". I have not been able to cause crashes with other option combinations or with default Classic Compact theme.

I have gone through all related CSS and can not find obvious CSS errors.
(In reply to comment #2)
> Just to be clear, this is much like bug 493240 in terms of branch/security
> stuff.

Security-wise? Or interface-using-wise?
blocking1.9.1: ? → .3+
The former; I have no idea what you mean by the latter.... ;)
Comment on attachment 392273 [details] [diff] [review]
Proposed patch

I still don't know of any way to ask DXR for things that call Cancel on an imgIRequest :(
Attachment #392273 - Flags: review?(joe) → review+
If it is any help, I think I got my theme/extension combo to stop crashing Firefox 3.5.1.  What I did was better synchronize my editBookmarkOverlay.css with the default Firefox editBookmarkOverlay.css by adding the following CSS to my version:

===begin added CSS====
/* Hide the value column of the tag autocomplete popup
* leaving only the comment column visible. This is
* so that only the tag being edited is shown in the
* popup.
*/
#editBMPanel_tagsField #treecolAutoCompleteValue {
  visibility: collapse;
}


/* ::::: bookmark panel dropdown icons ::::: */

#editBMPanel_folderMenuList[selectedIndex="0"],
#editBMPanel_toolbarFolderItem {
  list-style-image: url("chrome://browser/skin/places/bookmarksToolbar.png") !important;
  -moz-image-region: auto !important;
}

#editBMPanel_folderMenuList[selectedIndex="1"],
#editBMPanel_bmRootItem {
  list-style-image: url("chrome://browser/skin/places/bookmarksMenu.png") !important;
  -moz-image-region: auto !important;
}

#editBMPanel_folderMenuList[selectedIndex="2"],
#editBMPanel_unfiledRootItem {
  list-style-image: url("chrome://browser/skin/places/unsortedBookmarks.png") !important;
  -moz-image-region: auto !important;
}
===End Added CSS===

I have no idea why this mattered and I don't notice visual changes to the bookmarks library. What I do know is that since I added this code I have been unable to get Firefox to crash regardless of how the "background style" option is set in Classic Compact Options.  A test build of Classic Compact with the above CSS added can be downloaded and tried from http://environmentalchemistry.com/classiccompact.jar.  Once I have confirmation from others that this indeed stops the crashes I will push this update to AMO.
Whiteboard: [sg:critical?]
Summary: crash @imgRequestProxy::OnStopRequest(nsIRequest*, nsISupports*, unsigned int, int) ] Classic Compact theme → crash [@ imgRequestProxy::OnStopRequest(nsIRequest*, nsISupports*, unsigned int, int) ] Classic Compact theme
Pushed http://hg.mozilla.org/mozilla-central/rev/a7e5345ecbea
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Boris: Is this patch ready for 1.9.1? Can you request approval if so?
Attachment #392273 - Flags: approval1.9.1.3?
Attachment #392273 - Flags: approval1.9.1.3? → approval1.9.1.3+
Comment on attachment 392273 [details] [diff] [review]
Proposed patch

Approved for 1.9.1.3. a=ss
This does not appear to be fixed in 1.9.1. Using the extension and theme linked above (before they were fixed on August 4) with both 1.9.1.2 and a post-fix 1.9.1.3 build, I easily get the crash while doing organize bookmarks.

Tested with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.3pre) Gecko/20090817 Shiretoko/3.5.3pre (.NET CLR 3.5.30729).
So this is now crashing because of an nsImageBoxFrame that calls Cancel() in UpdateImage(), but then apparently goes away shortly thereafter. Calling CancelAndForgetObserver() instead should work, but I'm not sure if it's 100% safe (see: bug 393936).
Given the comments above, this isn't actually fixed for 1.9.1.3. We'll push it to 1.9.1.4.
blocking1.9.1: .3+ → .4+
Sigh. At some point we'll have solved this ownership model, honest!
Assignee: bzbarsky → joe
Status: RESOLVED → REOPENED
Attachment #397387 - Flags: superreview?(bzbarsky)
Attachment #397387 - Flags: review?
Resolution: FIXED → ---
Attachment #397387 - Flags: superreview?(bzbarsky)
Attachment #397387 - Flags: review?(bzbarsky)
Attachment #397387 - Flags: review?
Comment on attachment 397387 [details] [diff] [review]
Cancel -> CancelAndForgetObserver

r=bzbarsky
Attachment #397387 - Flags: review?(bzbarsky) → review+
http://hg.mozilla.org/mozilla-central/rev/08c42708aad8
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Comment on attachment 397387 [details] [diff] [review]
Cancel -> CancelAndForgetObserver

Applies cleanly to 1.9.1. Requesting approval.
Attachment #397387 - Flags: approval1.9.1.4?
Comment on attachment 397387 [details] [diff] [review]
Cancel -> CancelAndForgetObserver

Approved for 1.9.1.4, a=dveditz
Attachment #397387 - Flags: approval1.9.1.4? → approval1.9.1.4+
Flags: blocking1.9.2? → blocking1.9.2+
Verified fixed on trunk, 1.9.2, and 1.9.1 with the builds below and Classic Compact theme 3.2.0 installed.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20091006 Minefield/3.7a1pre (.NET CLR 3.5.30729) ID:20091006044117

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2b1pre) Gecko/20091007 Namoroka/3.6b1pre (.NET CLR 3.5.30729) ID:20091007045631

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.4) Gecko/20091007 Firefox/3.5.4 (.NET CLR 3.5.30729) ID:20091007001339
Status: RESOLVED → VERIFIED
Target Milestone: --- → mozilla1.9.3a1
nominating for 1.9.0 consideration:
  This is a regression from bug 393936. That patch didn't land in the main Mozilla repo for 1.9.0, but that bug has a 1.9.0 back-port by asac which presumably landed in Ubuntu and maybe other linux distros. If so they need this fix (and for bug 493240) on that branch also.
Flags: wanted1.9.0.x?
Whiteboard: [sg:critical?] → [sg:critical?] keep bug closed while investigating comment 23
Ubuntu didn't pick up the older bug, we can unhide this one.
Group: core-security
Whiteboard: [sg:critical?] keep bug closed while investigating comment 23 → [sg:critical?]
Crash Signature: [@ imgRequestProxy::OnStopRequest(nsIRequest*, nsISupports*, unsigned int, int) ]
You need to log in before you can comment on or make changes to this bug.