Bug 1459501 - Fix race in EventDispatcher queuing; r?esawin draft
authorJim Chen <nchen@mozilla.com>
Tue, 08 May 2018 17:38:11 -0400
changeset 792813 2a0b65c9f8af9e466fa956774c889aea6947360e
parent 792812 ac30cf0e3fee22c76bfa020e54fb1b5d3b38d5ca
child 792814 137870788ca3e5440597057c9755c9b5405dc56e
push id109188
push userbmo:nchen@mozilla.com
push dateTue, 08 May 2018 21:39:02 +0000
reviewersesawin
bugs1459501
milestone62.0a1
Bug 1459501 - Fix race in EventDispatcher queuing; r?esawin There's a race in EventDispatcher, where the ready state can change during a dispatch. In that case, we can end up neither dispatching an event nor queuing it, which effectively drops the event. MozReview-Commit-ID: GvjSUzjBrsT
mobile/android/geckoview/src/main/java/org/mozilla/gecko/EventDispatcher.java
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/EventDispatcher.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/EventDispatcher.java
@@ -231,33 +231,39 @@ public final class EventDispatcher exten
      * Dispatch event to any registered Bundle listeners (non-Gecko thread listeners).
      *
      * @param type Event type
      * @param message Bundle message
      * @param callback Optional object for callbacks from events.
      */
     public void dispatch(final String type, final GeckoBundle message,
                          final EventCallback callback) {
+        final boolean isGeckoReady;
         synchronized (this) {
-            if (isReadyForDispatchingToGecko() && mAttachedToGecko &&
-                hasGeckoListener(type)) {
+            isGeckoReady = isReadyForDispatchingToGecko();
+            if (isGeckoReady && mAttachedToGecko && hasGeckoListener(type)) {
                 dispatchToGecko(type, message, JavaCallbackDelegate.wrap(callback));
                 return;
             }
         }
 
-        if (!dispatchToThreads(type, message, /* callback */ callback)) {
-            Log.w(LOGTAG, "No listener for " + type);
-        }
+        dispatchToThreads(type, message, callback, isGeckoReady);
     }
 
     @WrapForJNI(calledFrom = "gecko")
     private boolean dispatchToThreads(final String type,
                                       final GeckoBundle message,
                                       final EventCallback callback) {
+        return dispatchToThreads(type, message, callback, /* isGeckoReady */ true);
+    }
+
+    private boolean dispatchToThreads(final String type,
+                                      final GeckoBundle message,
+                                      final EventCallback callback,
+                                      final boolean isGeckoReady) {
         final List<BundleEventListener> geckoListeners;
         synchronized (mGeckoThreadListeners) {
             geckoListeners = mGeckoThreadListeners.get(type);
         }
         if (geckoListeners != null && !geckoListeners.isEmpty()) {
             final boolean onGeckoThread = ThreadUtils.isOnGeckoThread();
             final EventCallback wrappedCallback = JavaCallbackDelegate.wrap(callback);
 
@@ -284,30 +290,31 @@ public final class EventDispatcher exten
             return true;
         }
 
         if (dispatchToThread(type, message, callback,
                              mBackgroundThreadListeners, ThreadUtils.getBackgroundHandler())) {
             return true;
         }
 
-        if (!isReadyForDispatchingToGecko()) {
+        if (!isGeckoReady) {
             // Usually, we discard an event if there is no listeners for it by
             // the time of the dispatch. However, if Gecko(View) is not ready and
             // there is no listener for this event that's possibly headed to
             // Gecko, we make a special exception to queue this event until
             // Gecko(View) is ready. This way, Gecko can first register its
             // listeners, and accept the event when it is ready.
             mNativeQueue.queueUntilReady(this, "dispatchToGecko",
                 String.class, type,
                 GeckoBundle.class, message,
                 EventCallback.class, JavaCallbackDelegate.wrap(callback));
             return true;
         }
 
+        Log.w(LOGTAG, "No listener for " + type);
         return false;
     }
 
     private boolean dispatchToThread(final String type,
                                      final GeckoBundle message,
                                      final EventCallback callback,
                                      final Map<String, List<BundleEventListener>> listenersMap,
                                      final Handler thread) {