From 41261b6bd43f801cdd52f1f2ccdd9f05e5de207d Mon Sep 17 00:00:00 2001
From: James Teh <jteh@mozilla.com>
Date: Tue, 28 May 2019 17:42:59 +0000
Subject: [PATCH] Bug 1543282 part 1: Reference counting for
 DocAccessibleParent. r=eeejay,nika

Supporting out-of-process iframes requires us to hold onto a DocAccessibleParent in BrowserBridgeParent.
However, we can't guarantee the order of cleanup between the two content processes.
Therefore, we need reference counting to kee the object alive.

Differential Revision: https://phabricator.services.mozilla.com/D32277

--HG--
extra : moz-landing-system : lando
---
 accessible/ipc/DocAccessibleParent.cpp | 16 +++++++++++-----
 accessible/ipc/DocAccessibleParent.h   | 16 ++++++++--------
 dom/ipc/BrowserParent.cpp              |  6 ++++--
 3 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/accessible/ipc/DocAccessibleParent.cpp b/accessible/ipc/DocAccessibleParent.cpp
index adb9175449a9..0c92c15d6cbc 100644
--- a/accessible/ipc/DocAccessibleParent.cpp
+++ b/accessible/ipc/DocAccessibleParent.cpp
@@ -676,12 +676,17 @@ void DocAccessibleParent::MaybeInitWindowEmulation() {
     isActive = browserParent->GetDocShellIsActive();
   }
 
-  nsWinUtils::NativeWindowCreateProc onCreate([this](HWND aHwnd) -> void {
+  // onCreate is guaranteed to be called synchronously by
+  // nsWinUtils::CreateNativeWindow, so this reference isn't really necessary.
+  // However, static analysis complains without it.
+  RefPtr<DocAccessibleParent> thisRef = this;
+  nsWinUtils::NativeWindowCreateProc onCreate([thisRef](HWND aHwnd) -> void {
     IDispatchHolder hWndAccHolder;
 
-    ::SetPropW(aHwnd, kPropNameDocAccParent, reinterpret_cast<HANDLE>(this));
+    ::SetPropW(aHwnd, kPropNameDocAccParent,
+               reinterpret_cast<HANDLE>(thisRef.get()));
 
-    SetEmulatedWindowHandle(aHwnd);
+    thisRef->SetEmulatedWindowHandle(aHwnd);
 
     RefPtr<IAccessible> hwndAcc;
     if (SUCCEEDED(::AccessibleObjectFromWindow(
@@ -692,8 +697,9 @@ void DocAccessibleParent::MaybeInitWindowEmulation() {
           mscom::ToProxyUniquePtr(std::move(wrapped))));
     }
 
-    Unused << SendEmulatedWindow(
-        reinterpret_cast<uintptr_t>(mEmulatedWindowHandle), hWndAccHolder);
+    Unused << thisRef->SendEmulatedWindow(
+        reinterpret_cast<uintptr_t>(thisRef->mEmulatedWindowHandle),
+        hWndAccHolder);
   });
 
   HWND parentWnd = reinterpret_cast<HWND>(rootDocument->GetNativeWindow());
diff --git a/accessible/ipc/DocAccessibleParent.h b/accessible/ipc/DocAccessibleParent.h
index 6d02ed621f03..71fed39b8293 100644
--- a/accessible/ipc/DocAccessibleParent.h
+++ b/accessible/ipc/DocAccessibleParent.h
@@ -26,6 +26,8 @@ class xpcAccessibleGeneric;
 class DocAccessibleParent : public ProxyAccessible,
                             public PDocAccessibleParent {
  public:
+  NS_INLINE_DECL_REFCOUNTING(DocAccessibleParent);
+
   DocAccessibleParent()
       : ProxyAccessible(this),
         mParentDoc(kNoParentDoc),
@@ -34,20 +36,12 @@ class DocAccessibleParent : public ProxyAccessible,
 #endif  // defined(XP_WIN)
         mTopLevel(false),
         mShutdown(false) {
-    MOZ_COUNT_CTOR_INHERITED(DocAccessibleParent, ProxyAccessible);
     sMaxDocID++;
     mActorID = sMaxDocID;
     MOZ_ASSERT(!LiveDocs().Get(mActorID));
     LiveDocs().Put(mActorID, this);
   }
 
-  ~DocAccessibleParent() {
-    LiveDocs().Remove(mActorID);
-    MOZ_COUNT_DTOR_INHERITED(DocAccessibleParent, ProxyAccessible);
-    MOZ_ASSERT(mChildDocs.Length() == 0);
-    MOZ_ASSERT(!ParentDoc());
-  }
-
   void SetTopLevel() { mTopLevel = true; }
   bool IsTopLevel() const { return mTopLevel; }
 
@@ -221,6 +215,12 @@ class DocAccessibleParent : public ProxyAccessible,
 #endif
 
  private:
+  ~DocAccessibleParent() {
+    LiveDocs().Remove(mActorID);
+    MOZ_ASSERT(mChildDocs.Length() == 0);
+    MOZ_ASSERT(!ParentDoc());
+  }
+
   class ProxyEntry : public PLDHashEntryHdr {
    public:
     explicit ProxyEntry(const void*) : mProxy(nullptr) {}
diff --git a/dom/ipc/BrowserParent.cpp b/dom/ipc/BrowserParent.cpp
index 1a139036a05b..91a2b4d06ad5 100644
--- a/dom/ipc/BrowserParent.cpp
+++ b/dom/ipc/BrowserParent.cpp
@@ -1059,7 +1059,8 @@ a11y::PDocAccessibleParent* BrowserParent::AllocPDocAccessibleParent(
     PDocAccessibleParent* aParent, const uint64_t&, const uint32_t&,
     const IAccessibleHolder&) {
 #ifdef ACCESSIBILITY
-  return new a11y::DocAccessibleParent();
+  // Reference freed in DeallocPDocAccessibleParent.
+  return do_AddRef(new a11y::DocAccessibleParent()).take();
 #else
   return nullptr;
 #endif
@@ -1067,7 +1068,8 @@ a11y::PDocAccessibleParent* BrowserParent::AllocPDocAccessibleParent(
 
 bool BrowserParent::DeallocPDocAccessibleParent(PDocAccessibleParent* aParent) {
 #ifdef ACCESSIBILITY
-  delete static_cast<a11y::DocAccessibleParent*>(aParent);
+  // Free reference from AllocPDocAccessibleParent.
+  static_cast<a11y::DocAccessibleParent*>(aParent)->Release();
 #endif
   return true;
 }