Backed out 2 changesets (bug 1394102) for wpt failures on sw.https.window.html. CLOSED TREE
authorBrindusan Cristian <cbrindusan@mozilla.com>
Thu, 28 Jun 2018 20:08:54 +0300
changeset 812177 9115a2685cb0ceb92a8eb5bff7b4a6a00af35bbb
parent 812139 bad873c0dbbe1c34582ba3ceefd58c7be7eecd8e
child 812178 2259143ecb07731b4b3c8eca8df5024251ffaf97
push id114476
push userbmo:mtabara@mozilla.com
push dateThu, 28 Jun 2018 18:08:54 +0000
bugs1394102
milestone63.0a1
backs outbad873c0dbbe1c34582ba3ceefd58c7be7eecd8e
afbadca69031d7d465e53bf4e9d8fd95ea33bc10
Backed out 2 changesets (bug 1394102) for wpt failures on sw.https.window.html. CLOSED TREE Backed out changeset bad873c0dbbe (bug 1394102) Backed out changeset afbadca69031 (bug 1394102)
dom/serviceworkers/ServiceWorkerPrivate.cpp
testing/web-platform/tests/fetch/api/abort/serviceworker-intercepted.https.html
testing/web-platform/tests/fetch/api/resources/sw-intercept.js
--- a/dom/serviceworkers/ServiceWorkerPrivate.cpp
+++ b/dom/serviceworkers/ServiceWorkerPrivate.cpp
@@ -6,17 +6,16 @@
 
 #include "ServiceWorkerPrivate.h"
 
 #include "ServiceWorkerCloneData.h"
 #include "ServiceWorkerManager.h"
 #include "nsContentUtils.h"
 #include "nsICacheInfoChannel.h"
 #include "nsIHttpChannelInternal.h"
-#include "nsIHttpProtocolHandler.h"
 #include "nsIHttpHeaderVisitor.h"
 #include "nsINamed.h"
 #include "nsINetworkInterceptController.h"
 #include "nsIPushErrorReporter.h"
 #include "nsISupportsImpl.h"
 #include "nsITimedChannel.h"
 #include "nsIUploadChannel2.h"
 #include "nsNetUtil.h"
