Possible place to head off a common crash [@ nsXBLPrototypeBinding::GetBindingURI]

VERIFIED FIXED in M18

Status

()

P1
critical
VERIFIED FIXED
18 years ago
18 years ago

People

(Reporter: jrgmorrison, Assigned: hyatt)

Tracking

({crash})

Trunk
x86
Windows 2000
crash
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [rtm++], crash signature, URL)

Attachments

(1 attachment)

(Reporter)

Description

18 years ago
Possible place to head off a common crash nsXBLPrototypeBinding::GetBindingURI
but this may just a brain-dead suggestion, oh XBL-fu master.

There is a talkback top crash reported for the following stack:

nsXBLPrototypeBinding::GetBindingURI 
[d:\builds\seamonkey\mozilla\layout\xbl\src\nsXBLPrototypeBinding.cpp, line 
253] 
nsXBLBinding::GetBindingURI 
[d:\builds\seamonkey\mozilla\layout\xbl\src\nsXBLBinding.cpp, line 1081] 
nsXBLService::LoadBindings 
[d:\builds\seamonkey\mozilla\layout\xbl\src\nsXBLService.cpp, line 620] 
nsCSSFrameConstructor::ConstructFrameInternal 
[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp, 
line 7503] 
nsCSSFrameConstructor::ConstructFrame 
[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp, 
line 7466] 
nsCSSFrameConstructor::ProcessChildren 
[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp, 
line 11281] 
nsCSSFrameConstructor::ConstructXULFrame 
[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp, 
line 6179] 
nsCSSFrameConstructor::ConstructFrameInternal 
[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp, 
line 7555] 
nsCSSFrameConstructor::ConstructFrame 
[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp, 
line 7466] 
nsCSSFrameConstructor::ContentInserted 
[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp, 
line 8816] 
nsCSSFrameConstructor::RecreateFramesForContent 
[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp, 
line 11138] 
nsCSSFrameConstructor::AttributeChanged 
[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp, 
line 10087] 
StyleSetImpl::AttributeChanged 
[d:\builds\seamonkey\mozilla\layout\base\src\nsStyleSet.cpp, line 1195] 
PresShell::AttributeChanged 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 3637] 
nsXULDocument::AttributeChanged 
[d:\builds\seamonkey\mozilla\rdf\content\src\nsXULDocument.cpp, line 1643] 
nsXULElement::SetAttribute 
[d:\builds\seamonkey\mozilla\rdf\content\src\nsXULElement.cpp, line 2776] 
nsXULElement::SetAttribute 
[d:\builds\seamonkey\mozilla\rdf\content\src\nsXULElement.cpp, line 2796] 
nsMenuFrame::MarkAsGenerated 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsMenuFrame.cpp, line 500] 
nsMenuBarFrame::SetCurrentMenuItem 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsMenuBarFrame.cpp, line 490] 
nsMenuFrame::HandleEvent 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsMenuFrame.cpp, line 420] 
PresShell::HandleEventInternal 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 4275] 
PresShell::HandleEvent 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 4193] 
nsView::HandleEvent [d:\builds\seamonkey\mozilla\view\src\nsView.cpp, line 379] 
nsViewManager2::DispatchEvent 
[d:\builds\seamonkey\mozilla\view\src\nsViewManager2.cpp, line 1439] 
HandleEvent [d:\builds\seamonkey\mozilla\view\src\nsView.cpp, line 68] 
nsWindow::DispatchEvent 
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 685] 
nsWindow::DispatchWindowEvent 
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 702] 
nsWindow::DispatchMouseEvent 
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 3892] 
ChildWindow::DispatchMouseEvent 
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 4100] 
nsWindow::ProcessMessage 
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 2985] 
nsWindow::WindowProc 
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 951] 
USER32.dll + 0x1820 (0x77e71820) 


The code at the crash is the following:
http://lxr.mozilla.org/seamonkey/source/layout/xbl/src/nsXBLPrototypeBinding.cp
p#246

