Bug 1442250 - 3. Track EventDispatcher attach/detach; r?esawin draft
authorJim Chen <nchen@mozilla.com>
Fri, 09 Mar 2018 12:34:37 -0500
changeset 765369 4c4a89e5c212dc8848a58ee80cf077dc7952f886
parent 765368 f96da9d5ccf565852fa93223f3c85c27411a2466
child 765370 0d67b4bd6047c7fc31258f421c09c233a9ddd42c
push id102052
push userbmo:nchen@mozilla.com
push dateFri, 09 Mar 2018 17:35:14 +0000
reviewersesawin
bugs1442250
milestone60.0a1
Bug 1442250 - 3. Track EventDispatcher attach/detach; r?esawin In an "attach > detach > attach" sequence, detach will post a call to disposeNative, so the sequence looks like "attach > detach > attach > disposeNative". In that case, disposeNative will cancel attach. This patch makes us ignore disposeNative in that case, so the second attach works as intended. MozReview-Commit-ID: Kr55PZcsPg1
widget/android/EventDispatcher.cpp
widget/android/EventDispatcher.h
--- a/widget/android/EventDispatcher.cpp
+++ b/widget/android/EventDispatcher.cpp
@@ -918,42 +918,60 @@ EventDispatcher::Attach(java::EventDispa
     MOZ_ASSERT(aDispatcher);
 
     if (mDispatcher) {
         if (mDispatcher == aDispatcher) {
             // Only need to update the window.
             mDOMWindow = aDOMWindow;
             return;
         }
+        mAttachCount--;
         mDispatcher->SetAttachedToGecko(java::EventDispatcher::REATTACHING);
     }
 
     java::EventDispatcher::LocalRef dispatcher(aDispatcher);
 
     NativesBase::AttachNative(dispatcher, this);
     mDispatcher = dispatcher;
     mDOMWindow = aDOMWindow;
 
     dispatcher->SetAttachedToGecko(java::EventDispatcher::ATTACHED);
+    mAttachCount++;
 }
 
 void
 EventDispatcher::Detach()
 {
     MOZ_ASSERT(NS_IsMainThread());
     MOZ_ASSERT(mDispatcher);
 
     // SetAttachedToGecko will call disposeNative for us. disposeNative will be
     // called later on the Gecko thread to make sure all pending
     // dispatchToGecko calls have completed.
+    mAttachCount--;
     mDispatcher->SetAttachedToGecko(java::EventDispatcher::DETACHED);
     mDispatcher = nullptr;
     mDOMWindow = nullptr;
 }
 
+void
+EventDispatcher::DisposeNative(const java::EventDispatcher::LocalRef& aInstance)
+{
+    JNIEnv* const env = jni::GetGeckoThreadEnv();
+    const auto natives = reinterpret_cast<RefPtr<EventDispatcher>*>(
+            jni::GetNativeHandle(env, aInstance.Get()));
+    MOZ_CATCH_JNI_EXCEPTION(env);
+
+    if (!(*natives)->mAttachCount) {
+        // Only actually dispose if we haven't attached again between calling
+        // Detach() and calling DisposeNative().
+        NativesBase::DisposeNative(aInstance);
+    }
+}
+
 bool
 EventDispatcher::HasGeckoListener(jni::String::Param aEvent)
 {
     // Can be called from any thread.
     MutexAutoLock lock(mLock);
     return !!mListenersMap.Get(aEvent->ToString());
 }
 
--- a/widget/android/EventDispatcher.h
+++ b/widget/android/EventDispatcher.h
@@ -40,30 +40,31 @@ public:
     void Attach(java::EventDispatcher::Param aDispatcher,
                 nsPIDOMWindowOuter* aDOMWindow);
     void Detach();
 
     nsresult Dispatch(const char16_t* aEvent,
                       java::GeckoBundle::Param aData = nullptr,
                       nsIAndroidEventCallback* aCallback = nullptr);
 
-    using NativesBase::DisposeNative;
-
     bool HasGeckoListener(jni::String::Param aEvent);
     void DispatchToGecko(jni::String::Param aEvent,
                          jni::Object::Param aData,
                          jni::Object::Param aCallback);
 
     static nsresult UnboxBundle(JSContext* aCx,
                                 jni::Object::Param aData,
                                 JS::MutableHandleValue aOut);
 
+    static void DisposeNative(const java::EventDispatcher::LocalRef& aInstance);
+
 private:
     java::EventDispatcher::GlobalRef mDispatcher;
     nsCOMPtr<nsPIDOMWindowOuter> mDOMWindow;
+    int32_t mAttachCount{0};
 
     virtual ~EventDispatcher() {}
 
     struct ListenersList {
         nsCOMArray<nsIAndroidEventListener> listeners{/* count */ 1};
         // 0 if the list can be modified
         uint32_t lockCount{0};
         // true if this list has a listener that is being unregistered