Bug 799234 - Fix seeking within a single stream's range within a multiplexed stream. r=biesi,amarchesini
authorMatthew Gregan <kinetik@flim.org>
Sat, 20 Oct 2012 08:29:41 +1300
changeset 110953 ad953d3aaebee624723e886b68c5d211dfb1bf97
parent 110952 e0ddf5c89441a592d1ed9e846bad8216a478ae08
child 110954 355df2b4546ae66d59399cca7e3e09cc3049d437
push id93
push usernmatsakis@mozilla.com
push dateWed, 31 Oct 2012 21:26:57 +0000
reviewersbiesi, amarchesini
bugs799234
milestone19.0a1
Bug 799234 - Fix seeking within a single stream's range within a multiplexed stream. r=biesi,amarchesini
xpcom/io/nsMultiplexInputStream.cpp
xpcom/tests/unit/test_seek_multiplex.js
--- a/xpcom/io/nsMultiplexInputStream.cpp
+++ b/xpcom/io/nsMultiplexInputStream.cpp
@@ -82,27 +82,40 @@ nsMultiplexInputStream::nsMultiplexInput
 /* readonly attribute unsigned long count; */
 NS_IMETHODIMP
 nsMultiplexInputStream::GetCount(uint32_t *aCount)
 {
     *aCount = mStreams.Length();
     return NS_OK;
 }
 
+static bool
+SeekableStreamAtBeginning(nsIInputStream *aStream)
+{
+    int64_t streamPos;
+    nsCOMPtr<nsISeekableStream> stream = do_QueryInterface(aStream);
+    if (stream && NS_SUCCEEDED(stream->Tell(&streamPos)) && streamPos != 0) {
+        return false;
+    }
+    return true;
+}
+
 /* void appendStream (in nsIInputStream stream); */
 NS_IMETHODIMP
 nsMultiplexInputStream::AppendStream(nsIInputStream *aStream)
 {
+    NS_ASSERTION(SeekableStreamAtBeginning(aStream), "Appended stream not at beginning.");
     return mStreams.AppendElement(aStream) ? NS_OK : NS_ERROR_OUT_OF_MEMORY;
 }
 
 /* void insertStream (in nsIInputStream stream, in unsigned long index); */
 NS_IMETHODIMP
 nsMultiplexInputStream::InsertStream(nsIInputStream *aStream, uint32_t aIndex)
 {
+    NS_ASSERTION(SeekableStreamAtBeginning(aStream), "Inserted stream not at beginning.");
     bool result = mStreams.InsertElementAt(aIndex, aStream);
     NS_ENSURE_TRUE(result, NS_ERROR_OUT_OF_MEMORY);
     if (mCurrentStream > aIndex ||
         (mCurrentStream == aIndex && mStartedReadingCurrent))
         ++mCurrentStream;
     return NS_OK;
 }
 
@@ -373,33 +386,37 @@ nsMultiplexInputStream::Seek(int32_t aWh
                 NS_ENSURE_SUCCESS(rv, rv);
 
                 mCurrentStream = i;
                 mStartedReadingCurrent = remaining != 0;
 
                 remaining = 0;
             }
             else if (remaining > streamPos) {
-                uint64_t avail;
-                rv = mStreams[i]->Available(&avail);
-                NS_ENSURE_SUCCESS(rv, rv);
+                if (i < oldCurrentStream) {
+                    // We're already at end so no need to seek this stream
+                    remaining -= streamPos;
+                    NS_ASSERTION(remaining >= 0, "Remaining invalid");
+                }
+                else {
+                    uint64_t avail;
+                    rv = mStreams[i]->Available(&avail);
+                    NS_ENSURE_SUCCESS(rv, rv);
 
-                int64_t newPos;
-                if (remaining < (streamPos + (int64_t) avail)) {
-                    newPos = remaining - streamPos;
-                    remaining = 0;
-                } else {
-                    newPos = streamPos + (int64_t)avail;
-                    remaining -= streamPos + avail;
+                    int64_t newPos = NS_MIN(remaining, streamPos + (int64_t)avail);
+
+                    rv = stream->Seek(NS_SEEK_SET, newPos);
+                    NS_ENSURE_SUCCESS(rv, rv);
+
+                    mCurrentStream = i;
+                    mStartedReadingCurrent = true;
+
+                    remaining -= newPos;
+                    NS_ASSERTION(remaining >= 0, "Remaining invalid");
                 }
-                rv = stream->Seek(NS_SEEK_CUR, newPos);
-                NS_ENSURE_SUCCESS(rv, rv);
-
-                mCurrentStream = i;
-                mStartedReadingCurrent = true;
             }
             else {
                 NS_ASSERTION(remaining == streamPos, "Huh?");
                 remaining = 0;
             }
         }
 
         return NS_OK;
