Bug 1492308 - 5. Make various objects use the new disposal mechanism; r=snorp
authorJim Chen <nchen@mozilla.com>
Tue, 02 Oct 2018 19:59:40 +0000
changeset 495022 005e374d721ecb778d3a8293e6a1c033c4916e0a
parent 495021 17a45b4d6fd5c7f36c6ba8004e615d4d348c3cbd
child 495023 1fa05a1ba7626bbe7e4144b933c490372d818234
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 - 5. Make various objects use the new disposal mechanism; r=snorp Make LayerViewSupport, NPZCSupport, GeckoEditableSupport, and SessionAccessibility use the new disposal mechanism to ensure the disposal is performed safely. Differential Revision: https://phabricator.services.mozilla.com/D7110
accessible/android/SessionAccessibility.cpp
accessible/android/SessionAccessibility.h
mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/LayerSession.java
mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/PanZoomController.java
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/SessionAccessibility.java
widget/android/GeckoEditableSupport.cpp
widget/android/GeckoEditableSupport.h
widget/android/nsWindow.cpp
--- a/accessible/android/SessionAccessibility.cpp
+++ b/accessible/android/SessionAccessibility.cpp
@@ -19,18 +19,25 @@
 
 template<>
 const char nsWindow::NativePtr<mozilla::a11y::SessionAccessibility>::sName[] =
   "SessionAccessibility";
 
 using namespace mozilla::a11y;
 
 void
-SessionAccessibility::SetAttached(bool aAttached)
+SessionAccessibility::SetAttached(bool aAttached,
+                                  already_AddRefed<Runnable> aRunnable)
 {
   if (RefPtr<nsThread> uiThread = GetAndroidUiThread()) {
     uiThread->Dispatch(NS_NewRunnableFunction(
       "SessionAccessibility::Attach",
       [aAttached,
        sa = java::SessionAccessibility::NativeProvider::GlobalRef(
-         mSessionAccessibility)] { sa->SetAttached(aAttached); }));
+           mSessionAccessibility),
+       runnable = RefPtr<Runnable>(aRunnable)] {
+        sa->SetAttached(aAttached);
+        if (runnable) {
+          runnable->Run();
+        }
+      }));
   }
 }
--- a/accessible/android/SessionAccessibility.h
+++ b/accessible/android/SessionAccessibility.h
@@ -20,31 +20,39 @@ public:
 
   SessionAccessibility(
     nsWindow::NativePtr<SessionAccessibility>* aPtr,
     nsWindow* aWindow,
     java::SessionAccessibility::NativeProvider::Param aSessionAccessibility)
     : mWindow(aPtr, aWindow)
     , mSessionAccessibility(aSessionAccessibility)
   {
-    SetAttached(true);
+    SetAttached(true, nullptr);
   }
 
-  void OnDetach() { SetAttached(false); }
+  void OnDetach(already_AddRefed<Runnable> aDisposer)
+  {
+    SetAttached(false, std::move(aDisposer));
+  }
+
+  const java::SessionAccessibility::NativeProvider::Ref& GetJavaAccessibility()
+  {
+    return mSessionAccessibility;
+  }
 
   // Native implementations
   using Base::AttachNative;
   using Base::DisposeNative;
 
   NS_INLINE_DECL_REFCOUNTING(SessionAccessibility)
 
 private:
   ~SessionAccessibility() {}
 
-  void SetAttached(bool aAttached);
+  void SetAttached(bool aAttached, already_AddRefed<Runnable> aRunnable);
 
   nsWindow::WindowPtr<SessionAccessibility> mWindow; // Parent only
   java::SessionAccessibility::NativeProvider::GlobalRef mSessionAccessibility;
 };
 
 } // namespace a11y
 } // namespace mozilla
 
--- 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/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/SessionAccessibility.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/SessionAccessibility.java
@@ -808,21 +808,18 @@ public class SessionAccessibility {
                     AccessibilityEvent.TYPE_WINDOW_STATE_CHANGED, View.NO_ID);
             ((ViewParent) mView).requestSendAccessibilityEvent(mView, event);
         }
     }
 
     /* package */ final class NativeProvider extends JNIObject {
         @WrapForJNI(calledFrom = "ui")
         private void setAttached(final boolean attached) {
-            if (attached) {
-                mAttached = true;
-            } else if (mAttached) {
-                mAttached = false;
-                disposeNative();
-            }
+            mAttached = attached;
         }
 
-        @WrapForJNI(calledFrom = "ui", dispatchTo = "gecko")
-        @Override
-        protected native void disposeNative();
+        @Override // JNIObject
+        protected void disposeNative() {
+            // Disposal happens in native code.
+            throw new UnsupportedOperationException();
+        }
     }
 }
--- 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/nsWindow.cpp
+++ b/widget/android/nsWindow.cpp
@@ -445,17 +445,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
@@ -479,30 +479,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:
@@ -897,24 +892,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;
     }
@@ -1176,30 +1173,33 @@ 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());
     }
 
     if (window.mSessionAccessibility) {
-        window.mSessionAccessibility.Detach();
+        window.mSessionAccessibility.Detach(
+                window.mSessionAccessibility->GetJavaAccessibility());
     }
 }
 
 /* static */ void
 nsWindow::GeckoViewSupport::Open(const jni::Class::LocalRef& aCls,
                                  GeckoSession::Window::Param aWindow,
                                  jni::Object::Param aQueue,
                                  jni::Object::Param aCompositor,
@@ -1294,35 +1294,37 @@ nsWindow::GeckoViewSupport::Transfer(con
                                      jni::Object::Param aQueue,
                                      jni::Object::Param aCompositor,
                                      jni::Object::Param aDispatcher,
                                      jni::Object::Param aSessionAccessibility,
                                      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);
 
     if (window.mSessionAccessibility) {
-        window.mSessionAccessibility.Detach();
+        window.mSessionAccessibility.Detach(
+                window.mSessionAccessibility->GetJavaAccessibility());
     }
     if (aSessionAccessibility) {
         AttachAccessibility(inst, aSessionAccessibility);
     }
 
     if (mIsReady) {
         // We're in a transfer; update init-data and notify JS code.
         window.mAndroidView->mInitData =
@@ -1343,17 +1345,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::GeckoViewSupport::AttachAccessibility(const GeckoSession::Window::LocalRef& inst,
@@ -1361,17 +1364,18 @@ nsWindow::GeckoViewSupport::AttachAccess
 {
     MOZ_ASSERT(!window.mSessionAccessibility);
     java::SessionAccessibility::NativeProvider::LocalRef sessionAccessibility(
       inst.Env());
     sessionAccessibility = java::SessionAccessibility::NativeProvider::Ref::From(
       aSessionAccessibility);
 
     if (window.mSessionAccessibility) {
-        window.mSessionAccessibility.Detach();
+        window.mSessionAccessibility.Detach(
+                window.mSessionAccessibility->GetJavaAccessibility());
     }
 
     window.mSessionAccessibility.Attach(
       sessionAccessibility, &window, sessionAccessibility);
 }
 
 void
 nsWindow::InitNatives()