Bug 1398120: Fix some StreamFilter state handling inconsistencies. r=mixedpuppy
☠☠ backed out by 4ada8f0d5cc0 ☠ ☠
authorKris Maglione <maglione.k@gmail.com>
Thu, 02 Nov 2017 12:27:45 -0700
changeset 443198 209df98be4672182897eb92c2b7b807b2f7901b7
parent 443197 5a05948f281bb49710e69b04d0286ff9c5f39d85
child 443199 504d8384818c4560b8bfa76dbd8f65e60e3b036b
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmixedpuppy
bugs1398120
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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.