197114 - htmlparser should use ReadSegments() to read DTDs

Status: RESOLVED FIXED

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

Description

[Alec Flett]

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.

Comment 1

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Attachment: Potential fix

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...

Comment 2

[Alec Flett]

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 3

[Darin Fisher]

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

Comment 4

[Alec Flett]

Comment on attachment 139818 [details] [diff] [review]
Potential fix

sr=alecf

Comment 5

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Taking.

Comment 6

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Made corrections suggested by Darin and checked in, marking fixed.