472312 - Trying to open the full-size image on Google images crashed the browser [@ nsSubDocumentFrame::ShowDocShell()]

Status: RESOLVED FIXED

(Core :: Layout: Images, Video, and HTML Frames, defect, P2)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090104 Shiretoko/3.1b3pre ID:20090104020455

I did a search on Google Images and tried to open one of the images directly in full-size by clicking the top link. For now I cannot supply a URL but will try to get the STR in some minutes.

crash report: bp-5337bcb0-0ce3-4282-aa72-579ff2090106

First 10 frames:

0  	XUL  	nsSubDocumentFrame::ShowDocShell  	layout/generic/nsFrameFrame.cpp:914
1 	XUL 	nsSubDocumentFrame::ShowViewer 	layout/generic/nsFrameFrame.cpp:327
2 	XUL 	nsSubDocumentFrame::Init 	layout/generic/nsFrameFrame.cpp:312
3 	XUL 	nsCSSFrameConstructor::InitAndRestoreFrame 	layout/base/nsCSSFrameConstructor.cpp:6782
4 	XUL 	nsCSSFrameConstructor::ConstructHTMLFrame 	layout/base/nsCSSFrameConstructor.cpp:5582
5 	XUL 	nsCSSFrameConstructor::ConstructFrameInternal 	layout/base/nsCSSFrameConstructor.cpp:7556
6 	XUL 	nsCSSFrameConstructor::ConstructFrame 	layout/base/nsCSSFrameConstructor.cpp:7428
7 	XUL 	nsCSSFrameConstructor::ContentAppended 	layout/base/nsCSSFrameConstructor.cpp:8612
8 	XUL 	PresShell::ContentAppended 	layout/base/nsPresShell.cpp:4720
9 	XUL 	nsNodeUtils::ContentAppended 	content/base/src/nsNodeUtils.cpp:119
10 	XUL 	nsContentSink::NotifyAppend 	content/base/src/nsContentSink.cpp:1332

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

The crash happened with the following STR but I'm not able to reproduce ATM.

Steps:
1. Open the given URL
2. Click onto the "See full size image" link while page is loading

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

As what I can see now, this is topcrasher #1 for Firefox 3.1b3pre on all platforms/OS. There is no crash with this top frame listed for 3.1b2.

http://crash-stats.mozilla.com/report/list?range_value=2&range_unit=weeks&version=Firefox%3A3.1b3pre&signature=nsSubDocumentFrame%3A%3AShowDocShell%28%29

The stack on Windows is a bit different:

bp-d1258a98-8530-4b59-ab2c-988802090101

0  	xul.dll  	nsSubDocumentFrame::ShowDocShell  	layout/generic/nsFrameFrame.cpp:995
1 	xul.dll 	nsStyleContext::GetStyleDisplay 	layout/style/nsStyleStructList.h:95
2 	xul.dll 	nsSubDocumentFrame::ShowViewer 	layout/generic/nsFrameFrame.cpp:327
3 	xul.dll 	nsSubDocumentFrame::Init 	layout/generic/nsFrameFrame.cpp:312

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Topcrasher #2 on trunk:

http://crash-stats.mozilla.com/report/list?range_value=2&range_unit=weeks&version=Firefox%3A3.2a1pre&signature=nsSubDocumentFrame%3A%3AShowDocShell%28%29

Comment 4

[Boris Zbarsky [:bzbarsky]]

The steps in comment 1 work for me on mac...

The stack's frame 0 or frame 1 is pretty clearly broken.  I suspect it's the frame 1 that's broken...  The line number in the Windows stack is totally bogus, but the Mac one is the call on the docshell inside ShowDocShell.

The only way to get back a null here is to have no owner content in the frameloader.  Not sure whether that can happen.  smaug?

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comments by users:

3.2a1:
* wandering around my amazon shopping cart.
* two tabs open. one on facebook with an ajax post message type popup and the other at the guardian. Crash happened after I switched to the guardian tab and started loading the page.
*I was adding a book to my wishlist on Amazon.com. This is the third time this has happened in 5 min with 2 seperate books.
* Clicking on an image in amazon.

