STR: 1. Open two tabs. 2. In one tab, load URL ( http://www.mikehemesath.com/svg_markers/demo/marker.svg ) 3. Close that tab. ACTUAL RESULTS: Crash [@ ExternalResourceHider ] The crash seems 100% reproducible, and the crash-stack seems to stay consistent. bp-92330b78-5623-41df-9130-8da4f2100815 bp-b45e7888-c12e-422b-9af2-238922100815 bp-94926f48-7510-4cda-9881-b7a652100815

Mozilla/5.0 (X11; Linux x86_64; rv:2.0b4pre) Gecko/20100815 Minefield/4.0b4pre

This is a very recent regression. Requesting blocking. WORKS: Mozilla/5.0 (X11; Linux x86_64; rv:2.0b4pre) Gecko/20100811 Minefield/4.0b4pre Built from http://hg.mozilla.org/mozilla-central/rev/ba956b17d834 BROKEN: Mozilla/5.0 (X11; Linux x86_64; rv:2.0b4pre) Gecko/20100812 Minefield/4.0b4pre Built from http://hg.mozilla.org/mozilla-central/rev/cdfff833edf9 Regression pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ba956b17d834&tochange=cdfff833edf9 Not immediately sure what from that log looks most guilty, though.

With targeted builds, I narrowed the regression range to: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=84f27c157997&tochange=d03022d86ce6 From that range, bug 577607 looks most suspicious -- in particular, its first part, which is about external-resource teardown.

So we're crashing in this function, on the attempted call to Hide(), because aData->mViewer is null. (in fact, all of the pointers in aData are null) 824 static PLDHashOperator 825 ExternalResourceHider(nsIURI* aKey, 826 nsExternalResourceMap::ExternalResource* aData, 827 void* aClosure) 828 { 829 aData->mViewer->Hide(); 830 return PL_DHASH_NEXT; 831 } and that chunk above was indeed added in bug 577607's first patch. FWIW, the ExternalResource aData's pointers are null because it gets set up via this call: 752 if (NS_FAILED(load->StartLoad(clone, aRequestingNode))) { 753 // Make sure we don't thrash things by trying this load again, since 754 // chances are it failed for good reasons (security check, etc). 755 AddExternalResource(clone, nsnull, nsnull, aDisplayDocument); Those "nsnull" pointers there end up being used to initialize aData's member variables. From looking at AddExternalResource, it looks like it's perfectly acceptable to have a null mViewer pointer on an ExternalResource object. So I think we just need to null-check aData->mViewer before calling Hide() on it.

(In reply to comment #6) > So I think we just > need to null-check aData->mViewer before calling Hide() on it. Here's a patch to do that. I haven't tested this fix yet (compiling it with a fresh build right now), but I'll bet this fixes it.

Comment on attachment 466172 [details] [diff] [review] fix v1 Confirmed that this trivial patch fixes the crash, on both the URL & attached testcase. (In reply to comment #6) > From looking at AddExternalResource, it looks like it's perfectly acceptable to > have a null mViewer pointer on an ExternalResource object. So I think we just > need to null-check aData->mViewer before calling Hide() on it. (Note also that the ~ExternalResource() destructor null-checks mViewer before making calls to its methods. So I'm pretty sure this added null-check in ExternalResourceHider is what we want.)