@@ -112,242 +111,16 @@ ServiceWorkerPrivate::~ServiceWorkerPriv
   MOZ_ASSERT(!mInfo);
   MOZ_ASSERT(mSupportsArray.IsEmpty());
 
   mIdleWorkerTimer->Cancel();
 }
 
 namespace {
 
-class AbortChannelObserver;
-
-// Simple runnable that calls AbortChannelObserver::MaybeAbortAndReleaseSignal()
-// on the worker thread.
-class AbortSignalRunnable : public WorkerRunnable
-{
-  RefPtr<AbortChannelObserver> mObserver;
-  bool mSignalToAbort;
-
-public:
-  AbortSignalRunnable(WorkerPrivate* aWorkerPrivate,
-                      AbortChannelObserver* aObserver,
-                      bool aSignalToAbort)
-    : WorkerRunnable(aWorkerPrivate)
-    , mObserver(aObserver)
-    , mSignalToAbort(aSignalToAbort)
-  {
-    MOZ_ASSERT(mObserver);
-  }
-
-  bool
-  WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override;
-};
-
-// This object observes the terminations of nsIChannel objects. When |mChannel|
-// is terminated, the AbortSignal object, owned by AbortSignalWorkerHolder, is
-// aborted on the worker thread.  This object is kept alive by the observer and
-// by FetchEventRunnable until the AbortSignal is created on the the worker
-// thread.
-class AbortChannelObserver final : public nsIObserver
-{
-public:
-  NS_DECL_THREADSAFE_ISUPPORTS
-
-  static already_AddRefed<AbortChannelObserver>
-  Create(nsIChannel* aChannel)
-  {
-    MOZ_ASSERT(aChannel);
-    AssertIsOnMainThread();
-
-    RefPtr<AbortChannelObserver> observer =
-      new AbortChannelObserver(aChannel);
-
-    // Let's use NS_HTTP_ON_STOP_REQUEST_TOPIC to know when the channel is
-    // released: maybe we have to abort the AbortSignal object.
-    nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
-    if (NS_WARN_IF(!obs)) {
-      return nullptr;
-    }
-
-    if (NS_FAILED(obs->AddObserver(observer, NS_HTTP_ON_STOP_REQUEST_TOPIC,
-                                   false))) {
-      return nullptr;
-    }
-
-    return observer.forget();
-  }
-
-  NS_IMETHOD
-  Observe(nsISupports* aSubject, const char* aTopic,
-          const char16_t* aData) override
-  {
-    AssertIsOnMainThread();
-
-    // This is not our channel.
-    if (!SameCOMIdentity(aSubject, mChannel)) {
-      return NS_OK;
-    }
-
-    // Maybe the observer is the only reason why this object is still alive.
-    // Let's keep it alive until the operation is completed.
-    RefPtr<AbortChannelObserver> kungFuDeathGrip = this;
-    nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
-    if (obs) {
-      obs->RemoveObserver(this, NS_HTTP_ON_STOP_REQUEST_TOPIC);
-    }
-
-    nsCOMPtr<nsIHttpChannelInternal> internalChannel =
-      do_QueryInterface(mChannel);
-    NS_ENSURE_TRUE(internalChannel, NS_ERROR_NOT_AVAILABLE);
-    mChannel = nullptr;
-
-    bool canceled = false;
-    Unused << NS_WARN_IF(NS_FAILED(internalChannel->GetCanceled(&canceled)));
-
-    // From now on, we touch cross-process things. We need to protect them with
-    // the locking of a mutex.
-    RefPtr<AbortSignalRunnable> r;
-    {
-      MutexAutoLock lock(mMutex);
-
-      mState = canceled ? eChannelAborted : eChannelSucceeded;
-
-      if (!mWorkerRef) {
-        // CreateSignal() has not been called yet.
-        return NS_OK;
-      }
-
-      // This will release AbortSignal on the correct thread.
-      r = new AbortSignalRunnable(mWorkerRef->GetUnsafePrivate(), this, canceled);
-    }
-
-    if (NS_WARN_IF(!r->Dispatch())) {
-      // If this happens, it's not a big deal. |mWorkerRef| will be notified
-      // and AbortSignal will be released.
-      return NS_ERROR_FAILURE;
-    }
-
-    return NS_OK;
-  }
-
-  already_AddRefed<AbortSignal>
-  CreateSignal(WorkerPrivate* aWorkerPrivate)
-  {
-    MOZ_ASSERT(aWorkerPrivate);
-    MOZ_ASSERT(aWorkerPrivate->IsServiceWorker());
-
-    RefPtr<AbortSignal> abortSignal;
-    {
-      MutexAutoLock lock(mMutex);
-      MOZ_DIAGNOSTIC_ASSERT(!mWorkerRef);
-
-      abortSignal = new AbortSignal(mState == eChannelAborted);
-
-      if (mState != eChannelActive) {
-        return abortSignal.forget();
-      }
-    }
-
-    // This is needed to keep AbortSignal alive.
-    RefPtr<AbortChannelObserver> self = this;
-    RefPtr<WeakWorkerRef> workerRef =
-      WeakWorkerRef::Create(aWorkerPrivate,
-        [self]() {
-          self->MaybeAbortAndReleaseSignal(false);
-        }
-      );
-    if (NS_WARN_IF(!workerRef)) {
-      return nullptr;
-    }
-
-    {
-      MutexAutoLock lock(mMutex);
-      if (mState == eChannelActive) {
-        mSignal = abortSignal;
-        mWorkerRef.swap(workerRef);
-      }
-    }
-
-    return abortSignal.forget();
-  }
-
-  void
-  Reset()
-  {
-    MOZ_ASSERT(IsCurrentThreadRunningWorker());
-
-    RefPtr<WeakWorkerRef> workerRef;
-    {
-      MutexAutoLock lock(mMutex);
-      mSignal = nullptr;
-      workerRef.swap(mWorkerRef);
-    }
-  }
-
-  void
-  MaybeAbortAndReleaseSignal(bool aToAbort)
-  {
-    MOZ_ASSERT(IsCurrentThreadRunningWorker());
-
-    RefPtr<WeakWorkerRef> workerRef;
-    {
-      MutexAutoLock lock(mMutex);
-      MOZ_ASSERT(mWorkerRef);
-
-      if (aToAbort && mSignal) {
-        mSignal->Abort();
-      }
-
-      workerRef.swap(mWorkerRef);
-      mSignal = nullptr;
-    }
-  }
-
-private:
-  explicit AbortChannelObserver(nsIChannel* aChannel)
-    : mChannel(aChannel)
-    , mMutex("AbortChannelObserver::mMutex")
-    , mState(eChannelActive)
-  {}
-
-  ~AbortChannelObserver()
-  {}
-
-  nsCOMPtr<nsIChannel> mChannel;
-
-  // This object is touched in main-thread and workers but it's always released
-  // on the worker thread. When touched, the mutex is locked.
-  RefPtr<WeakWorkerRef> mWorkerRef;
-
-  RefPtr<AbortSignal> mSignal;
-
-  mozilla::Mutex mMutex;
-
-  // The following is protected by mutex.
-  enum {
-    // The channel is active.
-    eChannelActive,
-
-    // the channel has been canceled.
-    eChannelAborted,
-
-    // the cannel has been terminated successfully.
-    eChannelSucceeded,
-  } mState;
-};
-
-NS_IMPL_ISUPPORTS(AbortChannelObserver, nsIObserver)
-
-bool
-AbortSignalRunnable::WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate)
-{
-  mObserver->MaybeAbortAndReleaseSignal(mSignalToAbort);
-  return true;
-}
-
 class CheckScriptEvaluationWithCallback final : public WorkerRunnable
 {
   nsMainThreadPtrHandle<ServiceWorkerPrivate> mServiceWorkerPrivate;
   nsMainThreadPtrHandle<KeepAliveToken> mKeepAliveToken;
 
   // The script evaluation result must be reported even if the runnable
   // is cancelled.
   RefPtr<LifeCycleEventCallback> mScriptEvaluationCallback;
@@ -1565,18 +1338,16 @@ class FetchEventRunnable : public Extend
   RequestRedirect mRequestRedirect;
   RequestCredentials mRequestCredentials;
   nsContentPolicyType mContentPolicyType;
   nsCOMPtr<nsIInputStream> mUploadStream;
   int64_t mUploadStreamContentLength;
   nsCString mReferrer;
   ReferrerPolicy mReferrerPolicy;
   nsString mIntegrity;
-  RefPtr<AbortChannelObserver> mAbortObserver;
-
 public:
   FetchEventRunnable(WorkerPrivate* aWorkerPrivate,
                      KeepAliveToken* aKeepAliveToken,
                      nsMainThreadPtrHandle<nsIInterceptedChannel>& aChannel,
                      // CSP checks might require the worker script spec
                      // later on.
                      const nsACString& aScriptSpec,
                      nsMainThreadPtrHandle<ServiceWorkerRegistrationInfo>& aRegistration,
@@ -1732,21 +1503,16 @@ public:
       MOZ_ASSERT(!mUploadStream);
       nsCOMPtr<nsIInputStream> uploadStream;
       rv = uploadChannel->CloneUploadStream(&mUploadStreamContentLength,
                                             getter_AddRefs(uploadStream));
       NS_ENSURE_SUCCESS(rv, rv);
       mUploadStream = uploadStream;
     }
 
-    mAbortObserver = AbortChannelObserver::Create(channel);
-    if (NS_WARN_IF(!mAbortObserver)) {
-      return NS_ERROR_FAILURE;
-    }
-
     return NS_OK;
   }
 
   bool
   WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
   {
     MOZ_ASSERT(aWorkerPrivate);
 
@@ -1769,19 +1535,16 @@ public:
 
   nsresult
   Cancel() override
   {
     nsCOMPtr<nsIRunnable> runnable = new ResumeRequest(mInterceptedChannel);
     if (NS_FAILED(mWorkerPrivate->DispatchToMainThread(runnable))) {
       NS_WARNING("Failed to resume channel on FetchEventRunnable::Cancel()!\n");
     }
-
-    mAbortObserver = nullptr;
-
     WorkerRunnable::Cancel();
     return NS_OK;
   }
 
 private:
   ~FetchEventRunnable() {}
 
   class ResumeRequest final : public Runnable {
@@ -1827,24 +1590,16 @@ private:
       ErrorResult result;
       internalHeaders->Set(mHeaderNames[i], mHeaderValues[i], result);
       if (NS_WARN_IF(result.Failed())) {
         result.SuppressException();
         return false;
       }
     }
 
-    RefPtr<AbortSignal> abortSignal =
-      mAbortObserver->CreateSignal(aWorkerPrivate);
-
-    RefPtr<AbortChannelObserver> observer = mAbortObserver.forget();
-    auto abortObserverRAII = mozilla::MakeScopeExit([observer] {
-      observer->Reset();
-    });
-
     ErrorResult result;
     internalHeaders->SetGuard(HeadersGuardEnum::Immutable, result);
     if (NS_WARN_IF(result.Failed())) {
       result.SuppressException();
       return false;
     }
     RefPtr<InternalRequest> internalReq = new InternalRequest(mSpec,
                                                               mFragment,
@@ -1874,17 +1629,19 @@ private:
       internalReq->SetPreferredAlternativeDataType(alternativeDataType);
     }
 
     nsCOMPtr<nsIGlobalObject> global = do_QueryInterface(globalObj.GetAsSupports());
     if (NS_WARN_IF(!global)) {
       return false;
     }
 
-    RefPtr<Request> request = new Request(global, internalReq, abortSignal);
+    // TODO This request object should be created with a AbortSignal object
+    // which should be aborted if the loading is aborted. See bug 1394102.
+    RefPtr<Request> request = new Request(global, internalReq, nullptr);
 
     MOZ_ASSERT_IF(internalReq->IsNavigationRequest(),
                   request->Redirect() == RequestRedirect::Manual);
 
     RootedDictionary<FetchEventInit> init(aCx);
     init.mRequest = request;
     init.mBubbles = false;
     init.mCancelable = true;
@@ -1922,17 +1679,16 @@ private:
                                              NS_ERROR_INTERCEPTION_FAILED);
       } else {
         runnable = new ResumeRequest(mInterceptedChannel);
       }
 
       MOZ_ALWAYS_SUCCEEDS(mWorkerPrivate->DispatchToMainThread(runnable.forget()));
     }
 
