Bug 1827386 - Part 1. NativeWeakPtr::Detach returns MozPromise to detect whether dispoer is finished. r=geckoview-reviewers,owlish

Actually, `NativeWeakPtr::Detach` may not release JNI ojbect immediately because
it depends on JNI object's `OnWeakNonIntrusiveDetach`i implementation.
`SessionAccessibility`'s `OnWeakNonIntrusiveDetach` implementation uses the
runnable to run on Android UI thread then disposer runs on main thread, so
if Detach is finished, JNI object isn't detached yet.

If calling `NativeWeakPtrHolder::Attach` immediately with same/recycled Java
object after `NettiveWeakPtr::Detach`, it is possible to run disposer for JNI
object by `OnWeakNonIntrusiveDetach` after Attach is finished. So it may
release newer attached object unfortunately.

So I would like to add a way to waiting for detach JNI object using
`MozPromise`.

Also, `MozPromise.h` includes `Natives.h` header (for `GeckoResult` support).
So I cannot modify inline method to use `MozPromise` due to recursive. So I
split implementation with `NativesInlines.h` as workaround.

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

accessible/android/SessionAccessibility.cpp

@@ -30,6 +30,7 @@
 #include "mozilla/a11y/DocAccessiblePlatformExtParent.h"
 #include "mozilla/a11y/DocManager.h"
 #include "mozilla/jni/GeckoBundleUtils.h"
+#include "mozilla/jni/NativesInlines.h"
 #include "mozilla/widget/GeckoViewSupport.h"
 #include "mozilla/MouseEvents.h"
 #include "mozilla/dom/MouseEventBinding.h"

widget/android/GeckoEditableSupport.cpp

@@ -17,6 +17,7 @@
 #include "mozilla/IMEStateManager.h"
 #include "mozilla/java/GeckoEditableChildWrappers.h"
 #include "mozilla/java/GeckoServiceChildProcessWrappers.h"
+#include "mozilla/jni/NativesInlines.h"
 #include "mozilla/Logging.h"
 #include "mozilla/MiscEvents.h"
 #include "mozilla/Preferences.h"

widget/android/jni/Natives.h

