From 8b895ca7104d991dc1cce40fc5a3b2b7ca077b68 Mon Sep 17 00:00:00 2001 From: Christoph Nakazawa Date: Fri, 29 Mar 2019 14:48:58 -0700 Subject: [PATCH] Back out "[react-native][PR][Microsoft] This change fixes currently broken ReactContext listeners mechanism." Summary: Original commit changeset: d506e5035a7f Reviewed By: JoshuaGross Differential Revision: D14689559 fbshipit-source-id: 9a8c8be0d2b7b9dd9be1ec038d7c3b356a5e3adf --- .../facebook/react/bridge/ReactContext.java | 92 ++++++++----------- .../react/bridge/SynchronizedWeakHashSet.java | 86 ----------------- 2 files changed, 38 insertions(+), 140 deletions(-) delete mode 100644 ReactAndroid/src/main/java/com/facebook/react/bridge/SynchronizedWeakHashSet.java diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java index 1a4c8949c..d07d4ae28 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java @@ -18,6 +18,7 @@ import com.facebook.react.bridge.queue.MessageQueueThread; import com.facebook.react.bridge.queue.ReactQueueConfiguration; import com.facebook.react.common.LifecycleState; import java.lang.ref.WeakReference; +import java.util.concurrent.CopyOnWriteArraySet; import javax.annotation.Nullable; /** @@ -31,35 +32,10 @@ public class ReactContext extends ContextWrapper { "ReactContext#getJSModule should only happen once initialize() has been called on your " + "native module."; - private final SynchronizedWeakHashSet mLifecycleEventListeners = - new SynchronizedWeakHashSet<>(); - private final SynchronizedWeakHashSet mActivityEventListeners = - new SynchronizedWeakHashSet<>(); - - - private final GuardedIteration mResumeIteration = - new GuardedIteration() { - @Override - public void onIterate(LifecycleEventListener listener) { - listener.onHostResume(); - } - }; - - private final GuardedIteration mPauseIteration = - new GuardedIteration() { - @Override - public void onIterate(LifecycleEventListener listener) { - listener.onHostPause(); - } - }; - - private final GuardedIteration mDestroyIteration = - new GuardedIteration() { - @Override - public void onIterate(LifecycleEventListener listener) { - listener.onHostDestroy(); - } - }; + private final CopyOnWriteArraySet mLifecycleEventListeners = + new CopyOnWriteArraySet<>(); + private final CopyOnWriteArraySet mActivityEventListeners = + new CopyOnWriteArraySet<>(); private LifecycleState mLifecycleState = LifecycleState.BEFORE_CREATE; @@ -212,19 +188,26 @@ public class ReactContext extends ContextWrapper { mLifecycleState = LifecycleState.RESUMED; mCurrentActivity = new WeakReference(activity); ReactMarker.logMarker(ReactMarkerConstants.ON_HOST_RESUME_START); - mLifecycleEventListeners.iterate(mResumeIteration); + for (LifecycleEventListener listener : mLifecycleEventListeners) { + try { + listener.onHostResume(); + } catch (RuntimeException e) { + handleException(e); + } + } ReactMarker.logMarker(ReactMarkerConstants.ON_HOST_RESUME_END); } - public void onNewIntent(@Nullable Activity activity, final Intent intent) { + public void onNewIntent(@Nullable Activity activity, Intent intent) { UiThreadUtil.assertOnUiThread(); mCurrentActivity = new WeakReference(activity); - mActivityEventListeners.iterate(new GuardedIteration() { - @Override - public void onIterate(ActivityEventListener listener) { + for (ActivityEventListener listener : mActivityEventListeners) { + try { listener.onNewIntent(intent); + } catch (RuntimeException e) { + handleException(e); } - }); + } } /** @@ -233,7 +216,13 @@ public class ReactContext extends ContextWrapper { public void onHostPause() { mLifecycleState = LifecycleState.BEFORE_RESUME; ReactMarker.logMarker(ReactMarkerConstants.ON_HOST_PAUSE_START); - mLifecycleEventListeners.iterate(mPauseIteration); + for (LifecycleEventListener listener : mLifecycleEventListeners) { + try { + listener.onHostPause(); + } catch (RuntimeException e) { + handleException(e); + } + } ReactMarker.logMarker(ReactMarkerConstants.ON_HOST_PAUSE_END); } @@ -243,7 +232,13 @@ public class ReactContext extends ContextWrapper { public void onHostDestroy() { UiThreadUtil.assertOnUiThread(); mLifecycleState = LifecycleState.BEFORE_CREATE; - mLifecycleEventListeners.iterate(mDestroyIteration); + for (LifecycleEventListener listener : mLifecycleEventListeners) { + try { + listener.onHostDestroy(); + } catch (RuntimeException e) { + handleException(e); + } + } mCurrentActivity = null; } @@ -261,13 +256,14 @@ public class ReactContext extends ContextWrapper { /** * Should be called by the hosting Fragment in {@link Fragment#onActivityResult} */ - public void onActivityResult(final Activity activity, final int requestCode, final int resultCode, final Intent data) { - mActivityEventListeners.iterate(new GuardedIteration() { - @Override - public void onIterate(ActivityEventListener listener) { + public void onActivityResult(Activity activity, int requestCode, int resultCode, Intent data) { + for (ActivityEventListener listener : mActivityEventListeners) { + try { listener.onActivityResult(activity, requestCode, resultCode, data); + } catch (RuntimeException e) { + handleException(e); } - }); + } } public void assertOnUiQueueThread() { @@ -363,16 +359,4 @@ public class ReactContext extends ContextWrapper { return mCatalystInstance.getJavaScriptContextHolder(); } - private abstract class GuardedIteration implements SynchronizedWeakHashSet.Iteration { - @Override - public void iterate(T listener) { - try { - onIterate(listener); - } catch (RuntimeException e) { - handleException(e); - } - } - - public abstract void onIterate(T listener); - } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/SynchronizedWeakHashSet.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/SynchronizedWeakHashSet.java deleted file mode 100644 index b7ce40b0c..000000000 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/SynchronizedWeakHashSet.java +++ /dev/null @@ -1,86 +0,0 @@ -// Copyright (c) Facebook, Inc. and its affiliates. - -// This source code is licensed under the MIT license found in the -// LICENSE file in the root directory of this source tree. -package com.facebook.react.bridge; - -import android.util.Pair; - -import java.util.ArrayDeque; -import java.util.Queue; -import java.util.WeakHashMap; - -/** - * Thread-safe Set based on the WeakHashMap. - * - * Doesn't implement the `iterator` method because it's tricky to support modifications - * to the collection while somebody is using an `Iterator` to iterate over it. - * - * Instead, it provides an `iterate` method for traversing the collection. Any add/remove operations - * that occur during iteration are postponed until the iteration has completed. - */ -public class SynchronizedWeakHashSet { - private WeakHashMap mMap = new WeakHashMap<>(); - private Queue> mPendingOperations = new ArrayDeque<>(); - private boolean mIterating; - - public boolean contains(T item) { - synchronized (mMap) { - return mMap.containsKey(item); - } - } - - public void add(T item) { - synchronized (mMap) { - if (mIterating) { - mPendingOperations.add(new Pair<>(item, Command.ADD)); - } else { - mMap.put(item, null); - } - } - } - - public void remove(T item) { - synchronized (mMap) { - if (mIterating) { - mPendingOperations.add(new Pair<>(item, Command.REMOVE)); - } else { - mMap.remove(item); - } - } - } - - public void iterate(Iteration iterated) { - synchronized (mMap) { - // Protection from modification during iteration on the same thread - mIterating = true; - for (T listener: mMap.keySet()) { - iterated.iterate(listener); - } - mIterating = false; - - while (!mPendingOperations.isEmpty()) { - Pair pair = mPendingOperations.poll(); - switch (pair.second) { - case ADD: - mMap.put(pair.first, null); - break; - case REMOVE: - mMap.remove(pair.first); - break; - default: - throw new AssertionException("Unsupported command" + pair.second); - } - } - } - } - - public interface Iteration { - void iterate(T item); - } - - private enum Command { - ADD, - REMOVE - } -}