Backed out 3 changesets (bug 1538979) for bustage on JSWindowActor.cpp. CLOSED TREE
authorCsoregi Natalia <ncsoregi@mozilla.com>
Fri, 10 May 2019 12:44:22 +0300
changeset 473369 012ce6437a4cbb2bdffb1d641dd08f0a4ca66b75
parent 473368 7b99b4edde300fb943804f5709064fba5e493a8e
child 473370 aa7e1ffb96b957caa99207ebcb4052049af3703c
push id35996
push userdvarga@mozilla.com
push dateFri, 10 May 2019 21:46:48 +0000
treeherdermozilla-central@362df4629f8f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1538979
milestone68.0a1
backs outa098226e42117f59f17112ed7be9e39710e18681
8e065761738c199fc06388dcfc22df7e92827152
9df2b856b6550578f75d862a1fb4579b5665742e
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 3 changesets (bug 1538979) for bustage on JSWindowActor.cpp. CLOSED TREE Backed out changeset a098226e4211 (bug 1538979) Backed out changeset 8e065761738c (bug 1538979) Backed out changeset 9df2b856b655 (bug 1538979)
dom/chrome-webidl/JSWindowActor.webidl
dom/ipc/JSWindowActor.cpp
dom/ipc/JSWindowActor.h
dom/ipc/JSWindowActorChild.cpp
dom/ipc/JSWindowActorChild.h
dom/ipc/JSWindowActorParent.cpp
dom/ipc/JSWindowActorParent.h
dom/ipc/WindowGlobalChild.cpp
dom/ipc/WindowGlobalParent.cpp
dom/ipc/tests/browser_JSWindowActor.js
toolkit/actors/TestChild.jsm
--- a/dom/chrome-webidl/JSWindowActor.webidl
+++ b/dom/chrome-webidl/JSWindowActor.webidl
@@ -46,17 +46,8 @@ JSWindowActorChild implements JSWindowAc
 // WebIDL callback interface version of the nsIObserver interface for use when
 // calling the observe method on JSWindowActors.
 //
 // NOTE: This isn't marked as ChromeOnly, as it has no interface object, and
 // thus cannot be conditionally exposed.
 callback interface MozObserverCallback {
   void observe(nsISupports subject, ByteString topic, DOMString? data);
 };
-
-// WebIDL callback interface calling the `willDestroy` and `didDestroy`
-// method on JSWindowActors.
-callback MozActorDestroyCallback = void();
-
-dictionary MozActorDestroyCallbacks {
-  [ChromeOnly] MozActorDestroyCallback willDestroy;
-  [ChromeOnly] MozActorDestroyCallback didDestroy;
-};
--- a/dom/ipc/JSWindowActor.cpp
+++ b/dom/ipc/JSWindowActor.cpp
@@ -34,47 +34,16 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 NS_IMPL_CYCLE_COLLECTION_TRACE_WRAPPERCACHE(JSWindowActor)
 
 JSWindowActor::JSWindowActor() : mNextQueryId(0) {}
 
 nsIGlobalObject* JSWindowActor::GetParentObject() const {
   return xpc::NativeGlobal(xpc::PrivilegedJunkScope());
 }
 
