Backed out changesets 2381f29f63c2 and 9f2469ea7f01 (bug 1193133) for frequent w-p-t crashes/asserts. a=me
authorRyan VanderMeulen <ryanvm@gmail.com>
Fri, 14 Aug 2015 09:00:59 -0400
changeset 257782 1b24022474297b294f3ccb3f0f9d7e477ea2c2ae
parent 257781 641235287674dd61ba38e72cc724763de74897a7
child 257790 8781f437a5d4e274aff5fb6fc9728d3b098b2bd1
push id29226
push userryanvm@gmail.com
push dateFri, 14 Aug 2015 13:01:14 +0000
treeherdermozilla-central@1b2402247429 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersme
bugs1193133
milestone43.0a1
backs out2381f29f63c23727944ca51f5b69781f53f9a442
9f2469ea7f016e36ba3470069a908acf57f80362
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
Backed out changesets 2381f29f63c2 and 9f2469ea7f01 (bug 1193133) for frequent w-p-t crashes/asserts. a=me
dom/workers/ServiceWorker.cpp
dom/workers/ServiceWorker.h
dom/workers/ServiceWorkerClient.cpp
dom/workers/ServiceWorkerClient.h
dom/workers/ServiceWorkerManager.cpp
dom/workers/ServiceWorkerWindowClient.cpp
testing/web-platform/mozilla/meta/service-workers/service-worker/fetch-event-async-respond-with.https.html.ini
testing/web-platform/mozilla/meta/service-workers/service-worker/fetch-event-respond-with-stops-propagation.https.html.ini
--- a/dom/workers/ServiceWorker.cpp
+++ b/dom/workers/ServiceWorker.cpp
@@ -47,16 +47,21 @@ ServiceWorker::ServiceWorker(nsPIDOMWind
   : DOMEventTargetHelper(aWindow),
     mInfo(aInfo),
     mSharedWorker(aSharedWorker)
 {
   AssertIsOnMainThread();
   MOZ_ASSERT(aInfo);
   MOZ_ASSERT(mSharedWorker);
 
+  if (aWindow) {
+    mDocument = aWindow->GetExtantDoc();
+    mWindow = aWindow->GetOuterWindow();
+  }
+
   // This will update our state too.
   mInfo->AppendWorker(this);
 }
 
 ServiceWorker::~ServiceWorker()
 {
   AssertIsOnMainThread();
   mInfo->RemoveWorker(this);
@@ -64,17 +69,17 @@ ServiceWorker::~ServiceWorker()
 
 NS_IMPL_ADDREF_INHERITED(ServiceWorker, DOMEventTargetHelper)
 NS_IMPL_RELEASE_INHERITED(ServiceWorker, DOMEventTargetHelper)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(ServiceWorker)
 NS_INTERFACE_MAP_END_INHERITING(DOMEventTargetHelper)
 
 NS_IMPL_CYCLE_COLLECTION_INHERITED(ServiceWorker, DOMEventTargetHelper,
-                                   mSharedWorker)
+                                   mSharedWorker, mDocument, mWindow)
 
 JSObject*
 ServiceWorker::WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto)
 {
   AssertIsOnMainThread();
 
   return ServiceWorkerBinding::Wrap(aCx, this, aGivenProto);
 }
