Bug 1492308 - 4. Add new mechanism for safely disposing native objects; r=snorp
authorJim Chen <nchen@mozilla.com>
Tue, 02 Oct 2018 19:59:37 +0000
changeset 495021 17a45b4d6fd5c7f36c6ba8004e615d4d348c3cbd
parent 495020 7893e8aae29590b426a71fb7fe151dc75b385143
child 495022 005e374d721ecb778d3a8293e6a1c033c4916e0a
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssnorp
bugs1492308
milestone64.0a1
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
Bug 1492308 - 4. Add new mechanism for safely disposing native objects; r=snorp Add a new mechanism in NativePtr::Detach to safely dispose objects. A new disposer Runnable is passed to OnDetach functions. The OnDetach functions invoke the disposer after changing the object state. The disposer then makes sure the object hasn't been attached to another native object in the meantime. Disposal is only performed when the current native object matches the original one. Differential Revision: https://phabricator.services.mozilla.com/D7109
widget/android/nsWindow.cpp
widget/android/nsWindow.h
--- a/widget/android/nsWindow.cpp
+++ b/widget/android/nsWindow.cpp
@@ -188,38 +188,89 @@ namespace {
             uiThread->Dispatch(NS_NewRunnableFunction(aName, std::move(aLambda)));
             return true;
         }
         return false;
     }
 } // namespace
 
 template<class Impl>
-template<class Instance, typename... Args> void
-nsWindow::NativePtr<Impl>::Attach(Instance aInstance, nsWindow* aWindow,
-                                  Args&&... aArgs)
+template<class Cls, typename... Args> void
+nsWindow::NativePtr<Impl>::Attach(const jni::LocalRef<Cls>& aInstance,
+                                  nsWindow* aWindow, Args&&... aArgs)
 {
     MOZ_ASSERT(NS_IsMainThread());
     MOZ_ASSERT(!mPtr && !mImpl);
 
     Impl* const impl = new Impl(
             this, aWindow, std::forward<Args>(aArgs)...);
     mImpl = impl;
 
     // CallAttachNative transfers ownership of impl.
-    CallAttachNative<Instance, Impl>(aInstance, impl);
+    CallAttachNative<>(aInstance, impl);
 }
 
-template<class Impl> void
-nsWindow::NativePtr<Impl>::Detach()
+template<class Impl>
+template<class Cls, typename T> void
+nsWindow::NativePtr<Impl>::Detach(const jni::Ref<Cls, T>& aInstance)
 {
     MOZ_ASSERT(NS_IsMainThread());
     MOZ_ASSERT(mPtr && mImpl);
 
-    mImpl->OnDetach();
+    // nsIRunnable that takes care of disposing the native object attached to
+    // the Java object in a safe manner.
+    class ImplDisposer : public Runnable {
+        const typename Cls::GlobalRef mInstance;
+        const uintptr_t mOldImpl;
+
+    public:
+        ImplDisposer(const typename Cls::LocalRef& aInstance)
+            : Runnable("nsWindow::NativePtr::Detach")
+            , mInstance(aInstance.Env(), aInstance)
+            , mOldImpl(aInstance ? jni::GetNativeHandle(aInstance.Env(),
+                                                        aInstance.Get()) : 0)
+        {
+            MOZ_CATCH_JNI_EXCEPTION(aInstance.Env());
+        }
+
+        NS_IMETHOD Run() override
+        {
+            if (!mInstance) {
+                return NS_OK;
+            }
+
+            if (!NS_IsMainThread()) {
+                NS_DispatchToMainThread(this);
+                return NS_OK;
+            }
+
+            typename Cls::LocalRef instance(jni::GetGeckoThreadEnv(),
+                                            mInstance);
+            auto newImpl = jni::GetNativeHandle(instance.Env(), instance.Get());
+            MOZ_CATCH_JNI_EXCEPTION(instance.Env());
+
+            if (mOldImpl == newImpl) {
+                // Only dispose the object if the native object has not changed.
+                Impl::DisposeNative(instance);
+            }
+            return NS_OK;
+        }
+    };
+
+    // Objects that use nsWindow::NativePtr are expected to implement a public
+    // member function with signature "void OnDetach(
+    // already_AddRefed<Runnable> aDisposer)".  This function should perform
+    // necessary cleanups for the native/Java objects, as well as mark the Java
+    // object as being disposed, so no native methods are called after that
+    // point. After this disposal step, the function must call "aDisposer->
+    // Run()" to finish disposing the native object. The disposer is
+    // thread-safe and may be called on any thread as necessary.
+    mImpl->OnDetach(do_AddRef(new ImplDisposer(
+            {jni::GetGeckoThreadEnv(), aInstance})));
+
     {
         Locked implLock(*this);
         mImpl = nullptr;
     }
 
     typename WindowPtr<Impl>::Locked lock(*mPtr);
     mPtr->mWindow = nullptr;
     mPtr->mPtr = nullptr;
--- a/widget/android/nsWindow.h
+++ b/widget/android/nsWindow.h
@@ -78,40 +78,43 @@ public:
     template<class Impl> class WindowPtr;
 
     // Smart pointer for holding a pointer to a native object class. The
     // pointer is automatically cleared when the object is destroyed.
     template<class Impl>
     class NativePtr final
     {
         friend WindowPtr<Impl>;
+        friend nsWindow;
 
         static const char sName[];
 
         WindowPtr<Impl>* mPtr;
         Impl* mImpl;
         mozilla::Mutex mImplLock;
 
+        NativePtr() : mPtr(nullptr), mImpl(nullptr), mImplLock(sName) {}
+        ~NativePtr() { MOZ_ASSERT(!mPtr); }
+
     public:
         class Locked;
 
-        NativePtr() : mPtr(nullptr), mImpl(nullptr), mImplLock(sName) {}
-        ~NativePtr() { MOZ_ASSERT(!mPtr); }
-
         operator Impl*() const
         {
             MOZ_ASSERT(NS_IsMainThread());
             return mImpl;
         }
 
         Impl* operator->() const { return operator Impl*(); }
 
-        template<class Instance, typename... Args>
-        void Attach(Instance aInstance, nsWindow* aWindow, Args&&... aArgs);
-        void Detach();
+        template<class Cls, typename... Args>
+        void Attach(const mozilla::jni::LocalRef<Cls>& aInstance,
+                    nsWindow* aWindow, Args&&... aArgs);
+        template<class Cls, typename T>
+        void Detach(const mozilla::jni::Ref<Cls, T>& aInstance);
     };
 
     template<class Impl>
     class WindowPtr final
     {
         friend NativePtr<Impl>;
 
         NativePtr<Impl>* mPtr;