Closed Bug 526497 Opened 15 years ago Closed 8 years ago

Input stream passed into nsIStreamListener.onDataAvailable doesn't implement nsISeekableStream

Categories

(Core :: Networking, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: Honza, Unassigned)

Details

(Whiteboard: [firebug-p2])

Attachments

(1 file)

An inputStream passed into the nsIStreamListener.onDataAvailable method doesn't implement nsISeekableStream interface.

This means that if the stream is read, data must be cloned into
another stream, so other listeners in the chain can also read it. This isn't
good for performance and it also makes the code more complicated.

I have been testing with nsITraceableChannel and nsIStreamListener used to get the response from the server.

Honza
(In reply to comment #30 from Bug 515051)
> the stream is the input end of the pipe. if you want to read the data and also
> make it available to the other listeners, the data will have to be copied
> somewhere in any case. it's not like the entire response is kept in memory.
So, if I understand correctly the data would have to be copied into a new stream, which could be worthless if nobody is actually listening?

Is there any other option how to make reading in this case easier?

Honza
Whiteboard: [firebug-p2]
We could perhaps add an API that lets you read data without consuming it (so that it's still available to other users).
Severity: normal → enhancement
OS: Windows Vista → All
Hardware: x86 → All
But note that you couldn't depend on that API being exposed on the streams that nsIStreamListener gets; at least not without an effective change to the nsIChannel contract...

I'm a little confused by the Firebug use case here.  Is it not solved by an input stream tee?  If not, why not?

That is, do you absolutely have to read the data _before_ calling the next listener in the chain?  Or are you ok with peeking at it as the next listener reads it?
> I'm a little confused by the Firebug use case here.  Is it not solved by an
> input stream tee?  If not, why not?
How should I use the input-stream-tee to simplify the code below?

This is shortened version of the code that is used in nsIStreamListener object beeing set using nsITraceableChannel.

The purpose of the code is to read the input stream (the network response) and also make a copy so, other listeners can read it (also, the request is canceled if there is an exception fired by a listener in the chain).

onDataAvailable: function(request, requestContext, inputStream, offset, count)
{
    var binaryInputStream = CCIN("@mozilla.org/binaryinputstream;1", "nsIBinaryInputStream");
    var storageStream = CCIN("@mozilla.org/storagestream;1", "nsIStorageStream");
    var binaryOutputStream = CCIN("@mozilla.org/binaryoutputstream;1", "nsIBinaryOutputStream");

    binaryInputStream.setInputStream(inputStream);
    storageStream.init(8192, count, null);
    binaryOutputStream.setOutputStream(storageStream.getOutputStream(0));

    var data = binaryInputStream.readBytes(count);
    binaryOutputStream.writeBytes(data, count);

    // NOW: Copy "data" into Firebug's internal cache.

    // Let other listeners use the stream.
    inputStream = storageStream.newInputStream(0);

    if (this.listener)
    {
        try
        {
            this.listener.onDataAvailable(request, requestContext, inputStream, offset, count);
        }
        catch(exc)
        {
            request.cancel(exc.result);
        }
    }
},

With support for nsISeekableStream the code could be as follows:

onDataAvailable: function(request, requestContext, inputStream, offset, count)
{
    // NOW: Read inputStream and copy content into Firebug's internal cache.
    inputStream.seek(nsISeekableStream.NS_SEEK_SET, 0);

    if (this.listener)
    {
        try
        {
            this.listener.onDataAvailable(request, requestContext, inputStream, offset, count);
        }
        catch(exc)
        {
            request.cancel(exc.result);
        }
    }
},


> That is, do you absolutely have to read the data _before_ calling the next
> listener in the chain?  Or are you ok with peeking at it as the next listener
> reads it?
Not sure if I understand this. Firebug can be just one of the listeners in the chain that is created as the nsITraceableChannel.setNewListener is used. If the this.listener.onDataAvailable would be called before the stream is copied, the data would be consumed by Firefox eventually (?) and reading the stream would throw an exception (e.g. NS_BASE_STREAM_WOULD_BLOCK).

At least, this is what I see when running the attached test case.

Honza
Jan, I'm saying that you could instead do something like this:

  onDataAvailable: function(request, ctx, inputStream, offset, count
  {
    var tee = /* create input stream tee; we might need to add a contract for this */
    tee.source = inputStream;
    tee.sink = /* output stream of your choice */
    if (this.listener)
    {
      try
      {
        this.listener.onDataAvailable(request, requestContext, tee,
                                      offset, count);
      }
      catch(exc)
      {
        request.cancel(exc.result);
      }
    }
  }

After which your output stream has had whatever data this.listener read written to it.
comment 6
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.