246 NS_IMETHODIMP 
247 nsXBLPrototypeBinding::GetBindingURI(nsCString& aResult)
248 {
249   nsCOMPtr<nsIXBLDocumentInfo> info;
250   GetXBLDocumentInfo(nsnull, getter_AddRefs(info));
251   
252   info->GetDocumentURI(aResult);    <-- Boom!
253   aResult += "#";
254   aResult += mID;
255   return NS_OK;
256 }
257 

but I don't think the calling code is prepared to deal with having 
an empty string returned for aResult, so can't just null check (I think).

Note that there are a few other code paths (most commonly the one below) by 
which this method is called, although apparently they don't crash on those code 
paths.

Just food for thought.

(The most common other code path ...)
nsXBLPrototypeBinding::GetBindingURI(nsXBLPrototypeBinding * const 0x028d8768, 
nsCString & {...}) line 254
nsXBLBinding::GetBindingURI(nsXBLBinding * const 0x02927808, nsCString & {...}) 
line 1081
nsXBLBinding::InstallProperties(nsXBLBinding * const 0x02927808, nsIContent * 
0x027e5598) line 744
nsXBLService::LoadBindings(nsXBLService * const 0x00dc0ef0, nsIContent * 
0x027e5598, const basic_nsAReadableString<unsigned short> & {...}, int 0, 
nsIXBLBinding * * 0x0012ecd8, int * 0x0012ecc0) line 686
nsCSSFrameConstructor::ConstructFrameInternal(nsIPresShell * 0x00dc74c0, 
nsIPresContext * 0x00d875f8, nsFrameConstructorState & {...}, nsIContent * 
0x027e5598, nsIFrame * 0x02868c04, nsIAtom * 0x00d03660, int 6, nsIStyleContext 
* 0x02901938, nsFrameItems & {...}, int 0) line 7497 + 73 bytes
nsCSSFrameConstructor::ConstructFrame(nsIPresShell * 0x00dc74c0, nsIPresContext 
* 0x00d875f8, nsFrameConstructorState & {...}, nsIContent * 0x027e5598, 
nsIFrame * 0x02868c04, nsFrameItems & {...}) line 7457 + 56 bytes
...
(Assignee)

Comment 1

18 years ago
Easy to bulletproof.  Nominating for rtm.
Status: NEW → ASSIGNED
Whiteboard: rtm
(Reporter)

Comment 2

18 years ago
Oops. 'rtm' nomination goes in the keywords, not in status whiteboard.

If there is a good fix for this, then we should do the bulletproofing.
Keywords: crash, rtm
Whiteboard: rtm

Comment 3

18 years ago
rtm+ need info. I'd expect simple bulletproofing fixes to still be welcomed, as
long as the crash wasn't too obscure.  Bulletproofing topcrashes should be a
no-brainer.  P1/M18
Priority: P3 → P1
Whiteboard: [rtm+ need info]
Target Milestone: --- → M18
(Assignee)

Comment 4

18 years ago
Created attachment 17277 [details] [diff] [review]
Head off crash.

Comment 5

18 years ago
a=waterson

Comment 6

18 years ago
r=saari

Comment 7

18 years ago
Isn't this ready to be marked [rtm+]?
(Assignee)

Comment 8

18 years ago
Yes! PDT bless me bless me bless me!
Whiteboard: [rtm+ need info] → [rtm+]

Comment 9

18 years ago
PDT says rtm++, please land on branch ASAP.
Whiteboard: [rtm+] → [rtm++]
(Assignee)

Comment 10

18 years ago
Fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
(Reporter)

Comment 11

18 years ago
verified bulletproofed trunk and branch.
Status: RESOLVED → VERIFIED
Crash Signature: [@ nsXBLPrototypeBinding::GetBindingURI]
You need to log in before you can comment on or make changes to this bug.