Last Comment Bug 508057 - crash [@ imgRequestProxy::OnStopRequest(nsIRequest*, nsISupports*, unsigned int, int) ] Classic Compact theme
: crash [@ imgRequestProxy::OnStopRequest(nsIRequest*, nsISupports*, unsigned i...
Status: VERIFIED FIXED
: [sg:critical?]
: crash, regression, testcase-wanted, verified1.9.1, verified1.9.2
Product: Core
Classification: Components
Component: ImageLib
: Trunk
: All All
: -- critical with 1 vote (vote)
: mozilla1.9.3a1
Assigned To: Joe Drew (:JOEDREW!)
: imagelib
: http://crash-stats.mozilla.com/query/...
:
: 393936
  Show dependency treegraph
 
Reported: 2009-08-03 10:35 PDT by Kevin Brosnan
Modified: 2010-07-15 09:55 PDT (History)
13 users (show)
dtownsend: blocking1.9.2+
dveditz: wanted1.9.0.x?
bzbarsky: in‑testsuite?
See Also:
Crash Signature:
[@ imgRequestProxy::OnStopRequest(nsIRequest*, nsISupports*, unsigned int, int) ]
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  beta1-fixed
  .4+
  .4-fixed


Attachments
Proposed patch (851 bytes, patch)
2009-08-03 10:46 PDT, Boris Zbarsky (:bz)
joe: review+
samuel.sidler+old: approval1.9.1.3+
Details | Diff | Splinter Review
Cancel -> CancelAndForgetObserver (647 bytes, patch)
2009-08-28 17:18 PDT, Joe Drew (:JOEDREW!)
bzbarsky: review+
dveditz: approval1.9.1.4+
Details | Diff | Splinter Review

Summon comment box

Description Kevin Brosnan 2009-08-03 10:35:40 PDT
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.
Comment 1 Boris Zbarsky (:bz) 2009-08-03 10:46:31 PDT
Created attachment 392273 [details] [review]
Proposed patch
Comment 2 Boris Zbarsky (:bz) 2009-08-03 10:49:59 PDT
Just to be clear, this is much like bug 493240 in terms of branch/security stuff.
Comment 3 KLB 2009-08-03 11:03:51 PDT
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.
Comment 4 Samuel Sidler (old account; do not CC) 2009-08-03 11:33:56 PDT
(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?
Comment 5 Boris Zbarsky (:bz) 2009-08-03 11:48:18 PDT
The former; I have no idea what you mean by the latter.... ;)
Comment 6 Joe Drew (:JOEDREW!) 2009-08-03 13:00:20 PDT
Comment on attachment 392273 [details] [review]
Proposed patch

I still don't know of any way to ask DXR for things that call Cancel on an imgIRequest :(
Comment 7 KLB 2009-08-03 14:13:49 PDT
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.
Comment 8 Boris Zbarsky (:bz) 2009-08-03 20:07:45 PDT
Pushed http://hg.mozilla.org/mozilla-central/rev/a7e5345ecbea
Comment 9 Samuel Sidler (old account; do not CC) 2009-08-09 20:17:26 PDT
Boris: Is this patch ready for 1.9.1? Can you request approval if so?
Comment 10 Samuel Sidler (old account; do not CC) 2009-08-09 20:36:21 PDT
Comment on attachment 392273 [details] [review]
Proposed patch

Approved for 1.9.1.3. a=ss
Comment 11 Boris Zbarsky (:bz) 2009-08-10 12:20:41 PDT
Pushed http://hg.mozilla.org/releases/mozilla-1.9.1/rev/604b99ffe2c9
Comment 12 Al Billings [:abillings] 2009-08-20 15:18:12 PDT
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).
Comment 13 Joe Drew (:JOEDREW!) 2009-08-21 14:25:48 PDT
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).
Comment 14 Samuel Sidler (old account; do not CC) 2009-08-27 10:16:33 PDT
Given the comments above, this isn't actually fixed for 1.9.1.3. We'll push it to 1.9.1.4.
Comment 15 Joe Drew (:JOEDREW!) 2009-08-28 17:18:19 PDT
Created attachment 397387 [details] [review]
Cancel -> CancelAndForgetObserver

Sigh. At some point we'll have solved this ownership model, honest!
Comment 16 Boris Zbarsky (:bz) 2009-08-28 17:24:02 PDT
Comment on attachment 397387 [details] [review]
Cancel -> CancelAndForgetObserver

r=bzbarsky
Comment 17 Joe Drew (:JOEDREW!) 2009-09-02 08:52:19 PDT
http://hg.mozilla.org/mozilla-central/rev/08c42708aad8
Comment 18 Joe Drew (:JOEDREW!) 2009-09-21 15:57:53 PDT
Comment on attachment 397387 [details] [review]
Cancel -> CancelAndForgetObserver

Applies cleanly to 1.9.1. Requesting approval.
Comment 19 Daniel Veditz 2009-09-21 16:49:23 PDT
Comment on attachment 397387 [details] [review]
Cancel -> CancelAndForgetObserver

Approved for 1.9.1.4, a=dveditz
Comment 20 Joe Drew (:JOEDREW!) 2009-09-22 11:27:04 PDT
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/0e2cdc91a769
Comment 21 Joe Drew (:JOEDREW!) 2009-09-22 12:38:53 PDT
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/41b236f244fd
Comment 22 Henrik Skupin (:whimboo) 2009-10-08 12:11:14 PDT
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
Comment 23 Daniel Veditz 2009-10-16 17:08:51 PDT
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.
Comment 24 Daniel Veditz 2010-07-15 09:55:49 PDT
Ubuntu didn't pick up the older bug, we can unhide this one.

Note You need to log in before you can comment on or make changes to this bug.