Change XULContentSinkImpl to fail early / avoid leaking element

ASSIGNED
Assigned to

Status

()

Core
XUL
ASSIGNED
8 years ago
8 years ago

People

(Reporter: timeless, Assigned: timeless)

Tracking

Trunk
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

4.50 KB, patch
neil@parkwaycc.co.uk
: review-
Details | Diff | Splinter Review
(Assignee)

Description

8 years ago
// Create the element
     nsXULPrototypeElement* element;
     rv = CreateElement(aNodeInfo, &element);
... 
     // Set the correct script-type for the element.
     rv = SetElementScriptType(element, aAttributes, aAttrLen);
leak element:
     if (NS_FAILED(rv)) return rv;
(Assignee)

Comment 1

8 years ago
Created attachment 465244 [details] [diff] [review]
patch
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #465244 - Flags: review?(neil)
(Assignee)

Comment 2

8 years ago
actually, let's do it a bit more consistently for the file...
Summary: XULContentSinkImpl::OpenRoot leaks element when SetElementScriptType fails → Change XULContentSinkImpl to fail early / avoid leaking element
(Assignee)

Comment 3

8 years ago
Created attachment 465264 [details] [diff] [review]
bigger patch
Attachment #465244 - Attachment is obsolete: true
Attachment #465264 - Flags: review?(neil)
Attachment #465244 - Flags: review?(neil)

Comment 4

8 years ago
Comment on attachment 465264 [details] [diff] [review]
bigger patch

I'm not too keen on either of the variants of failure propagation used here, so I'd rather you didn't change the style. (While there is one instance of /(NS_FAILED(rv))$/ in the file I guess that seems to be against file style.)

Unfortunately this crashed when I tried it.
Attachment #465264 - Flags: review?(neil) → review-
You need to log in before you can comment on or make changes to this bug.