@@ -44,6 +44,10 @@ static NativeException NullWeakPtr() {
 }
 
 namespace mozilla {
+
+template <typename ResolveValueT, typename RejectValueT, bool IsExclusive>
+class MozPromise;
+
 namespace jni {
 
 /**
@@ -623,71 +627,6 @@ class MOZ_HEAP_CLASS NativeWeakPtrControlBlock final {
   StorageType mNativeImpl;
 };
 
-/**
- * When a NativeWeakPtr is detached from its owning Java object, the calling
- * thread invokes the implementation's OnWeakNonIntrusiveDetach to perform
- * cleanup. We complete the remainder of the cleanup sequence on the Gecko
- * main thread by expecting OnWeakNonIntrusiveDetach implementations to invoke
- * this Runnable before exiting. It will move itself to the main thread if it
- * is not already there.
- */
-template <typename NativeImpl>
-class NativeWeakPtrDetachRunnable final : public Runnable {
- public:
-  NativeWeakPtrDetachRunnable(
-      already_AddRefed<detail::NativeWeakPtrControlBlock<NativeImpl>> aCtlBlock,
-      const Object::LocalRef& aOwner,
-      typename NativeWeakPtrControlBlockStorageTraits<NativeImpl>::Type
-          aNativeImpl)
-      : Runnable("mozilla::jni::detail::NativeWeakPtrDetachRunnable"),
-        mCtlBlock(aCtlBlock),
-        mOwner(aOwner),
-        mNativeImpl(std::move(aNativeImpl)),
-        mHasRun(false) {
-    MOZ_RELEASE_ASSERT(!!mCtlBlock);
-    MOZ_RELEASE_ASSERT(!!mNativeImpl);
-  }
-
-  NS_INLINE_DECL_REFCOUNTING_INHERITED(NativeWeakPtrDetachRunnable, Runnable)
-
-  NS_IMETHOD Run() override {
-    mHasRun = true;
-
-    if (!NS_IsMainThread()) {
-      NS_DispatchToMainThread(this);
-      return NS_OK;
-    }
-
-    // Get the owner object's native implementation
-    auto owner = ToLocalRef(mOwner);
-    auto attachedNativeImpl = NativePtrTraits<NativeImpl>::Get(owner);
-    MOZ_RELEASE_ASSERT(!!attachedNativeImpl);
-
-    // NativePtrTraits::ClearFinish cleans out the JNIObject's handle, which
-    // obviously we don't want to attempt unless that handle still points to
-    // our native implementation.
-    if (attachedNativeImpl->IsSame(mCtlBlock)) {
-      NativePtrTraits<NativeImpl>::ClearFinish(owner);
-    }
-
-    // Now we destroy that native object.
-    mNativeImpl = nullptr;
-    return NS_OK;
-  }
-
- private:
-  ~NativeWeakPtrDetachRunnable() {
-    // Guard against somebody forgetting to call this runnable.
-    MOZ_RELEASE_ASSERT(mHasRun, "You must run/dispatch this runnable!");
-  }
-
- private:
-  RefPtr<detail::NativeWeakPtrControlBlock<NativeImpl>> mCtlBlock;
-  Object::GlobalRef mOwner;
-  typename NativeWeakPtrControlBlockStorageTraits<NativeImpl>::Type mNativeImpl;
-  bool mHasRun;
-};
-
 /**
  * If you want to temporarily access the object held by a NativeWeakPtr, you
  * must obtain one of these Accessor objects from the pointer. Access must
@@ -755,6 +694,8 @@ class MOZ_STACK_CLASS Accessor final {
 
 }  // namespace detail
 
+using DetachPromise = mozilla::MozPromise<bool, nsresult, true>;
+
 /**
  * This class implements support for thread-safe weak pointers to native objects
  * that are owned by Java objects deriving from JNIObject.
@@ -791,31 +732,7 @@ class NativeWeakPtr {
    * Detach the underlying object's strong reference from its owning Java object
    * and clean it up.
    */
-  void Detach() {
-    if (!IsAttached()) {
-      // Never attached to begin with; no-op
-      return;
-    }
-
-    auto native = mCtlBlock->Clear();
-    if (!native) {
-      // Detach already in progress
-      return;
-    }
-
-    Object::LocalRef owner(mCtlBlock->GetJavaOwner());
-    MOZ_RELEASE_ASSERT(!!owner);
-
-    // Save the raw pointer before we move native into the runnable so that we
-    // may call OnWeakNonIntrusiveDetach on it even after moving native into
-    // the runnable.
-    NativeImpl* rawImpl =
-        detail::NativeWeakPtrControlBlock<NativeImpl>::StorageTraits::AsRaw(
-            native);
-    rawImpl->OnWeakNonIntrusiveDetach(
-        do_AddRef(new NativeWeakPtrDetachRunnable<NativeImpl>(
-            mCtlBlock.forget(), owner, std::move(native))));
-  }
+  RefPtr<DetachPromise> Detach();
 
   /**
    * This method does not indicate whether or not the weak pointer is still

widget/android/jni/NativesInlines.h

@@ -0,0 +1,116 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 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/. */
+
+#include "Natives.h"
+
+#include "mozilla/MozPromise.h"
+
+namespace mozilla::jni {
+
+namespace details {
+
+/**
+ * When a NativeWeakPtr is detached from its owning Java object, the calling
+ * thread invokes the implementation's OnWeakNonIntrusiveDetach to perform
+ * cleanup. We complete the remainder of the cleanup sequence on the Gecko
+ * main thread by expecting OnWeakNonIntrusiveDetach implementations to invoke
+ * this Runnable before exiting. It will move itself to the main thread if it
+ * is not already there.
+ */
+template <typename NativeImpl>
+class NativeWeakPtrDetachRunnable final : public Runnable {
+ public:
+  NativeWeakPtrDetachRunnable(
+      already_AddRefed<detail::NativeWeakPtrControlBlock<NativeImpl>> aCtlBlock,
+      const Object::LocalRef& aOwner,
+      typename NativeWeakPtrControlBlockStorageTraits<NativeImpl>::Type
+          aNativeImpl)
+      : Runnable("mozilla::jni::detail::NativeWeakPtrDetachRunnable"),
+        mCtlBlock(aCtlBlock),
+        mOwner(aOwner),
+        mNativeImpl(std::move(aNativeImpl)),
+        mHasRun(false) {
+    MOZ_RELEASE_ASSERT(!!mCtlBlock);
+    MOZ_RELEASE_ASSERT(!!mNativeImpl);
+  }
+
+  NS_INLINE_DECL_REFCOUNTING_INHERITED(NativeWeakPtrDetachRunnable, Runnable)
+
+  NS_IMETHOD Run() override {
+    mHasRun = true;
+
+    if (!NS_IsMainThread()) {
+      NS_DispatchToMainThread(this);
+      return NS_OK;
+    }
+
+    // Get the owner object's native implementation
+    auto owner = ToLocalRef(mOwner);
+    auto attachedNativeImpl = NativePtrTraits<NativeImpl>::Get(owner);
+    MOZ_RELEASE_ASSERT(!!attachedNativeImpl);
+
+    // NativePtrTraits::ClearFinish cleans out the JNIObject's handle, which
+    // obviously we don't want to attempt unless that handle still points to
+    // our native implementation.
+    if (attachedNativeImpl->IsSame(mCtlBlock)) {
+      NativePtrTraits<NativeImpl>::ClearFinish(owner);
+    }
+
+    // Now we destroy that native object.
+    mNativeImpl = nullptr;
+    mHolder.Resolve(true, __func__);
+    return NS_OK;
+  }
+
+  RefPtr<DetachPromise> GetPromise() { return mHolder.Ensure(__func__); }
+
+ private:
+  ~NativeWeakPtrDetachRunnable() {
+    // Guard against somebody forgetting to call this runnable.
+    MOZ_RELEASE_ASSERT(mHasRun, "You must run/dispatch this runnable!");
+  }
+
+ private:
+  RefPtr<detail::NativeWeakPtrControlBlock<NativeImpl>> mCtlBlock;
+  Object::GlobalRef mOwner;
+  MozPromiseHolder<DetachPromise> mHolder;
+  typename NativeWeakPtrControlBlockStorageTraits<NativeImpl>::Type mNativeImpl;
+  bool mHasRun;
+};
+
+}  // namespace details
+
+template <typename NativeImpl>
+RefPtr<DetachPromise> NativeWeakPtr<NativeImpl>::Detach() {
+  if (!IsAttached()) {
+    // Never attached to begin with; no-op
+    return DetachPromise::CreateAndResolve(true, __func__);
+  }
+
+  auto native = mCtlBlock->Clear();
+  if (!native) {
+    // Detach already in progress
+    return DetachPromise::CreateAndResolve(true, __func__);
+  }
+
+  Object::LocalRef owner(mCtlBlock->GetJavaOwner());
+  MOZ_RELEASE_ASSERT(!!owner);
+
+  // Save the raw pointer before we move native into the runnable so that we
+  // may call OnWeakNonIntrusiveDetach on it even after moving native into
+  // the runnable.
+  NativeImpl* rawImpl =
+      detail::NativeWeakPtrControlBlock<NativeImpl>::StorageTraits::AsRaw(
+          native);
+  RefPtr<details::NativeWeakPtrDetachRunnable<NativeImpl>> runnable =
+      new details::NativeWeakPtrDetachRunnable<NativeImpl>(
+          mCtlBlock.forget(), owner, std::move(native));
+  RefPtr<DetachPromise> promise = runnable->GetPromise();
+  rawImpl->OnWeakNonIntrusiveDetach(runnable.forget());
+  return promise;
+}
+
+}  // namespace mozilla::jni

widget/android/jni/moz.build

@@ -13,6 +13,7 @@ EXPORTS.mozilla.jni += [
     "GeckoBundleUtils.h",
     "GeckoResultUtils.h",
     "Natives.h",
+    "NativesInlines.h",
     "Refs.h",
     "TypeAdapter.h",
     "Types.h",

widget/android/nsWindow.cpp

@@ -90,6 +90,7 @@
 #include "mozilla/java/PanZoomControllerNatives.h"
 #include "mozilla/java/SessionAccessibilityWrappers.h"
 #include "mozilla/java/SurfaceControlManagerWrappers.h"
+#include "mozilla/jni/NativesInlines.h"
 #include "mozilla/layers/APZEventState.h"
 #include "mozilla/layers/APZInputBridge.h"
 #include "mozilla/layers/APZThreadUtils.h"