Unnecessary nsAutoString construction in HTMLContentSink::CreateContentObject.

RESOLVED FIXED in mozilla1.3alpha

Status

()

P2
normal
RESOLVED FIXED
16 years ago
16 years ago

People

(Reporter: jst, Assigned: jst)

Tracking

({perf})

Trunk
mozilla1.3alpha
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [HAVE FIX])

Attachments

(1 attachment)

(Assignee)

Description

16 years ago
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

16 years ago
Created attachment 106726 [details] [diff] [review]
Don't create an nsAutoString every time we create a content object.
(Assignee)

Updated

16 years ago
Attachment #106726 - Flags: superreview?(heikki)
Attachment #106726 - Flags: review?(harishd)
(Assignee)

Updated

16 years ago
Status: NEW → ASSIGNED
Keywords: perf
Priority: -- → P2
Whiteboard: [HAVE FIX]
Target Milestone: --- → mozilla1.3alpha

Comment 2

16 years ago
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

16 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.
Attachment #106726 - Flags: superreview?(heikki) → superreview+
(Assignee)

Comment 4

16 years ago
Fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 5

16 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?
You need to log in before you can comment on or make changes to this bug.