Bug 1398120: Fix some StreamFilter state handling inconsistencies. r?mixedpuppy draft
authorKris Maglione <maglione.k@gmail.com>
Thu, 02 Nov 2017 12:27:45 -0700
changeset 692223 891800bb6b81f25842cae60fa5fe7d98eaf46977
parent 690425 95cfe19746a3b03a5ea6ed946a0e3e38d99264e0
child 738710 d20abd6e27d37722f340bba42aa388f3ca82f7f3
push id87447
push usermaglione.k@gmail.com
push dateThu, 02 Nov 2017 19:28:22 +0000
reviewersmixedpuppy
bugs1398120
milestone58.0a1
Bug 1398120: Fix some StreamFilter state handling inconsistencies. r?mixedpuppy MozReview-Commit-ID: 2mLZ9DeqpE0
toolkit/components/extensions/test/mochitest/test_ext_webrequest_responseBody.html
toolkit/components/extensions/webrequest/StreamFilterChild.cpp
--- a/toolkit/components/extensions/test/mochitest/test_ext_webrequest_responseBody.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_webrequest_responseBody.html
@@ -182,34 +182,89 @@ const TASKS = [
           checkState("suspended");
 
           await new Promise(resolve => setTimeout(resolve, TIMEOUT * 3));
 
           checkState("suspended");
           filter.disconnect();
           checkState("disconnected");
 
-          for (let method of ["suspend", "resume", "close"]) {
+          for (let method of ["suspend", "resume", "close", "disconnect"]) {
             browser.test.assertThrows(
               () => {
                 filter[method]();
               },
               /.*/,
               `(${num}): ${method}() should throw while disconnected`);
           }
 
           browser.test.assertThrows(
             () => {
               filter.write(encoder.encode("Foo bar"));
             },
             /.*/,
             `(${num}): write() should throw while disconnected`);
 
+          resolve();
+        }
+      };
+
+      filter.onerror = event => {
+        browser.test.fail(`(${num}): Got unexpected error event: ${filter.error}`);
+      };
+    },
+    verify(response) {
+      is(response, PARTS.join(""), "Got expected final HTML");
+    },
+  },
+  {
+    url: "slow_response.sjs",
+    task(filter, resolve, num) {
+      let encoder = new TextEncoder("utf-8");
+
+      filter.onstop = event => {
+        browser.test.fail(`(${num}): Got unexpected onStop event while disconnected`);
+      };
+
+      let n = 0;
+      filter.ondata = async event => {
+        n++;
+
+        filter.write(event.data);
+
+        if (n == 3) {
+          filter.suspend();
+          await new Promise(resolve => setTimeout(resolve, TIMEOUT));
+          filter.resume();
+          filter.suspend();
+
+          await new Promise(resolve => setTimeout(resolve, TIMEOUT));
+
+          filter.resume();
+          await new Promise(resolve => setTimeout(resolve, 0));
+          filter.suspend();
+
           filter.disconnect();
 
+          for (let method of ["suspend", "resume", "close", "disconnect"]) {
+            browser.test.assertThrows(
+              () => {
+                filter[method]();
+              },
+              /.*/,
+              `(${num}): ${method}() should throw while disconnected`);
+          }
+
+          browser.test.assertThrows(
+            () => {
+              filter.write(encoder.encode("Foo bar"));
+            },
+            /.*/,
+            `(${num}): write() should throw while disconnected`);
+
           resolve();
         }
       };
 
       filter.onerror = event => {
         browser.test.fail(`(${num}): Got unexpected error event: ${filter.error}`);
       };
     },
--- a/toolkit/components/extensions/webrequest/StreamFilterChild.cpp
+++ b/toolkit/components/extensions/webrequest/StreamFilterChild.cpp
@@ -108,16 +108,30 @@ StreamFilterChild::Resume(ErrorResult& a
 
     default:
       aRv.Throw(NS_ERROR_FAILURE);
       return;
     }
     break;
 
   case State::Resuming:
+    switch (mNextState) {
+    case State::Suspending:
+      mNextState = State::Resuming;
+      break;
+
+    case State::TransferringData:
+      break;
+
+    default:
+      aRv.Throw(NS_ERROR_FAILURE);
+      return;
+    }
+    break;
+
   case State::TransferringData:
     break;
 
   default:
     aRv.Throw(NS_ERROR_FAILURE);
     return;
   }
 
@@ -137,31 +151,28 @@ StreamFilterChild::Disconnect(ErrorResul
     WriteBufferedData();
     SendDisconnect();
     break;
 
   case State::Suspending:
   case State::Resuming:
     switch (mNextState) {
     case State::Suspended:
+    case State::Suspending:
     case State::Resuming:
-    case State::Disconnecting:
+    case State::TransferringData:
       mNextState = State::Disconnecting;
       break;
 
     default:
       aRv.Throw(NS_ERROR_FAILURE);
       return;
     }
     break;
 
-  case State::Disconnecting:
-  case State::Disconnected:
-    break;
-
   default:
     aRv.Throw(NS_ERROR_FAILURE);
     return;
   }
 }
 
 void
 StreamFilterChild::Close(ErrorResult& aRv)
@@ -173,17 +184,28 @@ StreamFilterChild::Close(ErrorResult& aR
     mState = State::Closing;
     mNextState = State::Closed;
 
     SendClose();
     break;
 
   case State::Suspending:
   case State::Resuming:
-    mNextState = State::Closing;
+    switch (mNextState) {
+    case State::Suspended:
+    case State::Suspending:
+    case State::Resuming:
+    case State::TransferringData:
+      mNextState = State::Closing;
+      break;
+
+    default:
+      aRv.Throw(NS_ERROR_FAILURE);
+      return;
+    }
     break;
 
   case State::Closing:
     MOZ_DIAGNOSTIC_ASSERT(mNextState == State::Closed);
     break;
 
   case State::Closed:
     break;
@@ -218,16 +240,18 @@ StreamFilterChild::SetNextState()
 
   case State::Closing:
     mNextState = State::Closed;
     SendClose();
     break;
 
   case State::Disconnecting:
     mNextState = State::Disconnected;
+
+    WriteBufferedData();
     SendDisconnect();
     break;
 
   case State::FinishedTransferringData:
     if (mStreamFilter) {
       mStreamFilter->FireEvent(NS_LITERAL_STRING("stop"));
       // We don't need access to the stream filter after this point, so break our
       // reference cycle, so that it can be collected if we're the last reference.