Bug 1461738 - 2. Don't unregister listeners in GeckoSessionHandler; r?esawin draft
authorJim Chen <nchen@mozilla.com>
Tue, 15 May 2018 13:15:11 -0400
changeset 795336 ce723f61931ce212d2bd83a70efd0c82a86c356c
parent 795335 0c1e680333a94d5359f5f98a8c85a86ce9a3fd75
child 795337 3de81d04f3ac49194e5f74a9fd6271dd89143697
push id109938
push userbmo:nchen@mozilla.com
push dateTue, 15 May 2018 17:16:21 +0000
reviewersesawin
bugs1461738
milestone62.0a1
Bug 1461738 - 2. Don't unregister listeners in GeckoSessionHandler; r?esawin Unregistering listeners when we clear a delegate can lead to some event races between the Gecko and UI thread. It's unclear to me if unregistering events actually has much benefit, so I think we should just not unregister events at all. MozReview-Commit-ID: FS63NfbKgac
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSessionHandler.java
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSessionHandler.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSessionHandler.java
@@ -14,20 +14,20 @@ import org.mozilla.gecko.util.GeckoBundl
 import android.util.Log;
 
 /* package */ abstract class GeckoSessionHandler<Delegate>
     implements BundleEventListener {
 
     private static final String LOGTAG = "GeckoSessionHandler";
     private static final boolean DEBUG = false;
 
-    private Delegate mDelegate;
     private final String mModuleName;
     private final String[] mEvents;
-
+    private Delegate mDelegate;
+    private boolean mRegisteredListeners;
 
     /* package */ GeckoSessionHandler(final String module,
                                       final GeckoSession session,
                                       final String[] events) {
         session.handlersCount++;
 
         mModuleName = module;
         mEvents = events;
@@ -37,50 +37,34 @@ import android.util.Log;
         return mDelegate;
     }
 
     public void setDelegate(final Delegate delegate, final GeckoSession session) {
         if (mDelegate == delegate) {
             return;
         }
 
-        final boolean unsettingOldDelegate = mDelegate != null &&
-                                             delegate == null;
-        final boolean settingNewDelegate = mDelegate == null &&
-                                           delegate != null;
-
-        if (unsettingOldDelegate) {
-            unregister(session);
-        }
-
         mDelegate = delegate;
 
-        if (settingNewDelegate) {
-            register(session);
+        if (!mRegisteredListeners && delegate != null) {
+            session.getEventDispatcher().registerUiThreadListener(this, mEvents);
+            mRegisteredListeners = true;
         }
 
         // If session is not open, we will update module state during session opening.
         if (!session.isOpen()) {
             return;
         }
 
         final GeckoBundle msg = new GeckoBundle(2);
         msg.putString("module", mModuleName);
         msg.putBoolean("enabled", isEnabled());
         session.getEventDispatcher().dispatch("GeckoView:UpdateModuleState", msg);
     }
 
-    private void unregister(final GeckoSession session) {
-        session.getEventDispatcher().unregisterUiThreadListener(this, mEvents);
-    }
-
-    private void register(final GeckoSession session) {
-        session.getEventDispatcher().registerUiThreadListener(this, mEvents);
-    }
-
     public String getName() {
         return mModuleName;
     }
 
     public boolean isEnabled() {
         return mDelegate != null;
     }
 
@@ -88,17 +72,17 @@ import android.util.Log;
     public void handleMessage(final String event, final GeckoBundle message,
                               final EventCallback callback) {
         if (DEBUG) {
             Log.d(LOGTAG, mModuleName + " handleMessage: event = " + event);
         }
 
         if (mDelegate != null) {
             handleMessage(mDelegate, event, message, callback);
-        } else {
+        } else if (callback != null) {
             callback.sendError("No delegate registered");
         }
     }
 
     protected abstract void handleMessage(final Delegate delegate,
                                           final String event,
                                           final GeckoBundle message,
                                           final EventCallback callback);