499134 - Crash with DOM Inspector [@ inDOMView::AttributeChanged(nsIDocument*, nsIContent*, int, nsIAtom*, int, unsigned int) ]

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Alice0775 White]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090618 Firefox/3.5.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090618 Shiretoko/3.5pre ID:20090618042848

Random crash when click recent used folder drop-down button in Edit Bookmark panel.

Reproducible: Always

Steps to Reproduce:
1.Start Shiretoko with new profile
2.Using it for a while
3.Double click Star button,
4.click recent used folder drop-down button at right side of folder row.
Actual Results:  
Random crash

Expected Results:  
Do not crash

Crash report:
http://crash-stats.mozilla.com/report/index/50b22cc2-a979-42d6-8dfb-b18432090618

Comment 1

[timeless]

You have some sort of inspector extension (firebug? dom inspector?) installed+enabled.

Signature	inDOMView::AttributeChanged(nsIDocument*, nsIContent*, int, nsIAtom*, int, unsigned int)
UUID	50b22cc2-a979-42d6-8dfb-b18432090618
Time 	2009-06-18 06:46:24.991173
Uptime	1245
Last Crash	523939 seconds before submission
Product	Firefox
Version	3.5pre
Build ID	20090618042848
Branch	1.9.1
OS	Windows NT
OS Version	5.1.2600 Service Pack 3
CPU	x86
CPU Info	GenuineIntel family 15 model 2 stepping 7
Crash Reason	EXCEPTION_ACCESS_VIOLATION
Crash Address	0x10
User Comments	
Processor Notes 	
Crashing Thread
Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	inDOMView::AttributeChanged 	layout/inspector/src/inDOMView.cpp:779
1 	xul.dll 	nsNodeUtils::AttributeChanged 	content/base/src/nsNodeUtils.cpp:110
2 	xul.dll 	nsGenericElement::SetAttrAndNotify 	content/base/src/nsGenericElement.cpp:4346
3 	xul.dll 	nsGenericElement::SetAttr 	content/base/src/nsGenericElement.cpp:4277
4 	xul.dll 	nsIContent::SetAttr 	obj-firefox/dist/include/content/nsIContent.h:265
5 	xul.dll 	nsMenuFrame::PopupOpened 	layout/xul/base/src/nsMenuFrame.cpp:582
6 	xul.dll 	nsMenuPopupFrame::ShowPopup 	layout/xul/base/src/nsMenuPopupFrame.cpp:610
7 	xul.dll 	nsXULPopupManager::ShowPopupCallback 	layout/xul/base/src/nsXULPopupManager.cpp:578
8 	xul.dll 	nsXULPopupManager::FirePopupShowingEvent 	layout/xul/base/src/nsXULPopupManager.cpp:1047
9 	xul.dll 	nsXULPopupShowingEvent::Run 	layout/xul/base/src/nsXULPopupManager.cpp:2012
10 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:510
11 	xul.dll 	nsBaseAppShell::Run 	widget/src/xpwidgets/nsBaseAppShell.cpp:170
12 	xul.dll 	nsAppStartup::Run 	toolkit/components/startup/src/nsAppStartup.cpp:193
13 	nspr4.dll 	PR_GetEnv 	
14 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:110
15 	firefox.exe 	firefox.exe@0x21a7 	
16 	kernel32.dll 	kernel32.dll@0x17076

Comment 2

[Alice0775 White]

Oh yes. I have installed DOM Inspector.

Now,I disable DOMi.
I will look at the state for a while.

Comment 3

[timeless]

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/layout/inspector/src&command=DIFF_FRAMESET&file=inDOMView.cpp&rev2=1.42&rev1=1.41 introduced the bug.

Comment 4

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

Would you mind telling us what the bug is and saving the rest of us 5 minutes that you've clearly already spent?

Comment 5

[Alice0775 White]

This crash does not occur more than these past 1 month.
Can anybody change the status into WORKSFORME.

Comment 6

[timeless]

Attachment: patch

In the:
      // if this view has a root node but is not displaying it,
      // it is ok to act as if the changed attribute is on the root.
case, we don't get a contentRow, and we only want to use that row for some ordering. so we just insert the row...

Comment 7

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

Comment on attachment 415818 [details] [diff] [review]
patch

This code looks ok, but I don't understand the patch in bug 345982 at all; could you find somebody who does to review it?

Comment 8

[[e-mail]]

I couldn't figure out how to reproduce this.
If we're not showing element nodes, then we won't find an insert node, since we're appending to a flat list.
If we're showing element nodes, then the element whose attribute is changing will have a row, so that contentNode is not null.
Both cases seem to have a bug in that insertNode could be null when it shouldn't, which would have to be fixed carefully to avoid tripping this issue.

Comment 9

[timeless]

right, all i'm saying is roughly a subset of what you're saying: the current code is buggy. note that i'm not putting significant effort into rewriting (or understanding) this code, if this patch isn't what you want, reassign the bug.

reporter: thanks for the bug report, i'm glad you aren't crashing anymore at this time, but you've pointed out a bug in our code, and we'd kind of like to get rid of such bugs even if you can't hit them today, someone else might hit them tomorrow, and i'd rather avoid that (the someone might be me!).

Comment 10

[Shawn Wilsher :sdwilsh]

Comment on attachment 415818 [details] [diff] [review]
patch

clearing request per comment 8

Comment 11

[Andreas Jung]

I can reproduce a crash with almost the same signature:
@ inDOMView::AttributeChanged(nsIDocument*, nsIContent*, int, nsIAtom*, int)
bp-2a48f474-40f4-4390-89aa-3c24c2100626
bp-a9bf84bc-17ae-43f1-a56a-a675d2100626

Steps to reproduce:
1) Open http://www.w3.org/TR/SVG/images/filters/feColorMatrix.svg
2) Open DOM Inspector
3) right click on the last "text"-element -> Insert -> After
   Node Type:     Element
   Namespace URI: null
   Tag Name:      image
4) right click on attribute panel -> Insert
   Node Name:     x-link:href
   Node Value:    http:\\example.com
   Namespace URI: SVG
5) double click on newly created attribute
6) switch Namespace URI to XML -> OK
7) crash!

Is this the same bug or a different bug?

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a6pre) Gecko/20100625 Minefield/3.7a6pre

Comment 12

[timeless]

Andreas: i'm going to assume it's the same bug and thank you for the steps to reproduce. unfortunately my recollection of this bug is that i was seeking instruction from sdwilsh/neil as to what to do to fix it (beyond my proposed patch). since neil was looking for an understanding of how to reach the crash, hopefully he can use your steps

Comment 13

[[e-mail]]

Comment on attachment 415818 [details] [diff] [review]
patch

Well I think the whole block is bogus but this does at least wallpaper the crash. (In this case somehow the number of attributes tracked by the view disagrees with the actual number of attributes on the node. No idea why that happened though.)

Comment 14

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/427ee19c6233