-void JSWindowActor::StartDestroy() {
-  DestroyCallback(DestroyCallbackFunction::WillDestroy);
-}
-
-void JSWindowActor::AfterDestroy() {
-  DestroyCallback(DestroyCallbackFunction::DidDestroy);
-}
-
-void JSWindowActor::DestroyCallback(DestroyCallbackFunction callback) {
-  AutoEntryScript aes(xpc::PrivilegedJunkScope(),
-                      "JSWindowActor destroy callback");
-  JSContext* cx = aes.cx();
-  MozActorDestroyCallbacks callbacksHolder;
-  NS_ENSURE_TRUE_VOID(GetWrapper());
-  JS::Rooted<JS::Value> val(cx, JS::ObjectValue(*GetWrapper()));
-  if (NS_WARN_IF(!callbacksHolder.Init(cx, val))) {
-    return;
-  }
-
-  // Destroy callback is optional.
-  if (callback == DestroyCallbackFunction::WillDestroy) {
-    if (callbacksHolder.mWillDestroy.WasPassed()) {
-      callbacksHolder.mWillDestroy.Value()->Call();
-    }
-  } else {
-    if (callbacksHolder.mDidDestroy.WasPassed()) {
-      callbacksHolder.mDidDestroy.Value()->Call();
-    }
-  }
-}
-
 void JSWindowActor::RejectPendingQueries() {
   // Take our queries out, in case somehow rejecting promises can trigger
   // additions or removals.
   nsRefPtrHashtable<nsUint64HashKey, Promise> pendingQueries;
   mPendingQueries.SwapElements(pendingQueries);
   for (auto iter = pendingQueries.Iter(); !iter.Done(); iter.Next()) {
     iter.Data()->MaybeReject(NS_ERROR_NOT_AVAILABLE);
   }
--- a/dom/ipc/JSWindowActor.h
+++ b/dom/ipc/JSWindowActor.h
@@ -34,17 +34,16 @@ class QueryPromiseHandler;
 class JSWindowActor : public nsISupports, public nsWrapperCache {
  public:
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(JSWindowActor)
 
   JSWindowActor();
 
   enum class Type { Parent, Child };
-  enum class DestroyCallbackFunction { WillDestroy, DidDestroy };
 
   const nsString& Name() const { return mName; }
 
   void SendAsyncMessage(JSContext* aCx, const nsAString& aMessageName,
                         JS::Handle<JS::Value> aObj,
                         JS::Handle<JS::Value> aTransfers, ErrorResult& aRv);
 
   already_AddRefed<Promise> SendQuery(JSContext* aCx,
@@ -67,22 +66,16 @@ class JSWindowActor : public nsISupports
   virtual void SendRawMessage(const JSWindowActorMessageMeta& aMetadata,
                               ipc::StructuredCloneData&& aData,
                               ErrorResult& aRv) = 0;
 
   virtual ~JSWindowActor() = default;
 
   void SetName(const nsAString& aName);
 
-  void StartDestroy();
-
-  void AfterDestroy();
-
-  void DestroyCallback(DestroyCallbackFunction willDestroy);
-
  private:
   void ReceiveMessageOrQuery(JSContext* aCx,
                              const JSWindowActorMessageMeta& aMetadata,
                              JS::Handle<JS::Value> aData, ErrorResult& aRv);
 
   void ReceiveQueryReply(JSContext* aCx,
                          const JSWindowActorMessageMeta& aMetadata,
                          JS::Handle<JS::Value> aData, ErrorResult& aRv);
--- a/dom/ipc/JSWindowActorChild.cpp
+++ b/dom/ipc/JSWindowActorChild.cpp
@@ -10,18 +10,16 @@
 #include "mozilla/dom/WindowGlobalParent.h"
 #include "mozilla/dom/MessageManagerBinding.h"
 #include "mozilla/dom/BrowsingContext.h"
 #include "nsGlobalWindowInner.h"
 
 namespace mozilla {
 namespace dom {
 
-JSWindowActorChild::~JSWindowActorChild() { MOZ_ASSERT(!mManager); }
-
 JSObject* JSWindowActorChild::WrapObject(JSContext* aCx,
                                          JS::Handle<JSObject*> aGivenProto) {
   return JSWindowActorChild_Binding::Wrap(aCx, this, aGivenProto);
 }
 
 WindowGlobalChild* JSWindowActorChild::Manager() const { return mManager; }
 
 void JSWindowActorChild::Init(const nsAString& aName,
@@ -55,17 +53,17 @@ class AsyncMessageToParent : public Runn
   RefPtr<WindowGlobalParent> mParent;
 };
 
 }  // anonymous namespace
 
 void JSWindowActorChild::SendRawMessage(const JSWindowActorMessageMeta& aMeta,
                                         ipc::StructuredCloneData&& aData,
                                         ErrorResult& aRv) {
-  if (NS_WARN_IF(!mCanSend || !mManager || mManager->IsClosed())) {
+  if (NS_WARN_IF(!mManager || mManager->IsClosed())) {
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return;
   }
 
   if (mManager->IsInProcess()) {
     RefPtr<WindowGlobalParent> wgp = mManager->GetParentActor();
     nsCOMPtr<nsIRunnable> runnable =
         new AsyncMessageToParent(aMeta, std::move(aData), wgp);
@@ -109,26 +107,16 @@ BrowsingContext* JSWindowActorChild::Get
 Nullable<WindowProxyHolder> JSWindowActorChild::GetContentWindow(
     ErrorResult& aRv) {
   if (BrowsingContext* bc = GetBrowsingContext(aRv)) {
     return WindowProxyHolder(bc);
   }
   return nullptr;
 }
 
-void JSWindowActorChild::StartDestroy() {
-  JSWindowActor::StartDestroy();
-  mCanSend = false;
-}
-
-void JSWindowActorChild::AfterDestroy() {
-  JSWindowActor::AfterDestroy();
-  mManager = nullptr;
-}
-
 NS_IMPL_CYCLE_COLLECTION_INHERITED(JSWindowActorChild, JSWindowActor, mManager)
 
 NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN_INHERITED(JSWindowActorChild,
                                                JSWindowActor)
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(JSWindowActorChild)
 NS_INTERFACE_MAP_END_INHERITING(JSWindowActor)
--- a/dom/ipc/JSWindowActorChild.h
+++ b/dom/ipc/JSWindowActorChild.h
@@ -42,30 +42,28 @@ class JSWindowActorChild final : public 
 
   static already_AddRefed<JSWindowActorChild> Constructor(GlobalObject& aGlobal,
                                                           ErrorResult& aRv) {
     return MakeAndAddRef<JSWindowActorChild>();
   }
 
   WindowGlobalChild* Manager() const;
   void Init(const nsAString& aName, WindowGlobalChild* aManager);
-  void StartDestroy();
-  void AfterDestroy();
+
   Document* GetDocument(ErrorResult& aRv);
   BrowsingContext* GetBrowsingContext(ErrorResult& aRv);
   Nullable<WindowProxyHolder> GetContentWindow(ErrorResult& aRv);
 
  protected:
   void SendRawMessage(const JSWindowActorMessageMeta& aMeta,
                       ipc::StructuredCloneData&& aData,
                       ErrorResult& aRv) override;
 
  private:
-  ~JSWindowActorChild();
+  ~JSWindowActorChild() = default;
 
-  bool mCanSend = true;
   RefPtr<WindowGlobalChild> mManager;
 };
 
 }  // namespace dom
 }  // namespace mozilla
 
 #endif  // mozilla_dom_JSWindowActorChild_h
--- a/dom/ipc/JSWindowActorParent.cpp
+++ b/dom/ipc/JSWindowActorParent.cpp
@@ -7,18 +7,16 @@
 #include "mozilla/dom/JSWindowActorBinding.h"
 #include "mozilla/dom/JSWindowActorParent.h"
 #include "mozilla/dom/WindowGlobalParent.h"
 #include "mozilla/dom/MessageManagerBinding.h"
 
 namespace mozilla {
 namespace dom {
 
-JSWindowActorParent::~JSWindowActorParent() { MOZ_ASSERT(!mManager); }
-
 JSObject* JSWindowActorParent::WrapObject(JSContext* aCx,
                                           JS::Handle<JSObject*> aGivenProto) {
   return JSWindowActorParent_Binding::Wrap(aCx, this, aGivenProto);
 }
 
 WindowGlobalParent* JSWindowActorParent::Manager() const { return mManager; }
 
 void JSWindowActorParent::Init(const nsAString& aName,
@@ -52,17 +50,17 @@ class AsyncMessageToChild : public Runna
   RefPtr<WindowGlobalChild> mChild;
 };
 
 }  // anonymous namespace
 
 void JSWindowActorParent::SendRawMessage(const JSWindowActorMessageMeta& aMeta,
                                          ipc::StructuredCloneData&& aData,
                                          ErrorResult& aRv) {
-  if (NS_WARN_IF(!mCanSend || !mManager || mManager->IsClosed())) {
+  if (NS_WARN_IF(!mManager || mManager->IsClosed())) {
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return;
   }
 
   if (mManager->IsInProcess()) {
     RefPtr<WindowGlobalChild> wgc = mManager->GetChildActor();
     nsCOMPtr<nsIRunnable> runnable =
         new AsyncMessageToChild(aMeta, std::move(aData), wgc);
@@ -80,26 +78,16 @@ void JSWindowActorParent::SendRawMessage
   }
 
   if (NS_WARN_IF(!mManager->SendRawMessage(aMeta, msgData))) {
     aRv.Throw(NS_ERROR_UNEXPECTED);
     return;
   }
 }
 
-void JSWindowActorParent::StartDestroy() {
-  JSWindowActor::StartDestroy();
-  mCanSend = false;
-}
-
-void JSWindowActorParent::AfterDestroy() {
-  JSWindowActor::AfterDestroy();
-  mManager = nullptr;
-}
-
 NS_IMPL_CYCLE_COLLECTION_INHERITED(JSWindowActorParent, JSWindowActor, mManager)
 
 NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN_INHERITED(JSWindowActorParent,
                                                JSWindowActor)
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(JSWindowActorParent)
 NS_INTERFACE_MAP_END_INHERITING(JSWindowActor)
--- a/dom/ipc/JSWindowActorParent.h
+++ b/dom/ipc/JSWindowActorParent.h
@@ -37,27 +37,24 @@ class JSWindowActorParent final : public
 
   static already_AddRefed<JSWindowActorParent> Constructor(
       GlobalObject& aGlobal, ErrorResult& aRv) {
     return MakeAndAddRef<JSWindowActorParent>();
   }
 
   WindowGlobalParent* Manager() const;
   void Init(const nsAString& aName, WindowGlobalParent* aManager);
-  void StartDestroy();
-  void AfterDestroy();
 
  protected:
   void SendRawMessage(const JSWindowActorMessageMeta& aMeta,
                       ipc::StructuredCloneData&& aData,
                       ErrorResult& aRv) override;
 
  private:
-  ~JSWindowActorParent();
+  ~JSWindowActorParent() = default;
 
-  bool mCanSend = true;
   RefPtr<WindowGlobalParent> mManager;
 };
 
 }  // namespace dom
 }  // namespace mozilla
 
 #endif  // mozilla_dom_JSWindowActorParent_h
--- a/dom/ipc/WindowGlobalChild.cpp
+++ b/dom/ipc/WindowGlobalChild.cpp
@@ -139,26 +139,16 @@ already_AddRefed<BrowserChild> WindowGlo
 }
 
 void WindowGlobalChild::Destroy() {
   // Perform async IPC shutdown unless we're not in-process, and our
   // BrowserChild is in the process of being destroyed, which will destroy us as
   // well.
   RefPtr<BrowserChild> browserChild = GetBrowserChild();
   if (!browserChild || !browserChild->IsDestroyed()) {
-    // Make a copy so that we can avoid potential iterator invalidation when
-    // calling the user-provided Destroy() methods.
-    nsTArray<RefPtr<JSWindowActorChild>> windowActors(mWindowActors.Count());
-    for (auto iter = mWindowActors.Iter(); !iter.Done(); iter.Next()) {
-      windowActors.AppendElement(iter.UserData());
-    }
-
-    for (auto& windowActor : windowActors) {
-      windowActor->StartDestroy();
-    }
     SendDestroy();
   }
 
   mIPCClosed = true;
 }
 
 static nsresult ChangeFrameRemoteness(WindowGlobalChild* aWgc,
                                       BrowsingContext* aBc,
@@ -301,25 +291,22 @@ void WindowGlobalChild::ActorDestroy(Act
   mIPCClosed = true;
   gWindowGlobalChildById->Remove(mInnerWindowId);
 
   // Destroy our JSWindowActors, and reject any pending queries.
   nsRefPtrHashtable<nsStringHashKey, JSWindowActorChild> windowActors;
   mWindowActors.SwapElements(windowActors);
   for (auto iter = windowActors.Iter(); !iter.Done(); iter.Next()) {
     iter.Data()->RejectPendingQueries();
-    iter.Data()->AfterDestroy();
   }
-  windowActors.Clear();
 }
 
 WindowGlobalChild::~WindowGlobalChild() {
   MOZ_ASSERT(!gWindowGlobalChildById ||
              !gWindowGlobalChildById->Contains(mInnerWindowId));
-  MOZ_ASSERT(!mWindowActors.Count());
 }
 
 JSObject* WindowGlobalChild::WrapObject(JSContext* aCx,
                                         JS::Handle<JSObject*> aGivenProto) {
   return WindowGlobalChild_Binding::Wrap(aCx, this, aGivenProto);
 }
 
 nsISupports* WindowGlobalChild::GetParentObject() {
--- a/dom/ipc/WindowGlobalParent.cpp
+++ b/dom/ipc/WindowGlobalParent.cpp
@@ -165,26 +165,16 @@ IPCResult WindowGlobalParent::RecvBecome
   mBrowsingContext->SetCurrentWindowGlobal(this);
   return IPC_OK();
 }
 
 IPCResult WindowGlobalParent::RecvDestroy() {
   if (!mIPCClosed) {
     RefPtr<BrowserParent> browserParent = GetRemoteTab();
     if (!browserParent || !browserParent->IsDestroyed()) {
-      // Make a copy so that we can avoid potential iterator invalidation when
-      // calling the user-provided Destroy() methods.
-      nsTArray<RefPtr<JSWindowActorParent>> windowActors(mWindowActors.Count());
-      for (auto iter = mWindowActors.Iter(); !iter.Done(); iter.Next()) {
-        windowActors.AppendElement(iter.UserData());
-      }
-
-      for (auto& windowActor : windowActors) {
-        windowActor->StartDestroy();
-      }
       Unused << Send__delete__(this);
     }
   }
   return IPC_OK();
 }
 
 IPCResult WindowGlobalParent::RecvRawMessage(
     const JSWindowActorMessageMeta& aMeta, const ClonedMessageData& aData) {
@@ -304,30 +294,27 @@ void WindowGlobalParent::ActorDestroy(Ac
   gWindowGlobalParentsById->Remove(mInnerWindowId);
   mBrowsingContext->UnregisterWindowGlobal(this);
 
   // Destroy our JSWindowActors, and reject any pending queries.
   nsRefPtrHashtable<nsStringHashKey, JSWindowActorParent> windowActors;
   mWindowActors.SwapElements(windowActors);
   for (auto iter = windowActors.Iter(); !iter.Done(); iter.Next()) {
     iter.Data()->RejectPendingQueries();
-    iter.Data()->AfterDestroy();
   }
-  windowActors.Clear();
 
   nsCOMPtr<nsIObserverService> obs = services::GetObserverService();
   if (obs) {
     obs->NotifyObservers(this, "window-global-destroyed", nullptr);
   }
 }
 
 WindowGlobalParent::~WindowGlobalParent() {
   MOZ_ASSERT(!gWindowGlobalParentsById ||
              !gWindowGlobalParentsById->Contains(mInnerWindowId));
-  MOZ_ASSERT(!mWindowActors.Count());
 }
 
 JSObject* WindowGlobalParent::WrapObject(JSContext* aCx,
                                          JS::Handle<JSObject*> aGivenProto) {
   return WindowGlobalParent_Binding::Wrap(aCx, this, aGivenProto);
 }
 
 nsISupports* WindowGlobalParent::GetParentObject() {
--- a/dom/ipc/tests/browser_JSWindowActor.js
+++ b/dom/ipc/tests/browser_JSWindowActor.js
@@ -401,79 +401,8 @@ declTest("getActor with includeChrome", 
   includeChrome: true,
 
   async test(_browser, win) {
     let parent = win.docShell.browsingContext.currentWindowGlobal;
     let actorParent = parent.getActor("Test");
     ok(actorParent, "JSWindowActorParent should have value.");
   },
 });
-
-declTest("destroy actor by iframe remove", {
-  allFrames: true,
-
-  async test(browser) {
-    await ContentTask.spawn(browser, {}, async function() {
-      // Create and append an iframe into the window's document.
-      let frame = content.document.createElement("iframe");
-      frame.id = "frame";
-      content.document.body.appendChild(frame);
-      await ContentTaskUtils.waitForEvent(frame, "load");
-      is(content.window.frames.length, 1, "There should be an iframe.");
-      let child = frame.contentWindow.window.getWindowGlobalChild();
-      let actorChild = child.getActor("Test");
-      ok(actorChild, "JSWindowActorChild should have value.");
-
-      let willDestroyPromise = new Promise(resolve => {
-        const TOPIC = "test-js-window-actor-willdestroy";
-        Services.obs.addObserver(function obs(subject, topic, data) {
-          ok(data, "willDestroyCallback data should be true.");
-
-          Services.obs.removeObserver(obs, TOPIC);
-          resolve();
-        }, TOPIC);
-      });
-
-      let didDestroyPromise = new Promise(resolve => {
-        const TOPIC = "test-js-window-actor-diddestroy";
-        Services.obs.addObserver(function obs(subject, topic, data) {
-          ok(data, "didDestroyCallback data should be true.");
-
-          Services.obs.removeObserver(obs, TOPIC);
-          resolve();
-        }, TOPIC);
-      });
-
-      info("Remove frame");
-      content.document.getElementById("frame").remove();
-      await Promise.all([willDestroyPromise, didDestroyPromise]);
-
-      Assert.throws(() => child.getActor("Test"),
-        /InvalidStateError/, "Should throw if frame destroy.");
-    });
-  },
-});
-
-declTest("destroy actor by page navigates", {
-  allFrames: true,
-
-  async test(browser) {
-    info("creating an in-process frame");
-    await ContentTask.spawn(browser, URL, async function(url) {
-      let frame = content.document.createElement("iframe");
-      frame.src = url;
-      content.document.body.appendChild(frame);
-    });
-
-    info("navigating page");
-    await ContentTask.spawn(browser, TEST_URL, async function(url) {
-      let frame = content.document.querySelector("iframe");
-      frame.contentWindow.location = url;
-      let child = frame.contentWindow.window.getWindowGlobalChild();
-      let actorChild = child.getActor("Test");
-      ok(actorChild, "JSWindowActorChild should have value.");
-      await ContentTaskUtils.waitForEvent(frame, "load");
-
-      Assert.throws(() => child.getActor("Test"),
-              /InvalidStateError/, "Should throw if frame destroy.");
-    });
-  },
-});
--- a/toolkit/actors/TestChild.jsm
+++ b/toolkit/actors/TestChild.jsm
@@ -1,16 +1,14 @@
 /* vim: set ts=2 sw=2 sts=2 et tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 "use strict";
 
-const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
-
 var EXPORTED_SYMBOLS = ["TestChild"];
 
 class TestChild extends JSWindowActorChild {
   constructor() {
      super();
   }
 
   receiveMessage(aMessage) {
@@ -38,19 +36,9 @@ class TestChild extends JSWindowActorChi
 
   observe(subject, topic, data) {
     this.lastObserved = {subject, topic, data};
   }
 
   show() {
     return "TestChild";
   }
-
-  willDestroy() {
-    Services.obs.notifyObservers(
-      this, "test-js-window-actor-willdestroy", true);
-  }
-
-  didDestroy() {
-    Services.obs.notifyObservers(
-      this, "test-js-window-actor-diddestroy", true);
-  }
 }