Bug 1492308 - 1. Generate natives binding for all JNIObject classes; r=snorp, a=pascalc
authorJim Chen <nchen@mozilla.com>
Tue, 02 Oct 2018 19:59:28 +0000
changeset 490178 00ea8935b012372f3623ef15b4506b7f69f983bd
parent 490177 24ee15badf0605bb429c87566eaf4f58c56e73b1
child 490179 b75cfee22e1e01acd8f2d87780d787d43eeb15e7
push id9937
push userryanvm@gmail.com
push dateFri, 05 Oct 2018 15:10:08 +0000
treeherdermozilla-beta@01237eeedfac [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssnorp, pascalc
bugs1492308
milestone63.0
Bug 1492308 - 1. Generate natives binding for all JNIObject classes; r=snorp, a=pascalc Right now we skip generating natives binding for a class if the class doesn't have native methods. However, we should still generate the natives binding for JNIObject classes because these classes can still be attached to C++ objects even without native methods. Differential Revision: https://phabricator.services.mozilla.com/D7106 Bug 1492308 - 2. Create new native handle before clearing old one; r=snorp Create a new native handle first before deleting any old ones, so we can be sure the new handle would not have the same value as the old handle, which can confuse our code. Differential Revision: https://phabricator.services.mozilla.com/D7107 Bug 1492308 - 3. Add test for tapping after content crash; r=snorp Add a test for the crash scenario where PZC is used after content crashes and restarts. Differential Revision: https://phabricator.services.mozilla.com/D7108 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 Bug 1492308 - 5. Make various objects use the new disposal mechanism; r=snorp Make LayerViewSupport, NPZCSupport, and GeckoEditableSupport use the new disposal mechanism to ensure the disposal is performed safely. Differential Revision: https://phabricator.services.mozilla.com/D7110
mobile/android/annotations/src/main/java/org/mozilla/gecko/annotationProcessors/CodeGenerator.java
mobile/android/annotations/src/main/java/org/mozilla/gecko/annotationProcessors/utils/Utils.java
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/ContentDelegateTest.kt
mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/LayerSession.java
mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/PanZoomController.java
widget/android/GeckoEditableSupport.cpp
widget/android/GeckoEditableSupport.h
widget/android/jni/Natives.h
widget/android/nsWindow.cpp
widget/android/nsWindow.h
--- a/mobile/android/annotations/src/main/java/org/mozilla/gecko/annotationProcessors/CodeGenerator.java
+++ b/mobile/android/annotations/src/main/java/org/mozilla/gecko/annotationProcessors/CodeGenerator.java
@@ -561,49 +561,53 @@ public class CodeGenerator {
      * @return The bytes to be written to the wrappers file.
      */
     public String getWrapperFileContents() {
         cpp.append(
                 Utils.getIfdefFooter(options.ifdef));
         return cpp.toString();
     }
 
+    private boolean haveNatives() {
+        return nativesInits.length() > 0 || Utils.isJNIObject(cls);
+    }
+
     /**
      * Get the finalised bytes to go into the generated header file.
      *
      * @return The bytes to be written to the header file.
      */
     public String getHeaderFileContents() {
         if (this.callingThread == null) {
             this.callingThread = AnnotationInfo.CallingThread.ANY;
         }
 
         header.append(
                 "    static const mozilla::jni::CallingThread callingThread =\n" +
                 "            " + this.callingThread.nativeValue() + ";\n" +
                 "\n");
 
-        if (nativesInits.length() > 0) {
+        if (haveNatives()) {
             header.append(
                     "    template<class Impl> class Natives;\n");
         }
         header.append(
                 "};\n" +
                 "\n" +
                 Utils.getIfdefFooter(options.ifdef));
         return header.toString();
     }
 
     /**
      * Get the finalised bytes to go into the generated natives header file.
      *
      * @return The bytes to be written to the header file.
      */
     public String getNativesFileContents() {
-        if (nativesInits.length() == 0) {
+        if (!haveNatives()) {
             return "";
         }
         natives.append(
                 "    static const JNINativeMethod methods[" + numNativesInits + "];\n" +
                 "};\n" +
                 "\n" +
                 "template<class Impl>\n" +
                 "const JNINativeMethod " + clsName + "::Natives<Impl>::methods[] = {" + nativesInits + '\n' +
--- a/mobile/android/annotations/src/main/java/org/mozilla/gecko/annotationProcessors/utils/Utils.java
+++ b/mobile/android/annotations/src/main/java/org/mozilla/gecko/annotationProcessors/utils/Utils.java
@@ -338,9 +338,18 @@ public class Utils {
     }
 
     public static String getIfdefFooter(String ifdef) {
         if (ifdef.isEmpty()) {
             return "";
         }
         return "#endif // " + ifdef + "\n";
     }
+
+    public static boolean isJNIObject(Class<?> cls) {
+        for (; cls != null; cls = cls.getSuperclass()) {
+            if (cls.getName().equals("org.mozilla.gecko.mozglue.JNIObject")) {
+                return true;
+            }
+        }
+        return false;
+    }
 }
--- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/ContentDelegateTest.kt
+++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/ContentDelegateTest.kt
@@ -101,16 +101,43 @@ class ContentDelegateTest : BaseSessionT
             override fun onPageStop(session: GeckoSession, success: Boolean) {
                 assertThat("Page should load successfully", success, equalTo(true))
             }
         })
     }
 
     @IgnoreCrash
     @ReuseSession(false)
+    @WithDisplay(width = 10, height = 10)
+    @Test fun crashContent_tapAfterCrash() {
+        // This test doesn't make sense without multiprocess
+        assumeThat(sessionRule.env.isMultiprocess, equalTo(true))
+        // Cannot test x86 debug builds due to Gecko's "ah_crap_handler"
+        // that waits for debugger to attach during a SIGSEGV.
+        assumeThat(sessionRule.env.isDebugBuild && sessionRule.env.cpuArch == "x86",
+                   equalTo(false))
+
+        mainSession.delegateUntilTestEnd(object : Callbacks.ContentDelegate {
+            override fun onCrash(session: GeckoSession) {
+                mainSession.open()
+                mainSession.loadTestPath(HELLO_HTML_PATH)
+            }
+        })
+
+        mainSession.synthesizeTap(5, 5)
+        mainSession.loadUri(CONTENT_CRASH_URL)
+        mainSession.waitForPageStop()
+
+        mainSession.synthesizeTap(5, 5)
+        mainSession.reload()
+        mainSession.waitForPageStop()
+    }
+
+    @IgnoreCrash
+    @ReuseSession(false)
     @Test fun crashContentMultipleSessions() {
         // This test doesn't make sense without multiprocess
         assumeThat(sessionRule.env.isMultiprocess, equalTo(true))
         // Cannot test x86 debug builds due to Gecko's "ah_crap_handler"
         // that waits for debugger to attach during a SIGSEGV.
         assumeThat(sessionRule.env.isDebugBuild && sessionRule.env.cpuArch == "x86",
                    equalTo(false))
 
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/LayerSession.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/LayerSession.java
@@ -65,21 +65,22 @@ public class LayerSession {
         private void onCompositorAttached() {
             LayerSession.this.onCompositorAttached();
         }
 
         @WrapForJNI(calledFrom = "ui")
         private void onCompositorDetached() {
             // Clear out any pending calls on the UI thread.
             LayerSession.this.onCompositorDetached();
-            disposeNative();
         }
 
-        @WrapForJNI(calledFrom = "ui", dispatchTo = "gecko")
-        @Override protected native void disposeNative();
+        @Override protected void disposeNative() {
+            // Disposal happens in native code.
+            throw new UnsupportedOperationException();
+        }
 
         @WrapForJNI(calledFrom = "ui", dispatchTo = "gecko")
         public native void attachNPZC(PanZoomController npzc);
 
         @WrapForJNI(calledFrom = "ui", dispatchTo = "gecko")
         public native void onBoundsChanged(int left, int top, int width, int height);
 
         // Gecko thread pauses compositor; blocks UI thread.
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/PanZoomController.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/PanZoomController.java
@@ -265,23 +265,25 @@ public final class PanZoomController ext
 
     @WrapForJNI(calledFrom = "ui")
     private void setAttached(final boolean attached) {
         if (attached) {
             mAttached = true;
             flushEventQueue();
         } else if (mAttached) {
             mAttached = false;
-            disposeNative();
             enableEventQueue();
         }
     }
 
-    @WrapForJNI(calledFrom = "ui", dispatchTo = "gecko") @Override // JNIObject
-    protected native void disposeNative();
+    @Override // JNIObject
+    protected void disposeNative() {
+        // Disposal happens in native code.
+        throw new UnsupportedOperationException();
+    }
 
     @WrapForJNI(stubName = "SetIsLongpressEnabled") // Called from test thread.
     private native void nativeSetIsLongpressEnabled(boolean isLongpressEnabled);
 
     /**
      * Set whether Gecko should generate long-press events.
      *
      * @param isLongpressEnabled True if Gecko should generate long-press events.
--- a/widget/android/GeckoEditableSupport.cpp
+++ b/widget/android/GeckoEditableSupport.cpp
@@ -1380,17 +1380,20 @@ GeckoEditableSupport::NotifyIME(TextEven
 
 void
 GeckoEditableSupport::OnRemovedFrom(TextEventDispatcher* aTextEventDispatcher)
 {
     mDispatcher = nullptr;
 
     if (mIsRemote) {
         // When we're remote, detach every time.
-        OnDetach();
+        OnDetach(NS_NewRunnableFunction("GeckoEditableSupport::OnRemovedFrom",
+                 [editable = java::GeckoEditableChild::GlobalRef(mEditable)] {
+                     DisposeNative(editable);
+                 }));
     }
 }
 
 void
 GeckoEditableSupport::WillDispatchKeyboardEvent(
         TextEventDispatcher* aTextEventDispatcher,
         WidgetKeyboardEvent& aKeyboardEvent, uint32_t aIndexOfKeypress,
         void* aData)
--- a/widget/android/GeckoEditableSupport.h
+++ b/widget/android/GeckoEditableSupport.h
@@ -206,22 +206,27 @@ public:
 
     void SetInputContext(const InputContext& aContext,
                          const InputContextAction& aAction);
 
     InputContext GetInputContext();
 
     // GeckoEditableChild methods
     using EditableBase::AttachNative;
+    using EditableBase::DisposeNative;
 
-    void OnDetach() {
+    const java::GeckoEditableChild::Ref& GetJavaEditable() { return mEditable; }
+
+    void OnDetach(already_AddRefed<Runnable> aDisposer)
+    {
         RefPtr<GeckoEditableSupport> self(this);
-        nsAppShell::PostEvent([this, self] {
+        nsAppShell::PostEvent([this, self,
+                               disposer = RefPtr<Runnable>(aDisposer)] {
             mEditableAttached = false;
-            DisposeNative(mEditable);
+            disposer->Run();
         });
     }
 
     // Handle an Android KeyEvent.
     void OnKeyEvent(int32_t aAction, int32_t aKeyCode, int32_t aScanCode,
                     int32_t aMetaState, int32_t aKeyPressMetaState,
                     int64_t aTime, int32_t aDomPrintableKeyValue,
                     int32_t aRepeatCount, int32_t aFlags,
--- a/widget/android/jni/Natives.h
+++ b/widget/android/jni/Natives.h
@@ -220,19 +220,22 @@ struct NativePtr<Impl, /* Type = */ Nati
     static Impl* Get(const LocalRef& instance)
     {
         return Get(instance.Env(), instance.Get());
     }
 
     template<class LocalRef>
     static void Set(const LocalRef& instance, Impl* ptr)
     {
+        // Create the new handle first before clearing any old handle, so the
+        // new handle is guaranteed to have different value than any old handle.
+        const uintptr_t handle =
+                reinterpret_cast<uintptr_t>(new WeakPtr<Impl>(ptr));
         Clear(instance);
-        SetNativeHandle(instance.Env(), instance.Get(),
-                          reinterpret_cast<uintptr_t>(new WeakPtr<Impl>(ptr)));
+        SetNativeHandle(instance.Env(), instance.Get(), handle);
         MOZ_CATCH_JNI_EXCEPTION(instance.Env());
     }
 
     template<class LocalRef>
     static void Clear(const LocalRef& instance)
     {
         const auto ptr = reinterpret_cast<WeakPtr<Impl>*>(
                 GetNativeHandle(instance.Env(), instance.Get()));
@@ -265,19 +268,22 @@ struct NativePtr<Impl, /* Type = */ Nati
     static Impl* Get(const LocalRef& instance)
     {
         return Get(instance.Env(), instance.Get());
     }
 
     template<class LocalRef>
     static void Set(const LocalRef& instance, Impl* ptr)
     {
+        // Create the new handle first before clearing any old handle, so the
+        // new handle is guaranteed to have different value than any old handle.
+        const uintptr_t handle =
+                reinterpret_cast<uintptr_t>(new RefPtr<Impl>(ptr));
         Clear(instance);
-        SetNativeHandle(instance.Env(), instance.Get(),
-                        reinterpret_cast<uintptr_t>(new RefPtr<Impl>(ptr)));
+        SetNativeHandle(instance.Env(), instance.Get(), handle);
         MOZ_CATCH_JNI_EXCEPTION(instance.Env());
     }
 
     template<class LocalRef>
     static void Clear(const LocalRef& instance)
     {
         const auto ptr = reinterpret_cast<RefPtr<Impl>*>(
                 GetNativeHandle(instance.Env(), instance.Get()));
--- a/widget/android/nsWindow.cpp
+++ b/widget/android/nsWindow.cpp
@@ -187,38 +187,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;
@@ -388,17 +439,17 @@ public:
     }
 
     ~NPZCSupport()
     {}
 
     using Base::AttachNative;
     using Base::DisposeNative;
 
-    void OnDetach()
+    void OnDetach(already_AddRefed<Runnable> aDisposer)
     {
         // There are several considerations when shutting down NPZC. 1) The
         // Gecko thread may destroy NPZC at any time when nsWindow closes. 2)
         // There may be pending events on the Gecko thread when NPZC is
         // destroyed. 3) mWindow may not be available when the pending event
         // runs. 4) The UI thread may destroy NPZC at any time when GeckoView
         // is destroyed. 5) The UI thread may destroy NPZC at the same time as
         // Gecko thread trying to destroy NPZC. 6) There may be pending calls
@@ -422,30 +473,25 @@ public:
         // 6) is solved by keeping a destroyed flag in the Java NPZC instance,
         // and only make a pending call if the destroyed flag is not set.
         //
         // 7) is solved by taking a lock whenever mWindow is modified on the
         // Gecko thread or accessed on the UI thread. That way, we don't
         // release mWindow until the UI thread is done using it, thus avoiding
         // the race condition.
 
-        typedef PanZoomController::GlobalRef NPZCRef;
-        auto callDestroy = [] (const NPZCRef& npzc) {
-            npzc->SetAttached(false);
-        };
-
-        PanZoomController::GlobalRef npzc = mNPZC;
-        RefPtr<nsThread> uiThread = GetAndroidUiThread();
-        if (!uiThread) {
-            return;
+        if (RefPtr<nsThread> uiThread = GetAndroidUiThread()) {
+            uiThread->Dispatch(NS_NewRunnableFunction(
+                        "NPZCSupport::OnDetach",
+                        [npzc = PanZoomController::GlobalRef(mNPZC),
+                         disposer = RefPtr<Runnable>(aDisposer)] {
+                            npzc->SetAttached(false);
+                            disposer->Run();
+                        }));
         }
-        uiThread->Dispatch(NewRunnableFunction(
-                "OnDetachRunnable",
-                static_cast<void(*)(const NPZCRef&)>(callDestroy),
-                std::move(npzc)), nsIThread::DISPATCH_NORMAL);
     }
 
     const PanZoomController::Ref& GetJavaNPZC() const
     {
         return mNPZC;
     }
 
 public:
@@ -840,24 +886,26 @@ public:
     }
 
     ~LayerViewSupport()
     {}
 
     using Base::AttachNative;
     using Base::DisposeNative;
 
-    void OnDetach()
+    void OnDetach(already_AddRefed<Runnable> aDisposer)
     {
         if (RefPtr<nsThread> uiThread = GetAndroidUiThread()) {
-            LayerSession::Compositor::GlobalRef compositor(mCompositor);
             uiThread->Dispatch(NS_NewRunnableFunction(
                     "LayerViewSupport::OnDetach",
-                    [compositor] {
+                    [compositor =
+                            LayerSession::Compositor::GlobalRef(mCompositor),
+                     disposer = RefPtr<Runnable>(aDisposer)] {
                         compositor->OnCompositorDetached();
+                        disposer->Run();
                     }));
         }
     }
 
     const LayerSession::Compositor::Ref& GetJavaCompositor() const
     {
         return mCompositor;
     }
@@ -1119,26 +1167,28 @@ public:
 
 template<> const char
 nsWindow::NativePtr<nsWindow::LayerViewSupport>::sName[] = "LayerViewSupport";
 
 nsWindow::GeckoViewSupport::~GeckoViewSupport()
 {
     // Disassociate our GeckoEditable instance with our native object.
     if (window.mEditableSupport) {
-        window.mEditableSupport.Detach();
+        window.mEditableSupport.Detach(
+                window.mEditableSupport->GetJavaEditable());
         window.mEditableParent = nullptr;
     }
 
     if (window.mNPZCSupport) {
-        window.mNPZCSupport.Detach();
+        window.mNPZCSupport.Detach(window.mNPZCSupport->GetJavaNPZC());
     }
 
     if (window.mLayerViewSupport) {
-        window.mLayerViewSupport.Detach();
+        window.mLayerViewSupport.Detach(
+                window.mLayerViewSupport->GetJavaCompositor());
     }
 }
 
 /* static */ void
 nsWindow::GeckoViewSupport::Open(const jni::Class::LocalRef& aCls,
                                  GeckoSession::Window::Param aWindow,
                                  jni::Object::Param aQueue,
                                  jni::Object::Param aCompositor,
@@ -1231,24 +1281,25 @@ void
 nsWindow::GeckoViewSupport::Transfer(const GeckoSession::Window::LocalRef& inst,
                                      jni::Object::Param aQueue,
                                      jni::Object::Param aCompositor,
                                      jni::Object::Param aDispatcher,
                                      jni::Object::Param aInitData)
 {
     if (window.mNPZCSupport) {
         MOZ_ASSERT(window.mLayerViewSupport);
-        window.mNPZCSupport.Detach();
+        window.mNPZCSupport.Detach(window.mNPZCSupport->GetJavaNPZC());
     }
 
     auto compositor = LayerSession::Compositor::LocalRef(
             inst.Env(), LayerSession::Compositor::Ref::From(aCompositor));
     if (window.mLayerViewSupport &&
             window.mLayerViewSupport->GetJavaCompositor() != compositor) {
-        window.mLayerViewSupport.Detach();
+        window.mLayerViewSupport.Detach(
+                window.mLayerViewSupport->GetJavaCompositor());
     }
     if (!window.mLayerViewSupport) {
         window.mLayerViewSupport.Attach(compositor, &window, compositor);
     }
 
     MOZ_ASSERT(window.mAndroidView);
     window.mAndroidView->mEventDispatcher->Attach(
             java::EventDispatcher::Ref::From(aDispatcher), mDOMWindow);
@@ -1273,17 +1324,18 @@ void
 nsWindow::GeckoViewSupport::AttachEditable(const GeckoSession::Window::LocalRef& inst,
                                            jni::Object::Param aEditableParent,
                                            jni::Object::Param aEditableChild)
 {
     java::GeckoEditableChild::LocalRef editableChild(inst.Env());
     editableChild = java::GeckoEditableChild::Ref::From(aEditableChild);
 
     if (window.mEditableSupport) {
-        window.mEditableSupport.Detach();
+        window.mEditableSupport.Detach(
+                window.mEditableSupport->GetJavaEditable());
     }
 
     window.mEditableSupport.Attach(editableChild, &window, editableChild);
     window.mEditableParent = aEditableParent;
 }
 
 void
 nsWindow::InitNatives()
--- a/widget/android/nsWindow.h
+++ b/widget/android/nsWindow.h
@@ -74,40 +74,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;