263846 - [FIX]crash if i click on "actueel" above the website. [@ nsImageFrame::FrameChanged][@ nsImageFrame::SourceRectToDest]

Status: VERIFIED FIXED

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

Description

[Peter van der Sanden]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7) Gecko/20040707 Firefox/0.9.2
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7) Gecko/20040707 Firefox/0.9.2

Click on "actueel" above the website http://www.hema.nl
Firefox crashes and Netscape error message pop up.

No idea why.

Reproducible: Always
Steps to Reproduce:
1. visit www.hema.nl
2. click on "actueel" above
3. crash.




After restart FF: All icons in Bookmarks toolbar Folders are removed --> blanc
icon (default)

Comment 1

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.3) Gecko/20041011
Firefox/0.10

WFM

Update to FF1.0PR please

Comment 2

[Nickolay_Ponomarev]

WFM
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20041008
Firefox/0.10

Comment 3

[Tuukka Tolvanen (sp3000)]

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.3) Gecko/20041011 Firefox/0.10
firefox aviary cvs 2004-10-11

crashes with these STR:
1) http://www.hema.nl - wait for full load
2) actueel - wait for full load
3) back button
X) may need to repeat steps 2, 3 once or thrice

talkback reports from fx0.9:
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB1241269Y
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB1241424Z

Comment 4

[Tuukka Tolvanen (sp3000)]

stack between nsImageFrame::FrameChanged and
nsEventQueueImpl::ProcessPendingEvents looks same as bug 191336, but...

Comment 5

[Tuukka Tolvanen (sp3000)]

Attachment: bt full, aviary fx debug

Comment 6

[Tuukka Tolvanen (sp3000)]

Also crashes linux seamonkey 1.8a4 gtk2+xft;
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB1243611Z

Comment 7

[Tuukka Tolvanen (sp3000)]

    There is an
###!!! ASSERTION: Registering a placeholder for a frame that already has a
placeholder!: '!entry->placeholderFrame', file
/home/tt/src/mozilla/AVIARY/mozilla/layout/html/base/src/nsFrameManager.cpp,
line 556
Break: at file
/home/tt/src/mozilla/AVIARY/mozilla/layout/html/base/src/nsFrameManager.cpp,
line 556
    on one of the page loads leading to the crash, but not immediately before
    the crash.

Comment 8

[Peter van der Sanden]

Mozilla/5.0 (Windows; U; Windows NT 5.0; rv:1.7.3) Gecko/20041001 Firefox/0.10.1

With new Firefox 0.10.1: No crash found!

Comment 9

[Ryan Polk (Quark)]

*** Bug 264347 has been marked as a duplicate of this bug. ***

Comment 10

[Ryan Polk (Quark)]

Looks like a pretty common website to crash on according to talkback. 
nsImageFrame::SourceRectToDest is also prevelant.

Comment 11

[Tuukka Tolvanen (sp3000)]

Yeah, I put in nsImageFrame::FrameChanged since that's where the stacks converge.
     14 nsImageFrame::SourceRectToDest
     10 nsStyleContext::GetStyleData
      7 nsImageFrame::ConvertPxRectToTwips
      5 nsImageFrame::FrameChanged
      3 nsRuleNode::GetStyleData
      1 nsFrameManager::GetPropertyListFor

Comment 12

[Ben Fowler]

Attachment: Backtrace for this crash

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a5) Gecko/20041017
Firefox/0.9.1+

I am seeing this; debug build from my tree; backtrace suggests using memory
after free'ing it

Comment 13

[Andrew Schultz]

Attachment: testcase

load, then keep reloading this until it crashes.  Might take 5 reloads.
(tested with linux trunk 2004101606)

Comment 14

[Andrew Schultz]

this started crashing with checkin for bug 233094 although that patch did fix
the layout for the testcase, so it probably just exposed this bug.

Comment 15

[Boris Zbarsky [:bzbarsky]]

The key here is:

(gdb) frame 2
#2  0x412456ce in nsIFrame::GetStyleData(nsStyleStructID) const (this=0x8ac0318, 
    aSID=eStyleStruct_Visibility) at nsIFrame.h:612
612         return mStyleContext->GetStyleData(aSID);
(gdb) p *this
$4 = {<nsISupports> = {_vptr.nsISupports = 0xdadadada}, mRect = {x = -623191334, 
    y = -623191334, width = -623191334, height = -623191334}, mContent =
0xdadadada, 
  mStyleContext = 0xdadadada, mParent = 0xdadadada, mNextSibling = 0xdadadada, 
  mState = 3671775962}

According to botbot, "0xdadadada" is "the MSVC debug-alloc boundary-marker or
PL_FREE_PATTERN"

I'll take a look at the testcase.  Thanks for making one, Andrew!

Comment 16

[Boris Zbarsky [:bzbarsky]]

This is all mine.  The patch to bug 233094 wasn't quite right (and now I find
myself wondering how it actually worked).

Comment 17

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

Comment 18

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 162438 [details] [diff] [review]
Patch

The short story is that we weren't adding the floats to the float list of the
pseudo block right after constructing them.  I don't know how this worked given
that we then pop the float containing block and hence reset the float list in
the state....

In any case, what happened here is that the <table> inside a <span> triggered
WipeContainingBlock(), which destroyed all the newly created frames.  Or rather
tried to.  Since the float wasn't in the anonymous block's child list yet, the
float and all its descendants did NOT get destroyed.

So you end up with an image frame floating about that's not in the frame tree. 
When we tear down the frame tree we don't Destroy() it, but then we reclaim the
PLArena it's allocated from, possibly stomping on its memory (always stomping
with 0xdadadada in debug builds).

The image is animated, which means it makes callbacks to the image frame when
it switches animation frames.  If it does this after the image frame has been
stomped on, we crash.

All this patch does is move InsertOutOfFlow() up near the top of the file so I
can use it in ProcessTableForeignObject(), then makes us use it to insert any
floating descendants of anonymous block.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 162438 [details] [diff] [review]
Patch

Given that bug 233094 landed on branch, I think it's worth taking this as
well...  This is a very safe crash fix.

Comment 20

[Asa Dotzler [:asa]]

Comment on attachment 162438 [details] [diff] [review]
Patch

a=asa for branches checkins.

Comment 21

[Boris Zbarsky [:bzbarsky]]

Fixed on trunk, 1.7 branch, aviary branch.

Comment 22

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED on the trunk using build 2004-11-12-04, Windows XP.

Comment 23

[Olivier Cahagne]

*** Bug 271076 has been marked as a duplicate of this bug. ***