-    abortObserverRAII.release();
     return true;
   }
 };
 
 NS_IMPL_ISUPPORTS_INHERITED(FetchEventRunnable, WorkerRunnable, nsIHttpHeaderVisitor)
 
 } // anonymous namespace
 
--- a/testing/web-platform/tests/fetch/api/abort/serviceworker-intercepted.https.html
+++ b/testing/web-platform/tests/fetch/api/abort/serviceworker-intercepted.https.html
@@ -89,42 +89,11 @@
     const response = await w.fetch('data.json', { signal });
     const reader = response.body.getReader();
 
     controller.abort();
 
     await promise_rejects(t, "AbortError", reader.read());
     await promise_rejects(t, "AbortError", reader.closed);
   }, "Stream errors once aborted.");
-
-  promise_test(async t => {
-    const scope = SCOPE + "?q=aborted-signal";
-    await setupRegistration(t, scope);
-    const iframe = await with_iframe(scope);
-    const w = iframe.contentWindow;
-
-    const testReady = new Promise(resolve => {
-      w.navigator.serviceWorker.addEventListener('message', event => {
-        resolve(event.data);
-      }, { once: true });
-    });
-
-    const controller = new w.AbortController();
-    const signal = controller.signal;
-
-    w.fetch('data.json?signal', { signal });
-
-    assert_true((await testReady) == "ready", "");
-
-    const aborted = new Promise(resolve => {
-      w.navigator.serviceWorker.addEventListener('message', event => {
-        resolve(event.data);
-      }, { once: true });
-    });
-
-    controller.abort();
-
-    assert_true((await aborted) == "aborted", "");
-  }, "FetchEvent.request.signal must be aborted.");
-
 </script>
 </body>
 </html>
--- a/testing/web-platform/tests/fetch/api/resources/sw-intercept.js
+++ b/testing/web-platform/tests/fetch/api/resources/sw-intercept.js
@@ -1,20 +1,10 @@
 async function broadcast(msg) {
   for (const client of await clients.matchAll()) {
     client.postMessage(msg);
   }
 }
 
 addEventListener('fetch', event => {
-  if (event.request.url.endsWith('?signal')) {
-    event.respondWith(new Promise((resolve, reject) => {
-      event.request.signal.onabort = () => {
-        broadcast("aborted");
-        reject();
-      };
-      event.waitUntil(broadcast("ready"));
-    }));
-  } else {
-    event.waitUntil(broadcast(event.request.url));
-    event.respondWith(fetch(event.request));
-  }
+  event.waitUntil(broadcast(event.request.url));
+  event.respondWith(fetch(event.request));
 });