3.1b3pre:
* Had been using Facebook, and I wrote a note on a friend's wall and Shiretoko crashed
* Clicked "back" on an overly complex ebay page.
* browsing ebay (:

Seems like mostly happen on Amazon and Ebay. I'm also not able to reproduce my given steps from comment 1.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Chris, do you have access to some of the urls given by the crash reports? It would make the life easier to find possible crashing sites.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Haven't managed to reproduce (tested OSX and Linux).
But anyway, looking what could cause this.

Comment 8

[chris hofmann]

henrik,  hoping to have a list of urls soon.  I'll sanitize and post.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thanks Chris! Perhaps you should also have a look at bug 472471. I've already filed a separate bug on the URL retrieval.

Comment 10

[Marcia Knous [:marcia]]

I looked through the crash data on my laptop and found that I crashed in this stack on 12/27 ->http://crash-stats.mozilla.com/report/index/e8dbd91f-44da-46d5-9d03-ad9cb2081227

I recall I was looking at on item on Amazon, clicked the image link, and then I crashed. Since then I have not been able to reproduce the crash.

Comment 11

[chris hofmann]

here is a prelimiary list of urls WHERE
p.product = 'Firefox' AND p.version = '3.2a1pre'
AND '2008-12-28' <= day AND day <= '2009-01-05'
AND sig.signature = 'nsSubDocumentFrame::ShowDocShell()' 

I don't seem to reproduce crashes on any of these using Build identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090105 Shiretoko/3.1b3pre Ubiquity/0.1.4


http://www.demonoid.com/files/

http://www.google.co.uk/ig
http://www.google.com/ig

http://www.vg.no/
http://www.torrents.to/search/210/vue+7.html
http://www.thebostonchannel.com/index.html
http://www.geocities.com/Eureka/1103/ikikalenteri.html
http://www.ebaumsworld.com/pictures/view/80461072/
http://www.amazon.com/gp/cart/view.html/ref=gno_cart
http://slashdot.org/
http://scenereleases.info/category/movies/movies-dvd-rip/page/8
http://newyearseve.morganshotelgroup.com/

http://comics.com/pearls_before_swine/2009-01-02/
http://comics.com/get_fuzzy/2009-01-04/
http://comics.com/get_fuzzy/
http://comics.com/frazz/

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Based on crash reports it is bug 431082 which might have caused this.

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

Attachment: a patch

Boris, I'd like to try this patch. Destroy docshell when unbinding
element or destroying frameloader.
Doesn't regress bug 431082.

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

The patch is backing out part of bug 431082.

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

And passes mochitest etc.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Hrm.  We have no idea why that causes this problem do we?  :(

Could we add an assert for cases when we hit this code or something?  And see why it happens?

Comment 17

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 356013 [details] [diff] [review]
a patch

Please leave a comment that Destroy() is NOT to be called here, with pointer to this bug.  And add that NS_ERROR, please.

Comment 18

[Olli Pettay [:smaug][[e-mail]]]

Attachment: +NS_WARNING

I decided to use NS_WARNING because bug 431082 triggers it.

Comment 19

[Boris Zbarsky [:bzbarsky]]

NS_WARNING won't help much with catching it in other cases (e.g. in other tests Jesse cooks up)....

Comment 20

[chris hofmann]

I've got a longer list of URLs now (~1600).  Is it content from the sites on
these pages that seems to be causing the crash, or some interaction between
windows, or combination, or some other reason entirely?   Here is a rough
summary of sites that had more that 10 instances of crashes for this bug.  We
could drill down more on the specific page content for the sites below, but I'm
guessing that is not needed now.

crash instances - site

 132  http://images.google.com
  55  http://www.amazon.com
  52  http://viewmorepics.myspace.com
  48  http://profile.myspace.com
  33  http://apps.facebook.com
  27  http://www.google.com
  24  http://images.google.ca
  22  http://www.demonoid.com
  21  http://home.myspace.com
  21  http://comics.com
  20  http://www.facebook.com
  16  http://www.imdb.com
  15  http://www.nocturnalemail.biz
  13  http://www.stickam.com
  13  http://images.google.co.uk
  12  http://shop.ebay.com
  12  http://messaging.myspace.com
  12  http://bulletins.myspace.com
  11  http://www.kinopoisk.ru
  11  http://stickam.com
  10  http://www.daler.ru
  10  http://news.yahoo.com

Comment 21

[Olli Pettay [:smaug][[e-mail]]]

Attachment: +NS_ERROR then

Comment 22

[Olli Pettay [:smaug][[e-mail]]]

http://hg.mozilla.org/mozilla-central/rev/324aa9789a12
*Not* marking fixed before results from crash-stats.m.c

Comment 23

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

So all the leak test boxes are red because they're hitting this assertion... I'm going to back out the patch unless you beat me to it.

Comment 24

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Er, actually, only the Linux one; the Windows red was something else.

Comment 25

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

It actually did eventually show up on Mac and Windows as well.

Comment 26

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

And smaug landed http://hg.mozilla.org/mozilla-central/rev/ecb728cdc141 instead of backing out.

Comment 27

[Olli Pettay [:smaug][[e-mail]]]

I change NS_ERROR to NS_WARNING for now. I really need to know if the crash is
fixed in the nightlies.

Comment 28

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

It looks like this did in fact fix the crash, based on:
http://crash-stats.mozilla.com/report/list?product=Firefox&version=Firefox%3A3.2a1pre&query_search=signature&query_type=contains&query=ShowDocShell&date=&range_value=1&range_unit=weeks&do_query=1&signature=nsSubDocumentFrame%3A%3AShowDocShell%28%29
(note that the graph doesn't extend to the present when there are zero crashes for the latest days, so ending on 01/08 is good)

Comment 29

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Indeed. Build 20090108040418 is the latest build on trunk with the crash reported.

Smaug, I don't think that we need another patch for trunk and can mark this bug as fixed?

Comment 30

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 356021 [details] [diff] [review]
+NS_WARNING

Removing approval flag: this is a blocker, and so it doesn't need explicit a+

Comment 31

[Henrik Skupin [:whimboo][⌚️UTC+2]]

There are no more crash reports for builds later than Jan 8th. I believe we can safely mark this bug as fixed.

Comment 32

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Would still be nice to get a testcase somehow so we can add it to the crashtests.

Comment 33

[jag (Peter Annema)]

Is just NS_WARNING enough here? Comment 19 suggests otherwise. Locally I had worked around this by bailing in ShowDocShell if we got a nsnull docShell for an NS_OK return value (should we add an assertion there that we expect docShell never to be nsnull there? Or is crashing sufficient? ;-))