@@ -85,32 +90,30 @@ ServiceWorker::GetScriptURL(nsString& aU
   CopyUTF8toUTF16(mInfo->ScriptSpec(), aURL);
 }
 
 void
 ServiceWorker::PostMessage(JSContext* aCx, JS::Handle<JS::Value> aMessage,
                            const Optional<Sequence<JS::Value>>& aTransferable,
                            ErrorResult& aRv)
 {
+  WorkerPrivate* workerPrivate = GetWorkerPrivate();
+  MOZ_ASSERT(workerPrivate);
+
   if (State() == ServiceWorkerState::Redundant) {
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return;
   }
 
-  nsCOMPtr<nsPIDOMWindow> window = do_QueryInterface(GetParentObject());
-  if (!window || !window->GetExtantDoc()) {
-    NS_WARNING("Trying to call post message from an invalid dom object.");
-    aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
-    return;
-  }
+  MOZ_ASSERT(mDocument && mWindow,
+             "Cannot call PostMessage on a ServiceWorker object that doesn't "
+             "have a window");
 
-  WorkerPrivate* workerPrivate = GetWorkerPrivate();
-  MOZ_ASSERT(workerPrivate);
-
-  nsAutoPtr<ServiceWorkerClientInfo> clientInfo(new ServiceWorkerClientInfo(window->GetExtantDoc()));
+  nsAutoPtr<ServiceWorkerClientInfo> clientInfo(
+    new ServiceWorkerClientInfo(mDocument, mWindow));
 
   workerPrivate->PostMessageToServiceWorker(aCx, aMessage, aTransferable,
                                             clientInfo, aRv);
 }
 
 WorkerPrivate*
 ServiceWorker::GetWorkerPrivate() const
 {
--- a/dom/workers/ServiceWorker.h
+++ b/dom/workers/ServiceWorker.h
@@ -6,16 +6,17 @@
 
 #ifndef mozilla_dom_workers_serviceworker_h__
 #define mozilla_dom_workers_serviceworker_h__
 
 #include "mozilla/DOMEventTargetHelper.h"
 #include "mozilla/dom/BindingDeclarations.h"
 #include "mozilla/dom/ServiceWorkerBinding.h" // For ServiceWorkerState.
 
+class nsIDocument;
 class nsPIDOMWindow;
 
 namespace mozilla {
 namespace dom {
 
 namespace workers {
 
 class ServiceWorkerInfo;
@@ -86,15 +87,19 @@ private:
   ServiceWorkerState mState;
   const nsRefPtr<ServiceWorkerInfo> mInfo;
 
   // To allow ServiceWorkers to potentially drop the backing DOMEventTargetHelper and
   // re-instantiate it later, they simply own a SharedWorker member that
   // can be released and recreated as required rather than re-implement some of
   // the SharedWorker logic.
   nsRefPtr<SharedWorker> mSharedWorker;
+  // We need to keep the document and window alive for PostMessage to be able
+  // to access them.
+  nsCOMPtr<nsIDocument> mDocument;
+  nsCOMPtr<nsPIDOMWindow> mWindow;
 };
 
 } // namespace workers
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_workers_serviceworker_h__
--- a/dom/workers/ServiceWorkerClient.cpp
+++ b/dom/workers/ServiceWorkerClient.cpp
@@ -24,17 +24,18 @@ NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(Se
 NS_IMPL_CYCLE_COLLECTING_ADDREF(ServiceWorkerClient)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(ServiceWorkerClient)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(ServiceWorkerClient)
   NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
   NS_INTERFACE_MAP_ENTRY(nsISupports)
 NS_INTERFACE_MAP_END
 
-ServiceWorkerClientInfo::ServiceWorkerClientInfo(nsIDocument* aDoc)
+ServiceWorkerClientInfo::ServiceWorkerClientInfo(nsIDocument* aDoc,
+                                                 nsPIDOMWindow* aWindow)
   : mWindowId(0)
 {
   MOZ_ASSERT(aDoc);
   nsresult rv = aDoc->GetId(mClientId);
   if (NS_FAILED(rv)) {
     NS_WARNING("Failed to get the UUID of the document.");
   }
 
@@ -49,17 +50,17 @@ ServiceWorkerClientInfo::ServiceWorkerCl
   mVisibilityState = aDoc->VisibilityState();
 
   ErrorResult result;
   mFocused = aDoc->HasFocus(result);
   if (result.Failed()) {
     NS_WARNING("Failed to get focus information.");
   }
 
-  nsRefPtr<nsGlobalWindow> outerWindow = static_cast<nsGlobalWindow*>(aDoc->GetWindow());
+  nsRefPtr<nsGlobalWindow> outerWindow = static_cast<nsGlobalWindow*>(aWindow);
   MOZ_ASSERT(outerWindow);
   if (!outerWindow->IsTopLevelWindow()) {
     mFrameType = FrameType::Nested;
   } else if (outerWindow->HadOriginalOpener()) {
     mFrameType = FrameType::Auxiliary;
   } else {
     mFrameType = FrameType::Top_level;
   }
--- a/dom/workers/ServiceWorkerClient.h
+++ b/dom/workers/ServiceWorkerClient.h
@@ -10,33 +10,34 @@
 
 #include "nsCOMPtr.h"
 #include "nsWrapperCache.h"
 #include "mozilla/ErrorResult.h"
 #include "mozilla/dom/BindingDeclarations.h"
 #include "mozilla/dom/ClientBinding.h"
 
 class nsIDocument;
+class nsPIDOMWindow;
 
 namespace mozilla {
 namespace dom {
 namespace workers {
 
 class ServiceWorkerClient;
 class ServiceWorkerWindowClient;
 
 // Used as a container object for information needed to create
 // client objects.
 class ServiceWorkerClientInfo final
 {
   friend class ServiceWorkerClient;
   friend class ServiceWorkerWindowClient;
 
 public:
-  explicit ServiceWorkerClientInfo(nsIDocument* aDoc);
+  ServiceWorkerClientInfo(nsIDocument* aDoc, nsPIDOMWindow* aWindow);
 
 private:
   nsString mClientId;
   uint64_t mWindowId;
   nsString mUrl;
 
   // Window Clients
   VisibilityState mVisibilityState;
--- a/dom/workers/ServiceWorkerManager.cpp
+++ b/dom/workers/ServiceWorkerManager.cpp
@@ -3891,17 +3891,17 @@ ServiceWorkerManager::DispatchFetchEvent
     NS_NewRunnableMethod(aChannel, &nsIInterceptedChannel::ResetInterception);
 
   nsAutoPtr<ServiceWorkerClientInfo> clientInfo;
 
   if (!isNavigation) {
     MOZ_ASSERT(aDoc);
     aRv = GetDocumentController(aDoc->GetInnerWindow(), failRunnable,
                                 getter_AddRefs(serviceWorker));
-    clientInfo = new ServiceWorkerClientInfo(aDoc);
+    clientInfo = new ServiceWorkerClientInfo(aDoc, aDoc->GetWindow());
   } else {
     nsCOMPtr<nsIChannel> internalChannel;
     aRv = aChannel->GetChannel(getter_AddRefs(internalChannel));
     if (NS_WARN_IF(aRv.Failed())) {
       return;
     }
 
     nsCOMPtr<nsIURI> uri;
@@ -4271,17 +4271,17 @@ EnumControlledDocuments(nsISupports* aKe
   }
 
   nsCOMPtr<nsIDocument> document = do_QueryInterface(aKey);
 
   if (!document || !document->GetWindow()) {
     return PL_DHASH_NEXT;
   }
 
-  ServiceWorkerClientInfo clientInfo(document);
+  ServiceWorkerClientInfo clientInfo(document, document->GetWindow());
   data->mDocuments.AppendElement(clientInfo);
 
   return PL_DHASH_NEXT;
 }
 
 static void
 FireControllerChangeOnDocument(nsIDocument* aDocument)
 {
--- a/dom/workers/ServiceWorkerWindowClient.cpp
+++ b/dom/workers/ServiceWorkerWindowClient.cpp
@@ -85,24 +85,19 @@ public:
   NS_IMETHOD
   Run() override
   {
     AssertIsOnMainThread();
     nsGlobalWindow* window = nsGlobalWindow::GetInnerWindowWithId(mWindowId);
     UniquePtr<ServiceWorkerClientInfo> clientInfo;
 
     if (window) {
-      nsCOMPtr<nsIDocument> doc = window->GetDocument();
-      if (doc) {
-        nsContentUtils::DispatchChromeEvent(doc,
-                                            window->GetOuterWindow(),
-                                            NS_LITERAL_STRING("DOMServiceWorkerFocusClient"),
-                                            true, true);
-        clientInfo.reset(new ServiceWorkerClientInfo(doc));
-      }
+      nsContentUtils::DispatchChromeEvent(window->GetExtantDoc(), window->GetOuterWindow(), NS_LITERAL_STRING("DOMServiceWorkerFocusClient"), true, true);
+      clientInfo.reset(new ServiceWorkerClientInfo(window->GetDocument(),
+                                                   window->GetOuterWindow()));
     }
 
     DispatchResult(Move(clientInfo));
     return NS_OK;
   }
 
 private:
   void
--- a/testing/web-platform/mozilla/meta/service-workers/service-worker/fetch-event-async-respond-with.https.html.ini
+++ b/testing/web-platform/mozilla/meta/service-workers/service-worker/fetch-event-async-respond-with.https.html.ini
@@ -1,5 +1,3 @@
 [fetch-event-async-respond-with.https.html]
   type: testharness
-  expected: OK
-  [Calling respondWith asynchronously throws an exception]
-    expected: FAIL
+  expected: CRASH
--- a/testing/web-platform/mozilla/meta/service-workers/service-worker/fetch-event-respond-with-stops-propagation.https.html.ini
+++ b/testing/web-platform/mozilla/meta/service-workers/service-worker/fetch-event-respond-with-stops-propagation.https.html.ini
@@ -1,6 +1,3 @@
 [fetch-event-respond-with-stops-propagation.https.html]
   type: testharness
-  expected: OK
-  [respondWith() invokes stopImmediatePropagation()]
-    expected: FAIL
-
+  expected: CRASH