Closed
Bug 180816
Opened 22 years ago
Closed 22 years ago
Unnecessary nsAutoString construction in HTMLContentSink::CreateContentObject.
Categories
(Core :: DOM: Core & HTML, defect, P2)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla1.3alpha
People
(Reporter: jst, Assigned: jst)
Details
(Keywords: perf, Whiteboard: [HAVE FIX])
Attachments
(1 file)
5.01 KB,
patch
|
harishd
:
review+
hjtoi-bugzilla
:
superreview+
|
Details | Diff | Splinter Review |
Every time a content object is created we create an nsAutoString that's only used when creating textarea's, this can be avoided and we'd shave off the over head of creating (and destroying) the nsAutoString object for ever content object that's created. Patch coming up.
Assignee | ||
Comment 1•22 years ago
|
||
Assignee | ||
Updated•22 years ago
|
Attachment #106726 -
Flags: superreview?(heikki)
Attachment #106726 -
Flags: review?(harishd)
Assignee | ||
Updated•22 years ago
|
Status: NEW → ASSIGNED
Keywords: perf
Priority: -- → P2
Whiteboard: [HAVE FIX]
Target Milestone: --- → mozilla1.3alpha
Comment on attachment 106726 [details] [diff] [review] Don't create an nsAutoString every time we create a content object. >+ nsCOMPtr<nsIDOMHTMLTextAreaElement> ta(do_QueryInterface(*aResult)); >+ NS_ASSERTION(ta, "Huh? text area doesn't implement " >+ "nsIDOMHTMLTextAreaElement?"); >+ >+ ta->SetDefaultValue(Substring(start, end)); You've null check here. no? r=harishd
Attachment #106726 -
Flags: review?(harishd) → review+
Assignee | ||
Comment 3•22 years ago
|
||
I decided that the null check was not necessary at that point, we'll never create textarea elements that don't implement nsIDOMHTMLTextAreaElement, if we start doing that, we'll find out soon enough from the assertion. If someone disagrees with me I can add back the null check, but IMNSHO the check is code-bloat we can well live w/o.
Updated•22 years ago
|
Attachment #106726 -
Flags: superreview?(heikki) → superreview+
Assignee | ||
Comment 4•22 years ago
|
||
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 5•22 years ago
|
||
Looking at the patch, a question arised: If the 'skippedContent' is just a CRLF (or CR or LF), then it is not empty, but after skipping the initial CRLF, it becomes empty. Could this happen a lot?, and if so could it save even more by also shaving an nsAutoString for just a 'CRLF' string?
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•