Closed
Bug 703133
Opened 13 years ago
Closed 12 years ago
Firefox Crash @ nsImageLoadingContent::OnStartContainer
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
VERIFIED
FIXED
mozilla12
People
(Reporter: marcia, Assigned: khuey)
References
Details
(Keywords: crash, regression, topcrash, Whiteboard: [qa!])
Crash Data
Attachments
(2 files, 3 obsolete files)
12.01 KB,
text/plain
|
Details | |
3.53 KB,
patch
|
bzbarsky
:
review+
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Seen while looking at new trunk crashes. Signature appears in low volume in Windows and Mac. Trunk crashes started showing up using 20111116030946 build. One comment from Windows crasher: I installed an update, let it autorestart, focused taskbar, opened Skype, focused Firefox and it crashed. Possible pushlog regression range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9ae1d4f44b8b&tochange=086dea3f0bad Not sure if Bug 682077 is involved? Adding Scott in case. https://crash-stats.mozilla.com/report/index/579b3561-a834-42b1-ad6b-b3cf12111116 Frame Module Signature [Expand] Source 0 xul.dll nsImageLoadingContent::OnStartContainer content/base/src/nsImageLoadingContent.cpp:213 1 xul.dll mozilla::TimeStamp::Now xpcom/ds/TimeStamp.cpp:127 2 xul.dll imgRequestProxy::OnStartContainer image/src/imgRequestProxy.cpp:641 3 xul.dll imgStatusTracker::SyncNotify image/src/imgStatusTracker.cpp:230 4 nspr4.dll MD_CURRENT_THREAD nsprpub/pr/src/md/windows/w95thred.c:308 5 mozutils.dll arena_dalloc memory/jemalloc/jemalloc.c:4536 6 xul.dll nsEventQueue::GetEvent xpcom/threads/nsEventQueue.cpp:80 7 nspr4.dll MD_CURRENT_THREAD nsprpub/pr/src/md/windows/w95thred.c:308 8 nspr4.dll PR_Unlock nsprpub/pr/src/threads/combined/prulock.c:347 9 xul.dll nsBaseAppShell::OnProcessNextEvent widget/src/xpwidgets/nsBaseAppShell.cpp:340 10 xul.dll imgRequestNotifyRunnable::Run image/src/imgStatusTracker.cpp:117 11 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:631 12 xul.dll TimerThread::RemoveTimer xpcom/threads/TimerThread.cpp:420 13 xul.dll NS_ProcessNextEvent_P obj-firefox/xpcom/build/nsThreadUtils.cpp:245 14 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:110 15 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:201 16 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:175 17 msvcr90.dll getenv_helper_nolock f:\\dd\\vctools\\crt_bld\\self_64_amd64\\crt\\src\\getenv.c:154 18 xul.dll nsThreadManager::GetCurrentThread xpcom/threads/nsThreadManager.cpp:218 19 xul.dll nsBaseAppShell::Run widget/src/xpwidgets/nsBaseAppShell.cpp:189 20 xul.dll nsAppStartup::Run toolkit/components/startup/nsAppStartup.cpp:228 21 xul.dll XRE_main toolkit/xre/nsAppRunner.cpp:3551
Comment 1•13 years ago
|
||
This is likely a regression from bug 666446. I'll look at it and see what I can do to fix it. Thanks for reporting!
Updated•13 years ago
|
Assignee: nobody → sjohnson
Comment 2•13 years ago
|
||
So, actually, I think I was incorrect in comment 1. bug 666446 doesn't appear to be involved. I will still see if I can track down the push that caused this, though, and hopefully find a fix for it.
No longer blocks: 666446
Comment 3•13 years ago
|
||
Hi Marcia - Could you provide some URLs where this is happening? I'm trying to reproduce and am having difficulty. I'd like to see if it's something that specific to the website, and if so, if I can reliably reproduce it for debugging. Thanks!
Keywords: needURLs
Reporter | ||
Comment 4•13 years ago
|
||
Scott: I asked KaiRo to help out with this request since I am working remotely. (In reply to Scott Johnson (:jwir3) from comment #3) > Hi Marcia - > > Could you provide some URLs where this is happening? I'm trying to reproduce > and am having difficulty. I'd like to see if it's something that specific to > the website, and if so, if I can reliably reproduce it for debugging. > > Thanks!
Comment 5•13 years ago
|
||
(In reply to Marcia Knous [:marcia] from comment #4) > Scott: I asked KaiRo to help out with this request since I am working > remotely. Ok, thanks Marcia!
Comment 6•13 years ago
|
||
URLs from 2011-11-22: 7 \N 2 about:blank 1 http://www.vindictuswiki.com/w/images/5/5d/Queen%27s_Light_Glory_Set_%28View_1%29.png 1 http://www.teekult.ch/components/com_virtuemart/shop_image/product/1304.jpg 1 http://www.tdfast.com/wallpapers_res1/450_10760_4.jpg 1 http://www.hofwat.com/worf.jpg 1 http://www.foxyboxmods.com/pb/wp_b7a43857/images/img157954de0bd3d13734.jpg 1 http://www.foxyboxmods.com/pb/wp_b7a43857/images/img149734de09b53e08f7.JPG 1 http://www.femjoyangels.com/pictures/rachel/warm-up/10.jpg?0 1 http://www.facebook.com/ajax/pagelet/generic.php/PhotoViewerPagelet?... 1 http://www.cameras.co.uk/cameraimages/Olympus-vr-310-outdoors-1-large.jpg 1 http://www.blacksexyshemales.com/wp-content/uploads/2010/06/shemale-01.jpg 1 http://www.asiafuns.com/uploadedimages/new_260154.jpeg 1 http://www.akasa.co.uk/img/product/common/gallery/00/AK-ICR-14_g03.jpg 1 http://thechive.files.wordpress.com/2011/11/diora-baird-5.jpg?w=500 1 https://twimg0-a.akamaihd.net/profile_images/259358523/IMG_3106_2.jpg 1 https://lh4.googleusercontent.com/-C8id55qpfOg/TrrRVf_Ax0I/AAAAAAAAByc/LJPqJdBx2qU/h301/daddy.jpg 1 http://sagakikeita.com/works/56.jpg 1 http://s1-04.twitpicproxy.com/photos/large/453029261.jpg 1 http://pic4all.eu/images/TrickyOldTeacher.11.11.21.Tiffany.XXX.1080p.WMV-SEXORS_1.jpg 1 http://livedoor.blogimg.jp/umashika1209/imgs/b/0/b00888ee.jpg 1 http://livedoor.blogimg.jp/umashika1209/imgs/2/4/2431906c.jpg 1 http://linux-sitebuilder.cp.hostnet.nl/sites/cc/cc6f4f59833b47dc2c3bf17c828439ca/__preview/images/logo_bg.gif?template=av-077&colorScheme=blue&header=headers2&button=buttons3 1 http://indiangirlsclub.com/wp-content/uploads/2011/06/21/1-pakistani-girls-nude-pics.jpg 1 http://img200.imageshack.us/img200/4792/79553666.gif 1 http://images8.simpatie.ro/simpatie/SWEET_ROXXY_7_2135761483.jpg 1 http://images8.simpatie.ro/simpatie/good_girl_kyty_8_1588498259.jpg 1 http://images8.simpatie.ro/simpatie/alexandrytza_1_1835659309.jpg 1 http://images.4chan.org/b/src/1322002079961.png 1 http://images.4chan.org/a/src/1322026697939.png 1 http://images.4chan.org/a/src/1321988744503.jpg 1 http://i.imgur.com/xHIar.jpg 1 http://i.imgur.com/VBsyN.png 1 http://i.imgur.com/oz7aS.png 1 http://i.imgur.com/MSpZm.png 1 http://i.imgur.com/Jy242.png 1 http://i.imgur.com/DVBF9.jpg 1 http://i.imgur.com/dR9DP.jpg 1 http://i.imgur.com/bQi00b.jpg 1 http://i.imgur.com/AkxD9.jpg 1 http://gallys.gayrevenge.com/gr/30/th3.jpg 1 http://forums.vr-zone.com/photopost/data/500/medium/DSC00827.JPG 1 http://forsan.fsky.info/bitrix/templates/1/banner.jpg 1 http://donation.24htremblant.com/Pictures/908Team%2024H.jpg 1 http://cdn-ak.f.st-hatena.com/images/fotolife/F/Factory-T/20080615/20080615172129.jpg 1 http://bit.ly/tGqWOi 1 http://3.bp.blogspot.com/-Vd-hVpvOLYQ/ToGSyRuWlTI/AAAAAAAAKhU/0bXIZUn_boI/s1600/2sdfhdghj.jpg 1 http://3.bp.blogspot.com/-LFPzzgnBrvc/TcmCpOaaklI/AAAAAAAAAUM/1sxMa9n9HH8/s1600/tumblr_l1ls6z5ehc1qzdoopz.jpg
Keywords: needURLs
Comment 7•13 years ago
|
||
Thanks KaiRo.
Comment 8•13 years ago
|
||
i might be reading the crash-stats wrong, but isn't this the second-most crasher on mac-nightlies?
Comment 9•13 years ago
|
||
(In reply to matthias koplenig from comment #8) > i might be reading the crash-stats wrong, but isn't this the second-most > crasher on mac-nightlies? Hm, yes it does look that way.
Comment 10•13 years ago
|
||
I just ran into this, but it's not reproducible. http://hg.mozilla.org/mozilla-central/annotate/3c8147998124/content/base/src/nsImageLoadingContent.cpp#l213 is where it crashes; a null dereference there doesn't make a _ton_ of sense, but I don't know what the lifetime of nsImageLoadingContent listeners is. Adding Boris, who might be able to shed some light.
Comment 11•13 years ago
|
||
Got it on http://imgur.com/YsKly: http://crash-stats.mozilla.com/report/index/bp-d0b3703a-a814-4b01-b563-117382111125
Comment 12•13 years ago
|
||
This seems to only be occurring on image documents from the URLs in comment 6. Jared changed image documents in bug 700854 and bug 700856, which are included in the possible regression range; I'm adding him to see if he has any ideas.
Comment 13•13 years ago
|
||
(In reply to Joe Drew (:JOEDREW!) from comment #12) > This seems to only be occurring on image documents from the URLs in comment > 6. Jared changed image documents in bug 700854 and bug 700856, which are > included in the possible regression range; I'm adding him to see if he has > any ideas. I tried visiting the URLs above but I can't reproduce these crashes. Joe & Benoit: did you have a lot of memory pressure at the time?
Comment 14•13 years ago
|
||
I didn't, I don't think. Certainly, I had only a couple of tabs open.
Comment 15•13 years ago
|
||
I think in one instance I was running a canvas app, but not the other times: http://alteredqualia.com/visualization/evolve/
Assignee | ||
Comment 16•13 years ago
|
||
(In reply to Joe Drew (:JOEDREW!) from comment #10) > I just ran into this, but it's not reproducible. > http://hg.mozilla.org/mozilla-central/annotate/3c8147998124/content/base/src/ > nsImageLoadingContent.cpp#l213 is where it crashes; a null dereference there > doesn't make a _ton_ of sense, but I don't know what the lifetime of > nsImageLoadingContent listeners is. > > Adding Boris, who might be able to shed some light. hg@1 144// Macro to call some func on each observer. This handles observers hg@1 145// removing themselves. hg@1 146#define LOOP_OVER_OBSERVERS(func_) \ hg@1 147 PR_BEGIN_MACRO \ hg@1 148 for (ImageObserver* observer = &mObserverList, *next; observer; \ hg@1 149 observer = next) { \ hg@1 150 next = observer->mNext; \ hg@1 151 if (observer->mObserver) { \ hg@1 152 observer->mObserver->func_; \ hg@1 153 } \ hg@1 154 } \ hg@1 155 PR_END_MACRO There's nothing in there that can be dereferenced without being null checked ... a null dereference there doesn't make any sense.
Comment 17•13 years ago
|
||
I took a quick look myself and I have no idea either. Can we get the raw report from socorro and look at the PC/IP address? With the disassemble we should be able to tell what we were meant to look up.
Comment 18•13 years ago
|
||
Since it doesn't look like this is affected by bug 682077, I'm going to re-push it.
Comment 19•13 years ago
|
||
One thing we might be able to do to determine what is causing this is to temporarily move the implementation of the LOOP_OVER_OBSERVERS macro into the function in question, in order to see which line exactly (and perhaps which variable) is being dereferenced incorrectly. My thinking is that this isn't a NULL crash - that there must be some garbage in one of these pointers (similar to how we do frame poisoning, perhaps?) and it's passing the null check, but it's actually an invalid pointer. The question is why it's getting set to this invalid value in the first place...
Comment 20•13 years ago
|
||
This sounds like the reproducible crash i was investigating in bug 376997. Specifically see comment 74 and 75.
Comment 21•13 years ago
|
||
(In reply to Timothy Nikkel (:tn) from comment #20) > This sounds like the reproducible crash i was investigating in bug 376997. > Specifically see comment 74 and 75. I've confirmed this is the same crash. To reproduce: 1) Pull code from rev 1363c893427c from Oct. 18. 2) Push the patch in this attachment on top of this rev. 3) Compile. 4) Run this mochitest: dom/tests/mochitest/dom-level2-html/test_HTMLFrameElement01.html. It should exhibit this crash. Now, I'll work on tracing this back and figuring out where the pointer is getting set to this invalid value. Thanks to Timothy for walking me through how to reproduce this!
Comment 22•13 years ago
|
||
Hm, so I think I found what is going on here. Somehow, ImageObserver* next and ImageObserver* observer are set to the same address: 214 for (ImageObserver* observer = &mObserverList, *next = nsnull; (gdb) n 216 next = observer->mNext; (gdb) p next $1 = (nsImageLoadingContent::ImageObserver *) 0x7f07e1614c10 (gdb) p observer $2 = (nsImageLoadingContent::ImageObserver *) 0x7f07e1614c10 I'm not quite sure how this is happening, but when the line next = observer->mNext is hit, somehow, it sets both pointers to an invalid value, since observer->mNext is invalid. If I make the change so that the code looks like this: ImageObserver* observer = &mObserverList; ImageObserver* next = nsnull; while (observer) { next = observer->mNext; if (observer->mObserver) { observer->mObserver->OnStartContainer(aRequest, aContainer); } observer = next; } instead of this: for (ImageObserver* observer = &mObserverList, *next; observer; observer = next) { next = observer->mNext; if (observer->mObserver) { observer->mObserver->OnStartContainer(aRequest, aContainer); } } Then, the crash doesn't happen, even though the code should be the same. I think there is something subtle going on here in how these two (seemingly identical) code blocks are compiled.
Comment 23•12 years ago
|
||
We should probably track this for Fx11, as it's pretty bad, and a regression. I can reproduce it consistently by installing Reddit Enhancement Suite <http://reddit.honestbleeps.com/>, then clicking "expand image" on a Reddit comment such as <http://www.reddit.com/r/gaming/comments/n8nsf/image_from_current_thread_on_v_thought_it_might/c375r44>, then closing that image and clicking on the image link itself (which loads it in a tab). Immediate crash.
tracking-firefox11:
--- → ?
Comment 24•12 years ago
|
||
(In reply to Joe Drew (:JOEDREW!) from comment #23) > We should probably track this for Fx11, as it's pretty bad, and a regression. > > I can reproduce it consistently by installing Reddit Enhancement Suite > <http://reddit.honestbleeps.com/>, then clicking "expand image" on a Reddit > comment such as > <http://www.reddit.com/r/gaming/comments/n8nsf/ > image_from_current_thread_on_v_thought_it_might/c375r44>, then closing that > image and clicking on the image link itself (which loads it in a tab). > Immediate crash. Joe, can you see if this patch alleviates the crash for you?
Reporter | ||
Updated•12 years ago
|
Keywords: reproducible
Comment 25•12 years ago
|
||
That patch doesn't fix the crash, no.
Since I've been seeing this crash a lot viewing standalone images, I added the following assertion patch to me tree: https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/83e81a185e6f/img-notify-assert since this looked like a crash-on-unwind. In other words, the current notification code is set up to tolerate the removal of the observer being notified, but it's not ok with *other* observers being removed during the notification, and adding observers during a notification isn't great either. In any case, just now I hit the crash, and got these stacks. They've got some funny frames in them, but I think they show the point of what's going on.
MediaDocument::GetFileName really does seem like the wrong time to call SetDocumentCharacterSet. Could we have done that earlier?
This is probably a regression from bug 700854, but I really blame bug 199237.
I suppose one possibility might be to just remove the SetDocumentCharacterSet(docCharset) call in MediaDocument::GetFileName and leave everything else as-is. This code seems a bit wacky to me, but perhaps it makes more sense to somebody else.
Comment 30•12 years ago
|
||
Hrm. There's other wackiness here too: 1) Why does SetDocumentCharacterSet do a _sync_ style reconstruct? 2) nsImageLoadingContent should use nsTObserverArray, I think. It'd work much better.
Comment 31•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #30) > Hrm. There's other wackiness here too: > > 1) Why does SetDocumentCharacterSet do a _sync_ style reconstruct? The only reason to anything with style at all is performing language-sensitive font fallback with the root language inferred from the encoding, right? I don't see why that needs to be sync.
Comment 32•12 years ago
|
||
This is our #2 crash on the trunk. I think it's worthy of the top crash keyword for tracking.
Keywords: topcrash
I was noticing this crash about once a day before I put this patch in my tree; I haven't seen it since. I don't really see why this is here -- if we don't SetDocumentCharacterSet presumably we'd just go through the same code path the next time around. But I'm not really an expert on this code, either. Thoughts?
Attachment #583784 -
Flags: review?(bzbarsky)
Comment 34•12 years ago
|
||
> I don't really see why this is here The CVS blame says: 1.7 <jshin@mailaps.org> 2003-07-25 22:17 bug 199237 : fix the rendering of the non-ASCII title of a non-text doc(image, media) opened in a new tab/window (r=bryner, sr=bz) So we really do want the charset set at some point. The question is whether we can make it async.
Comment 35•12 years ago
|
||
Comment on attachment 583784 [details] [diff] [review] remove the SetDocumentCharacterSet call I don't think this is right....
Attachment #583784 -
Flags: review?(bzbarsky) → review-
Comment 36•12 years ago
|
||
But maybe it's ok if no one actually looks at the document character set?
Comment 37•12 years ago
|
||
I'm getting this on any image, regardless of url if I right click the image and select "View Image" from the context menu on a webpage.
Comment 38•12 years ago
|
||
Reassigning to default as I'm not actively working on this. Feel free to take. :)
Assignee: sjohnson → nobody
Comment 39•12 years ago
|
||
Is there a possibility that the patch in bug 708431 could fix this? The patch there also includes a fix for ImageDocument.
Updated•12 years ago
|
Comment 40•12 years ago
|
||
Can we get this assigned to somebody else? It's a reproducible regression and top crash.
(In reply to Boris Zbarsky (:bz) from comment #36) > But maybe it's ok if no one actually looks at the document character set? I don't see why anything would care what the document character set for an image is other than the code that gets the filename to display... i.e., this code.
Comment 42•12 years ago
|
||
Johnny - could we find somebody to take a quick look at this in the 11 Aurora timeframe? Thanks!
Comment 43•12 years ago
|
||
I can't reproduce this crash using my steps in comment 23 any more, with or without the patch on bug 708431. :(
Comment 44•12 years ago
|
||
(Note: on Nightly.)
Comment 46•12 years ago
|
||
(In reply to David Baron [:dbaron] from comment #41) > I don't see why anything would care what the document character set for an > image is other than the code that gets the filename to display... i.e., this > code. Save as code might, also for filenames.... Kyle, you may want to see comment 30.
Comment 47•12 years ago
|
||
Still crashes for me. Clicking on images on wikipedia triggers the bug quickly. Program received signal SIGSEGV, Segmentation fault. 0x00007ffff5e38413 in nsImageLoadingContent::OnStartContainer(imgIRequest*, imgIContainer*) (this=0x7fffdaecf430, aRequest=0x7fffd2ee3e40, aContainer=0x7fffd2e24e30) at /var/tmp/mozilla-central/content/base/src/nsImageLoadingContent.cpp:213 213 LOOP_OVER_OBSERVERS(OnStartContainer(aRequest, aContainer)); (gdb) bt #0 0x00007ffff5e38413 in nsImageLoadingContent::OnStartContainer(imgIRequest*, imgIContainer*) (this=0x7fffdaecf430, aRequest=0x7fffd2ee3e40, aContainer=0x7fffd2e24e30) at /var/tmp/mozilla-central/content/base/src/nsImageLoadingContent.cpp:213 #1 0x00007ffff5d8061a in imgRequestProxy::OnStartContainer (this=0x7fffd2ee3e40, image=0x7fffd2e24e30) at /var/tmp/mozilla-central/image/src/imgRequestProxy.cpp:641 #2 0x00007ffff5fabf02 in imgStatusTracker::SyncNotify (this=0x7fffd44a4da0, proxy=0x7fffd2ee3e40) at /var/tmp/mozilla-central/image/src/imgStatusTracker.cpp:230 #3 0x00007ffff5fac095 in imgRequestNotifyRunnable::Run (this=0x7fffd10ff0a0) at /var/tmp/mozilla-central/image/src/imgStatusTracker.cpp:117 #4 0x00007ffff61de972 in nsThread::ProcessNextEvent (this=0x7ffff71257c0, mayWait=<optimized out>, result=0x7fffffffa00f) at /var/tmp/mozilla-central/xpcom/threads/nsThread.cpp:660 #5 0x00007ffff611e237 in NS_ProcessNextEvent_P (thread=<optimized out>, mayWait=false) at /var/tmp/mozilla-central/moz-build-dir/xpcom/build/nsThreadUtils.cpp:245 #6 0x00007ffff6309a93 in mozilla::ipc::MessagePump::Run (this=0x7ffff71b4700, aDelegate=0x7ffff71e40b0) at /var/tmp/mozilla-central/ipc/glue/MessagePump.cpp:110 #7 0x00007ffff5d1708f in RunInternal (this=<optimized out>) at /var/tmp/mozilla-central/ipc/chromium/src/base/message_loop.cc:208 #8 MessageLoop::Run (this=<optimized out>) at /var/tmp/mozilla-central/ipc/chromium/src/base/message_loop.cc:201 #9 0x00007ffff6315a63 in nsBaseAppShell::Run (this=0x7fffef5053c0) at /var/tmp/mozilla-central/widget/src/xpwidgets/nsBaseAppShell.cpp:189 #10 0x00007ffff5dd0647 in nsAppStartup::Run (this=0x7fffef578bf0) at /var/tmp/mozilla-central/toolkit/components/startup/nsAppStartup.cpp:220 #11 0x00007ffff669ad6a in XRE_main (argc=<optimized out>, argv=<optimized out>, aAppData=<optimized out>) at /var/tmp/mozilla-central/toolkit/xre/nsAppRunner.cpp:3523 #12 0x00000000004066cb in do_main (argv=0x7fffffffe5d8, argc=1, exePath=0x7fffffffa4c0 "/var/tmp/mozilla-central/moz-build-dir/dist/bin/") at /var/tmp/mozilla-central/browser/app/nsBrowserApp.cpp:201 #13 main (argc=<optimized out>, argv=<optimized out>) at /var/tmp/mozilla-central/browser/app/nsBrowserApp.cpp:287
Comment 49•12 years ago
|
||
In 12.0a1, it's #4 top crasher on Windows or Linux and #1 on Mac OS X. But in 11.0a2 on Windows, it's only #144 top crasher (#1 on Mac OS X and #6 on Linux)!
Summary: Firefox Crash @ nsImageLoadingContent → Firefox Crash @ nsImageLoadingContent::OnStartContainer
Whiteboard: [STR in comment 23]
Comment 50•12 years ago
|
||
Have these crashes in Nightly gone down since the patch for bug 708431 landed (Jan 8, 2012)? It contained a fix for ImageDocument.cpp and VideoDocument.cpp.
Comment 51•12 years ago
|
||
(In reply to Jared Wein [:jwein and :jaws] from comment #50) > Have these crashes in Nightly gone down since the patch for bug 708431 > landed (Jan 8, 2012)? Those crashes fluctuate across builds so it's too soon to conclude but I'd say it has no impact: 23 crashes in 12.0a1/20120109 against about 30 crashes on average per build. See the graph tab in: https://crash-stats.mozilla.com/report/list?product=Firefox&version=Firefox%3A12.0a1&query_search=signature&query_type=contains&query=nsImageLoadingContent%3A%3AOnStartContainer%28imgIRequest*%2C%20imgIContainer*%29&reason_type=contains&range_value=4&range_unit=weeks&hang_type=any&process_type=any&do_query=1&signature=nsImageLoadingContent%3A%3AOnStartContainer%28imgIRequest*%2C%20imgIContainer*%29
Comment 52•12 years ago
|
||
Thanks scoobidiver. I added few numbers from chofmann's daily reports.... Jan 09, 2012 - 37 crashes Jan 08, 2012 - 56 crashes Jan 07, 2012 - 33 crashes Jan 06, 2012 - (couldn't get data) Jan 05, 2012 - 59 crashes Jan 04, 2012 - 36 crashes
Assignee | ||
Updated•12 years ago
|
Keywords: reproducible → testcase-wanted
Whiteboard: [STR in comment 23]
Assignee | ||
Updated•12 years ago
|
Attachment #580942 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Attachment #577756 -
Attachment is obsolete: true
Assignee | ||
Comment 53•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #30) > Hrm. There's other wackiness here too: > > 1) Why does SetDocumentCharacterSet do a _sync_ style reconstruct? Because the prescontext observers for character set changes and does a sync style reconstruct: http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresContext.cpp#1110 This was added in Bug 103282. This seems like the kind of thing we could do off a runnable. > 2) nsImageLoadingContent should use nsTObserverArray, I think. It'd work > much better. Yeah, it should. I'm not sure that that is sufficient for nsImageLoadingContent to be safe for reentrancy though.
Assignee | ||
Comment 54•12 years ago
|
||
Currently percolating through try.
Attachment #583784 -
Attachment is obsolete: true
Comment 55•12 years ago
|
||
Comment on attachment 587683 [details] [diff] [review] Do reflow off a runnable One question I had was whether we need the sync style data rebuild at all (even off the event) or whether we can just use the existing PostRebuildAllStyleDataEvent mechanism.
Assignee | ||
Comment 56•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #55) > Comment on attachment 587683 [details] [diff] [review] > Do reflow off a runnable > > One question I had was whether we need the sync style data rebuild at all > (even off the event) or whether we can just use the existing > PostRebuildAllStyleDataEvent mechanism. I have no idea. Doing the UpdateCharset call at the same time as the style data rebuild seems less likely to break things than doing the UpdateCharset call immediately and doing the style data rebuild later.
Assignee | ||
Comment 57•12 years ago
|
||
Comment on attachment 587683 [details] [diff] [review] Do reflow off a runnable This passed try on the platforms it built on (still need to figure out some gcc weirdness) but the idea is evident enough.
Attachment #587683 -
Flags: review?(bzbarsky)
Comment 58•12 years ago
|
||
Comment on attachment 587683 [details] [diff] [review] Do reflow off a runnable >+ CharSetChangingRunnable(nsPresContext* aPresContext, >+ nsCString& aCharSet) aCharSet should be |const nsCString|. Surprised that compiled. r=me modulo that
Attachment #587683 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 59•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #58) > Comment on attachment 587683 [details] [diff] [review] > Do reflow off a runnable > > >+ CharSetChangingRunnable(nsPresContext* aPresContext, > >+ nsCString& aCharSet) > > aCharSet should be |const nsCString|. Surprised that compiled. > > r=me modulo that It doesn't on gcc, and that might be why! /me mumbles about traveling and not having his normal machines.
Comment 60•12 years ago
|
||
> aCharSet should be |const nsCString|.
Or more precisely |const nsCString&|
Assignee | ||
Comment 61•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7c7d2a8db7ff Marcia, et al.: Can QA watch crashstats to see if this drops off?
Assignee | ||
Comment 62•12 years ago
|
||
Not a single report on the 1/12 nightly so far.
Comment 63•12 years ago
|
||
I was able to reproduce this crash extremely reliably until 2012-1-11. With 2012-1-12 it indeed seems to be gone. Great, thank you very much.
Assignee | ||
Comment 64•12 years ago
|
||
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #62) > Not a single report on the 1/12 nightly so far. And not one on any later nightlies. I think we can call this fixed.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
Assignee | ||
Updated•12 years ago
|
Attachment #587683 -
Flags: approval-mozilla-aurora?
Updated•12 years ago
|
Status: RESOLVED → VERIFIED
Comment 65•12 years ago
|
||
Comment on attachment 587683 [details] [diff] [review] Do reflow off a runnable [Triage Comment] Top crash, and had some time to bake on m-c. Let's uplift to Aurora.
Attachment #587683 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 66•12 years ago
|
||
Call me selfish (having just had three crashes from this), but any news on uplift? :D
Updated•12 years ago
|
status-firefox11:
--- → affected
Assignee | ||
Comment 67•12 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/bee0171b9597
Comment 68•12 years ago
|
||
Marking verified for Firefox 11 since there have been no crash reports in Beta.
Whiteboard: [qa!]
Comment 69•12 years ago
|
||
This behavior was still present in v13 and is now present in v14 (just updated today, 14.0.1). Pasting an image URL into the address bar and pressing Enter, clicking "View Image" in a context menu, or just clicking a direct link to a JPG (cf. Google Image Search results) will result in a crash. This never happens immediately after the browser is launched, but after trying to directly view an image a few times it will inevitably recur. I've submitted Crash Reports numerous times in the past few weeks.
Comment 70•12 years ago
|
||
You should probably file a new bug with those steps to reproduce.
Comment 71•12 years ago
|
||
Sure, new bug report created: https://bugzilla.mozilla.org/show_bug.cgi?id=781566 Ironically all my crash reports point to this bug as "Related"
Updated•9 years ago
|
Keywords: testcase-wanted
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•