--- a/xpcom/tests/unit/test_seek_multiplex.js
+++ b/xpcom/tests/unit/test_seek_multiplex.js
@@ -8,167 +8,166 @@ const CC = Components.Constructor;
 const Cc = Components.classes;
 
 // The string we use as data.
 const data = "0123456789";
 // Number of streams in the multiplex stream.
 const count = 10;
 
 function test_multiplex_streams() {
-  try {
-    var MultiplexStream = CC("@mozilla.org/io/multiplex-input-stream;1",
-                             "nsIMultiplexInputStream");
-                             do_check_eq(1, 1);
+  var MultiplexStream = CC("@mozilla.org/io/multiplex-input-stream;1",
+                           "nsIMultiplexInputStream");
+  do_check_eq(1, 1);
 
-    var BinaryInputStream = Components.Constructor("@mozilla.org/binaryinputstream;1",
-                                                   "nsIBinaryInputStream");
-    var BinaryOutputStream = Components.Constructor("@mozilla.org/binaryoutputstream;1",
-                                                    "nsIBinaryOutputStream",
-                                                    "setOutputStream");
-    var multiplex = new MultiplexStream();
-    for (var i = 0; i < count; ++i) {
-      let s = Cc["@mozilla.org/io/string-input-stream;1"]
-                .createInstance(Ci.nsIStringInputStream);
-      s.setData(data, data.length);
+  var BinaryInputStream = Components.Constructor("@mozilla.org/binaryinputstream;1",
+                                                 "nsIBinaryInputStream");
+  var BinaryOutputStream = Components.Constructor("@mozilla.org/binaryoutputstream;1",
+                                                  "nsIBinaryOutputStream",
+                                                  "setOutputStream");
+  var multiplex = new MultiplexStream();
+  for (var i = 0; i < count; ++i) {
+    let s = Cc["@mozilla.org/io/string-input-stream;1"]
+              .createInstance(Ci.nsIStringInputStream);
+    s.setData(data, data.length);
 
-      multiplex.appendStream(s);
-    }
-    var seekable = multiplex.QueryInterface(Ci.nsISeekableStream);
-    var sis = Cc["@mozilla.org/scriptableinputstream;1"]
-                .createInstance(Ci.nsIScriptableInputStream);
-    sis.init(seekable);
-    // Read some data.
-    var readData = sis.read(20);
-    do_check_eq(readData, data + data);
-    // -- Tests for NS_SEEK_SET
-    // Seek accross stream.
-    seekable.seek(Ci.nsISeekableStream.NS_SEEK_SET, 35);
-    do_check_eq(seekable.tell(), 35);
-    do_check_eq(seekable.available(), 65);
-    readData = sis.read(5);
-    do_check_eq(readData, data.slice(5));
-    do_check_eq(seekable.available(), 60);
-    // Seek at stream boundaries.
-    seekable.seek(Ci.nsISeekableStream.NS_SEEK_SET, 40);
-    do_check_eq(seekable.tell(), 40);
-    do_check_eq(seekable.available(), 60);
-    readData = sis.read(10);
-    do_check_eq(readData, data);
-    do_check_eq(seekable.tell(), 50);
-    do_check_eq(seekable.available(), 50);
-    // Rewind and read accross streams.
-    seekable.seek(Ci.nsISeekableStream.NS_SEEK_SET, 39);
-    do_check_eq(seekable.tell(), 39);
-    do_check_eq(seekable.available(), 61);
-    readData = sis.read(11);
-    do_check_eq(readData, data.slice(9) + data);
-    do_check_eq(seekable.tell(), 50);
-    do_check_eq(seekable.available(), 50);
-    // Rewind to the beginning.
-    seekable.seek(Ci.nsISeekableStream.NS_SEEK_SET, 0);
-    do_check_eq(seekable.tell(), 0);
-    do_check_eq(seekable.available(), 100);
-    // Seek to some randome location
-    seekable.seek(Ci.nsISeekableStream.NS_SEEK_SET, 50);
-    // -- Tests for NS_SEEK_CUR
-    // Positive seek.
-    seekable.seek(Ci.nsISeekableStream.NS_SEEK_CUR, 15);
-    do_check_eq(seekable.tell(), 65);
-    do_check_eq(seekable.available(), 35);
-    readData = sis.read(10);
-    do_check_eq(readData, data.slice(5) + data.slice(0, 5));
-    do_check_eq(seekable.tell(), 75);
-    do_check_eq(seekable.available(), 25);
-    // Negative seek.
-    seekable.seek(Ci.nsISeekableStream.NS_SEEK_CUR, -15);
-    do_check_eq(seekable.tell(), 60);
-    do_check_eq(seekable.available(), 40);
-    readData = sis.read(10);
-    do_check_eq(readData, data);
-    do_check_eq(seekable.tell(), 70);
-    do_check_eq(seekable.available(), 30);
+    multiplex.appendStream(s);
+  }
+  var seekable = multiplex.QueryInterface(Ci.nsISeekableStream);
+  var sis = Cc["@mozilla.org/scriptableinputstream;1"]
+              .createInstance(Ci.nsIScriptableInputStream);
+  sis.init(seekable);
+  // Read some data.
+  var readData = sis.read(20);
+  do_check_eq(readData, data + data);
+  // -- Tests for NS_SEEK_SET
+  // Seek to a non-zero, non-stream-boundary offset.
+  seekable.seek(Ci.nsISeekableStream.NS_SEEK_SET, 2);
+  do_check_eq(seekable.tell(), 2);
+  do_check_eq(seekable.available(), 98);
+  seekable.seek(Ci.nsISeekableStream.NS_SEEK_SET, 9);
+  do_check_eq(seekable.tell(), 9);
+  do_check_eq(seekable.available(), 91);
+  // Seek across stream boundary.
+  seekable.seek(Ci.nsISeekableStream.NS_SEEK_SET, 35);
+  do_check_eq(seekable.tell(), 35);
+  do_check_eq(seekable.available(), 65);
+  readData = sis.read(5);
+  do_check_eq(readData, data.slice(5));
+  do_check_eq(seekable.available(), 60);
+  // Seek at stream boundaries.
+  seekable.seek(Ci.nsISeekableStream.NS_SEEK_SET, 40);
+  do_check_eq(seekable.tell(), 40);
+  do_check_eq(seekable.available(), 60);
+  readData = sis.read(10);
+  do_check_eq(readData, data);
+  do_check_eq(seekable.tell(), 50);
+  do_check_eq(seekable.available(), 50);
+  // Rewind and read across streams.
+  seekable.seek(Ci.nsISeekableStream.NS_SEEK_SET, 39);
+  do_check_eq(seekable.tell(), 39);
+  do_check_eq(seekable.available(), 61);
+  readData = sis.read(11);
+  do_check_eq(readData, data.slice(9) + data);
+  do_check_eq(seekable.tell(), 50);
+  do_check_eq(seekable.available(), 50);
+  // Rewind to the beginning.
+  seekable.seek(Ci.nsISeekableStream.NS_SEEK_SET, 0);
+  do_check_eq(seekable.tell(), 0);
+  do_check_eq(seekable.available(), 100);
+  // Seek to some random location
+  seekable.seek(Ci.nsISeekableStream.NS_SEEK_SET, 50);
+  // -- Tests for NS_SEEK_CUR
+  // Positive seek.
+  seekable.seek(Ci.nsISeekableStream.NS_SEEK_CUR, 15);
+  do_check_eq(seekable.tell(), 65);
+  do_check_eq(seekable.available(), 35);
+  readData = sis.read(10);
+  do_check_eq(readData, data.slice(5) + data.slice(0, 5));
+  do_check_eq(seekable.tell(), 75);
+  do_check_eq(seekable.available(), 25);
+  // Negative seek.
+  seekable.seek(Ci.nsISeekableStream.NS_SEEK_CUR, -15);
+  do_check_eq(seekable.tell(), 60);
+  do_check_eq(seekable.available(), 40);
+  readData = sis.read(10);
+  do_check_eq(readData, data);
+  do_check_eq(seekable.tell(), 70);
+  do_check_eq(seekable.available(), 30);
 
-    // -- Tests for NS_SEEK_END
-    // Normal read.
-    seekable.seek(Ci.nsISeekableStream.NS_SEEK_END, -5);
-    do_check_eq(seekable.tell(), data.length * count - 5);
-    readData = sis.read(5);
-    do_check_eq(readData, data.slice(5));
-    do_check_eq(seekable.tell(), data.length * count);
-    // Read accros streams.
-    seekable.seek(Ci.nsISeekableStream.NS_SEEK_END, -15);
-    do_check_eq(seekable.tell(), data.length * count - 15);
-    readData = sis.read(15);
-    do_check_eq(readData, data.slice(5) + data);
-    do_check_eq(seekable.tell(), data.length * count);
+  // -- Tests for NS_SEEK_END
+  // Normal read.
+  seekable.seek(Ci.nsISeekableStream.NS_SEEK_END, -5);
+  do_check_eq(seekable.tell(), data.length * count - 5);
+  readData = sis.read(5);
+  do_check_eq(readData, data.slice(5));
+  do_check_eq(seekable.tell(), data.length * count);
+  // Read across streams.
+  seekable.seek(Ci.nsISeekableStream.NS_SEEK_END, -15);
+  do_check_eq(seekable.tell(), data.length * count - 15);
+  readData = sis.read(15);
+  do_check_eq(readData, data.slice(5) + data);
+  do_check_eq(seekable.tell(), data.length * count);
 
-    // -- Try to do various edge cases
-    // Forward seek from the end, should throw.
-    var caught = false;
-    try {
-      seekable.seek(Ci.nsISeekableStream.NS_SEEK_END, 15);
-    } catch(e) {
-      caught = true;
-    }
-    do_check_eq(caught, true);
-    do_check_eq(seekable.tell(), data.length * count);
-    // Backward seek from the beginning, should be clamped.
-    seekable.seek(Ci.nsISeekableStream.NS_SEEK_SET, 0);
-    do_check_eq(seekable.tell(), 0);
-    seekable.seek(Ci.nsISeekableStream.NS_SEEK_CUR, -15);
-    do_check_eq(seekable.tell(), 0);
-    // Seek too far: shoul be clamped.
-    seekable.seek(Ci.nsISeekableStream.NS_SEEK_SET, 0);
-    do_check_eq(seekable.tell(), 0);
-    seekable.seek(Ci.nsISeekableStream.NS_SEEK_CUR, 3 * data.length * count);
-    do_check_eq(seekable.tell(), 100);
-    seekable.seek(Ci.nsISeekableStream.NS_SEEK_SET, data.length * count);
-    do_check_eq(seekable.tell(), 100);
-    seekable.seek(Ci.nsISeekableStream.NS_SEEK_CUR, -2 * data.length * count);
-    do_check_eq(seekable.tell(), 0);
+  // -- Try to do various edge cases
+  // Forward seek from the end, should throw.
+  var caught = false;
+  try {
+    seekable.seek(Ci.nsISeekableStream.NS_SEEK_END, 15);
   } catch(e) {
-    dump(e + "\n");
+    caught = true;
   }
+  do_check_eq(caught, true);
+  do_check_eq(seekable.tell(), data.length * count);
+  // Backward seek from the beginning, should be clamped.
+  seekable.seek(Ci.nsISeekableStream.NS_SEEK_SET, 0);
+  do_check_eq(seekable.tell(), 0);
+  seekable.seek(Ci.nsISeekableStream.NS_SEEK_CUR, -15);
+  do_check_eq(seekable.tell(), 0);
+  // Seek too far: should be clamped.
+  seekable.seek(Ci.nsISeekableStream.NS_SEEK_SET, 0);
+  do_check_eq(seekable.tell(), 0);
+  seekable.seek(Ci.nsISeekableStream.NS_SEEK_CUR, 3 * data.length * count);
+  do_check_eq(seekable.tell(), 100);
+  seekable.seek(Ci.nsISeekableStream.NS_SEEK_SET, data.length * count);
+  do_check_eq(seekable.tell(), 100);
+  seekable.seek(Ci.nsISeekableStream.NS_SEEK_CUR, -2 * data.length * count);
+  do_check_eq(seekable.tell(), 0);
 }
 
 function test_multiplex_bug797871() {
 
   var data = "1234567890123456789012345678901234567890";
 
-  try {
-    var MultiplexStream = CC("@mozilla.org/io/multiplex-input-stream;1",
-                             "nsIMultiplexInputStream");
-    do_check_eq(1, 1);
+  var MultiplexStream = CC("@mozilla.org/io/multiplex-input-stream;1",
+                           "nsIMultiplexInputStream");
+  do_check_eq(1, 1);
 
-    var BinaryInputStream = Components.Constructor("@mozilla.org/binaryinputstream;1",
-                                                   "nsIBinaryInputStream");
-    var BinaryOutputStream = Components.Constructor("@mozilla.org/binaryoutputstream;1",
-                                                    "nsIBinaryOutputStream",
-                                                    "setOutputStream");
-    var multiplex = new MultiplexStream();
-    let s = Cc["@mozilla.org/io/string-input-stream;1"]
-              .createInstance(Ci.nsIStringInputStream);
-    s.setData(data, data.length);
+  var BinaryInputStream = Components.Constructor("@mozilla.org/binaryinputstream;1",
+                                                 "nsIBinaryInputStream");
+  var BinaryOutputStream = Components.Constructor("@mozilla.org/binaryoutputstream;1",
+                                                  "nsIBinaryOutputStream",
+                                                  "setOutputStream");
+  var multiplex = new MultiplexStream();
+  let s = Cc["@mozilla.org/io/string-input-stream;1"]
+            .createInstance(Ci.nsIStringInputStream);
+  s.setData(data, data.length);
 
-    multiplex.appendStream(s);
+  multiplex.appendStream(s);
 
-    var seekable = multiplex.QueryInterface(Ci.nsISeekableStream);
-    var sis = Cc["@mozilla.org/scriptableinputstream;1"]
-                .createInstance(Ci.nsIScriptableInputStream);
-    sis.init(seekable);
+  var seekable = multiplex.QueryInterface(Ci.nsISeekableStream);
+  var sis = Cc["@mozilla.org/scriptableinputstream;1"]
+              .createInstance(Ci.nsIScriptableInputStream);
+  sis.init(seekable);
 
-    seekable.seek(Ci.nsISeekableStream.NS_SEEK_SET, 8);
-    do_check_eq(seekable.tell(), 8);
-    readData = sis.read(2);
-    do_check_eq(seekable.tell(), 10);
+  seekable.seek(Ci.nsISeekableStream.NS_SEEK_SET, 8);
+  do_check_eq(seekable.tell(), 8);
+  readData = sis.read(2);
+  do_check_eq(seekable.tell(), 10);
 
-    seekable.seek(Ci.nsISeekableStream.NS_SEEK_SET, 20);
-    do_check_eq(seekable.tell(), 20);
-  } catch(e) {
-    do_note_exception(e, "exception in test_multiplex_bug797871");
-  }
+  seekable.seek(Ci.nsISeekableStream.NS_SEEK_SET, 20);
+  do_check_eq(seekable.tell(), 20);
 }
 
 function run_test() {
   test_multiplex_streams();
   test_multiplex_bug797871();
 }