Bug 1418430. P2 - simplify the if statement of "reopen on error". draft
authorJW Wang <jwwang@mozilla.com>
Wed, 22 Nov 2017 11:21:57 +0800
changeset 701671 19b4ab022ab7b68b66f50e61afd313137f8baec4
parent 701670 1ad9f57e48da14be376174aa2a2c3eb123b2cc03
child 702369 103f5e6ad9f7212cfb5e33da96dc1da3b366cd1a
push id90231
push userjwwang@mozilla.com
push dateWed, 22 Nov 2017 03:44:14 +0000
bugs1418430
milestone59.0a1
Bug 1418430. P2 - simplify the if statement of "reopen on error". A truth table is listed to illustrate all conditions of length/offset/reopen. The original code doesn't handle the following cases correctly: 1. length == offset == 0, shouldn't reopen the channel for there is no data to download. 2. length == -1 && offset > 0, should reopen the channel if seekable. MozReview-Commit-ID: IisnrA8hK4M
dom/media/MediaCache.cpp
--- a/dom/media/MediaCache.cpp
+++ b/dom/media/MediaCache.cpp
@@ -2140,21 +2140,35 @@ MediaCacheStream::NotifyDataEndedInterna
 
   // Note that aStatus might have succeeded --- this might be a normal close
   // --- even in situations where the server cut us off because we were
   // suspended. It is also possible that the server sends us fewer bytes than
   // requested. So we need to "reopen on error" in that case too. The only
   // cases where we don't need to reopen are when *we* closed the stream.
   // But don't reopen if we need to seek and we don't think we can... that would
   // cause us to just re-read the stream, which would be really bad.
+  /*
+   * | length |    offset |   reopen |
+   * +--------+-----------+----------+
+   * |     -1 |         0 |      yes |
+   * +--------+-----------+----------+
+   * |     -1 |       > 0 | seekable |
+   * +--------+-----------+----------+
+   * |      0 |         X |       no |
+   * +--------+-----------+----------+
+   * |    > 0 |         0 |      yes |
+   * +--------+-----------+----------+
+   * |    > 0 | != length | seekable |
+   * +--------+-----------+----------+
+   * |    > 0 | == length |       no |
+   */
   if (aReopenOnError && aStatus != NS_ERROR_PARSED_DATA_CACHED &&
       aStatus != NS_BINDING_ABORTED &&
-      (mChannelOffset == 0 ||
-       (mStreamLength > 0 && mChannelOffset != mStreamLength &&
-        mIsTransportSeekable))) {
+      (mChannelOffset == 0 || mIsTransportSeekable) &&
+      mChannelOffset != mStreamLength) {
     // If the stream did close normally, restart the channel if we're either
     // at the start of the resource, or if the server is seekable and we're
     // not at the end of stream. We don't restart the stream if we're at the
     // end because not all web servers handle this case consistently; see:
     // https://bugzilla.mozilla.org/show_bug.cgi?id=1373618#c36
     mClient->CacheClientSeek(mChannelOffset, false);
     return;
     // Note CacheClientSeek() will call Seek() asynchronously which might fail