260281 - nsIUnicharInputStream is not scriptable

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Michiel van Leeuwen (email: mvl+moz@)]

nsIUnicharStreamLoader is scriptable. It wants an nsIUnicharStreamLoaderObserver
to call onStreamComplete. nsIUnicharStreamLoaderObserver is scriptable too.
but onStreamComplete takes an nsIUnicharInputStream, which isn't scriptable. So
if I want to implement the observer in JS, I can't access the inputstream. So I
guess the inputstream should be scriptable too.

Comment 1

[Benjamin Smedberg]

You're going to IDLize it also?

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

IDLizes the file. this could be easier to use from JS, but that'd be rather
inefficient from C++ then, I suppose...

Comment 3

[Boris Zbarsky [:bzbarsky]]

Note that that conflicts with the patch in bug 263973 (not fatally, but some
merging will have to happen somewhere).

Comment 4

[Darin Fisher]

Are you trying to expose nsIUnicharStreamListener because nsIUnicharStreamLoader
uses it?  Hmm... this interface is not very ideal for JS.  I don't think the
read method will work properly.  How about if nsIUnicharStreamLoader returned a
nsIStringEnumerator instead?  We could arrange to have the enumerator
implementation allocate sharable strings (using nsString) such that GetNext
would be fast for C++ callers.

Comment 5

[Boris Zbarsky [:bzbarsky]]

> How about if nsIUnicharStreamLoader returned a nsIStringEnumerator instead?

Then we'd need to write a bunch of glue code to build an nsIUnicharInputStream
out of those for the CSSLoader to use.

Note also that this would be nontrivial because PRUnichars could be split across
segment boundaries in the pipe in the streamloader.  Right now we just handle
that transparently in the unichar stream impl, but if we were returning strings
the loader would have to deal with it...

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #5)
> Note also that this would be nontrivial because PRUnichars could be split across
> segment boundaries in the pipe in the streamloader.  Right now we just handle
> that transparently in the unichar stream impl, but if we were returning strings
> the loader would have to deal with it...

Why? The enumerator impl could deal with it.

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

(and UTF-16 surrogates will make that much more fun, too)

Comment 8

[Boris Zbarsky [:bzbarsky]]

> Why? The enumerator impl could deal with it.

True, but then we still either need to share the code with the existing stream
(difficult, imo) or copy it over (undesirable).

Comment 9

[Darin Fisher]

> Then we'd need to write a bunch of glue code to build an nsIUnicharInputStream
> out of those for the CSSLoader to use.

I was also hoping to give the CSSLoader something better to chew on than a
nsIUnicharInputStream.  What do you think, could the CSSLoader be converted over
to read directly from nsIStringEnumerator instead?

Comment 10

[Boris Zbarsky [:bzbarsky]]

The CSSLoader really doesn't care.  It's the CSSScanner you have to worry about.

Right now the scanner reads data in 256-byte chunks from the stream.  It could
probably be converted to use a string enumerator, but that would require some
hacking to avoid an extra string copy on the read from the enumerator.

Comment 11

[Darin Fisher]

Comment on attachment 163070 [details] [diff] [review]
patch

marking minus because i don't think the read method is actually scriptable.

Comment 12

[Darin Fisher]

i'm not sure that nsIStringEnumerator is best for the CSS scanner... i was
mainly just pondering that idea.  it does seem to me that what would be best for
the CSS scanner is if it could just simply have direct access to the list of
buffers.  reading character at a time from this stream interface isn't great.

it'd also be nice to have a scriptable version of nsIUnicharStreamLoader, but
that means coming up with something that makes both JavaScript consumers and the
CSS scanner happy.

Comment 13

[Christian :Biesinger (don't email me, ping me on IRC)]

ok, how about we make read's signature:
  AString read(unsigned long aCount);
thoughts?

it would force a copy, but the current code does this anyway:
  memcpy(aBuf, us + mPos, sizeof(PRUnichar) * amount);

Comment 14

[Boris Zbarsky [:bzbarsky]]

That could work, with some changes to the CSS scanner...

(I've also been toying with a ReadSegments-like thing on the unichar stream, and
just making the scanner use that, at which point Read() can just do whatever it
wants.)

Comment 15

[Christian :Biesinger (don't email me, ping me on IRC)]

fixed by bug 295047. (including nsIConverterInputStream)