Closed Bug 197114 Opened 17 years ago Closed 16 years ago

htmlparser should use ReadSegments() to read DTDs

Categories

(Core :: DOM: HTML Parser, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla1.7alpha

People

(Reporter: alecf, Assigned: hjtoi-bugzilla)

Details

(Keywords: memory-footprint)

Attachments

(1 file)

Now that nsIUnicharInputStream supports ReadSegments(), the htmlparser should be
using that to feed data to expat - since nsIUnicharInputStream has an internal
buffer, this will avoid an extra copy.
Status: NEW → ASSIGNED
Keywords: footprint
Target Milestone: --- → mozilla1.4beta
Priority: -- → P2
Target Milestone: mozilla1.4beta → mozilla1.5alpha
Attached patch Potential fixSplinter Review
Alec, is this what you had in mind?

Darin, is this correct/safe use of ReadSegments (I based this on your
comment(s) in bug 11232)? I am wondering if I should be calling available(),
OnDataAvailable and all the dance that nsSyncLoadService.cpp does...
Attachment #139818 - Flags: superreview?(darin)
Attachment #139818 - Flags: review?(alecf)
its been a while, but I think this is what I meant.. this looks ok to me, but
I'd want to hear from darin first...
Comment on attachment 139818 [details] [diff] [review]
Potential fix

>Index: htmlparser/src/nsExpatDriver.cpp


>+NS_METHOD
>+ExternalDTDStreamreaderFunc(nsIUnicharInputStream* aIn,

nit: you should declare this method static:

static NS_METHOD
ExternalDTDStreamreaderFunc(nsIUnicharInputStream* aIn,
...


>+{
>+  // Pass the buffer to expat for parsing. XML_Parse returns 0 for
>+  // fatal errors.
>+  if (XML_Parse((XML_Parser)aClosure, (char *)aFromSegment, 
>+                aCount * sizeof(PRUnichar), 0)) {
>+    *aWriteCount = aCount;
>+    return NS_OK;
>+  } else {
>+    *aWriteCount = 0;
>+    return NS_ERROR_FAILURE;
>+  }
>+}

nit: "else" after "return" is extraneous ;)


sr=darin
Attachment #139818 - Flags: superreview?(darin) → superreview+
Comment on attachment 139818 [details] [diff] [review]
Potential fix

sr=alecf
Attachment #139818 - Flags: review?(alecf) → review+
Taking.
Assignee: alecf → hjtoi-bugzilla
Status: ASSIGNED → NEW
Target Milestone: mozilla1.5alpha → mozilla1.7alpha
Made corrections suggested by Darin and checked in, marking fixed.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.