Add UI-Thread assertions in methods that mutate Views

Summary:
As part of `T54997838` we're auditing where removeView could possibly be called in a background thread, and adding annotations to indicate places where we don't think it's possible.

Changelog: [internal]

Reviewed By: makovkastar

Differential Revision: D18320461

fbshipit-source-id: 84b6b9e293d903f835fc42bc98614efb54158986
This commit is contained in:
Joshua Gross
2019-11-05 13:18:59 -08:00
committed by Facebook Github Bot
parent 3281714312
commit a7f56e73e4
4 changed files with 38 additions and 0 deletions
@@ -10,6 +10,7 @@ package com.facebook.react.uimanager;
import android.view.View;
import android.view.ViewGroup;
import androidx.annotation.Nullable;
import com.facebook.react.bridge.UiThreadUtil;
import java.util.List;
import java.util.WeakHashMap;
@@ -44,6 +45,8 @@ public abstract class ViewGroupManager<T extends ViewGroup>
* @param views the set of views to add
*/
public void addViews(T parent, List<View> views) {
UiThreadUtil.assertOnUiThread();
for (int i = 0, size = views.size(); i < size; i++) {
addView(parent, views.get(i), i);
}
@@ -66,10 +69,14 @@ public abstract class ViewGroupManager<T extends ViewGroup>
}
public void removeViewAt(T parent, int index) {
UiThreadUtil.assertOnUiThread();
parent.removeViewAt(index);
}
public void removeView(T parent, View view) {
UiThreadUtil.assertOnUiThread();
for (int i = 0; i < getChildCount(parent); i++) {
if (getChildAt(parent, i) == view) {
removeViewAt(parent, i);
@@ -79,6 +86,8 @@ public abstract class ViewGroupManager<T extends ViewGroup>
}
public void removeAllViews(T parent) {
UiThreadUtil.assertOnUiThread();
for (int i = getChildCount(parent) - 1; i >= 0; i--) {
removeViewAt(parent, i);
}
@@ -27,6 +27,7 @@ import com.facebook.react.R;
import com.facebook.react.bridge.GuardedRunnable;
import com.facebook.react.bridge.LifecycleEventListener;
import com.facebook.react.bridge.ReactContext;
import com.facebook.react.bridge.UiThreadUtil;
import com.facebook.react.bridge.WritableMap;
import com.facebook.react.bridge.WritableNativeMap;
import com.facebook.react.common.annotations.VisibleForTesting;
@@ -97,6 +98,8 @@ public class ReactModalHostView extends ViewGroup implements LifecycleEventListe
@Override
public void addView(View child, int index) {
UiThreadUtil.assertOnUiThread();
mHostView.addView(child, index);
}
@@ -112,11 +115,15 @@ public class ReactModalHostView extends ViewGroup implements LifecycleEventListe
@Override
public void removeView(View child) {
UiThreadUtil.assertOnUiThread();
mHostView.removeView(child);
}
@Override
public void removeViewAt(int index) {
UiThreadUtil.assertOnUiThread();
View child = getChildAt(index);
mHostView.removeView(child);
}
@@ -140,6 +147,8 @@ public class ReactModalHostView extends ViewGroup implements LifecycleEventListe
}
private void dismiss() {
UiThreadUtil.assertOnUiThread();
if (mDialog != null) {
if (mDialog.isShowing()) {
Activity dialogContext =
@@ -217,6 +226,8 @@ public class ReactModalHostView extends ViewGroup implements LifecycleEventListe
* new Dialog.
*/
protected void showOrUpdate() {
UiThreadUtil.assertOnUiThread();
// If the existing Dialog is currently up, we may need to redraw it or we may be able to update
// the property without having to recreate the dialog
if (mDialog != null) {
@@ -8,6 +8,7 @@
package com.facebook.react.views.view;
import android.view.View;
import com.facebook.react.bridge.UiThreadUtil;
import com.facebook.react.uimanager.ViewGroupManager;
import com.facebook.react.uimanager.annotations.ReactProp;
@@ -21,11 +22,15 @@ public abstract class ReactClippingViewManager<T extends ReactViewGroup>
@ReactProp(
name = com.facebook.react.uimanager.ReactClippingViewGroupHelper.PROP_REMOVE_CLIPPED_SUBVIEWS)
public void setRemoveClippedSubviews(T view, boolean removeClippedSubviews) {
UiThreadUtil.assertOnUiThread();
view.setRemoveClippedSubviews(removeClippedSubviews);
}
@Override
public void addView(T parent, View child, int index) {
UiThreadUtil.assertOnUiThread();
boolean removeClippedSubviews = parent.getRemoveClippedSubviews();
if (removeClippedSubviews) {
parent.addViewWithSubviewClippingEnabled(child, index);
@@ -56,6 +61,8 @@ public abstract class ReactClippingViewManager<T extends ReactViewGroup>
@Override
public void removeViewAt(T parent, int index) {
UiThreadUtil.assertOnUiThread();
boolean removeClippedSubviews = parent.getRemoveClippedSubviews();
if (removeClippedSubviews) {
View child = getChildAt(parent, index);
@@ -70,6 +77,8 @@ public abstract class ReactClippingViewManager<T extends ReactViewGroup>
@Override
public void removeAllViews(T parent) {
UiThreadUtil.assertOnUiThread();
boolean removeClippedSubviews = parent.getRemoveClippedSubviews();
if (removeClippedSubviews) {
parent.removeAllViewsWithSubviewClippingEnabled();
@@ -29,6 +29,7 @@ import androidx.annotation.Nullable;
import com.facebook.common.logging.FLog;
import com.facebook.infer.annotation.Assertions;
import com.facebook.react.bridge.ReactContext;
import com.facebook.react.bridge.UiThreadUtil;
import com.facebook.react.common.annotations.VisibleForTesting;
import com.facebook.react.modules.i18nmanager.I18nUtil;
import com.facebook.react.touch.OnInterceptTouchEventListener;
@@ -358,6 +359,8 @@ public class ReactViewGroup extends ViewGroup
}
private void updateSubviewClipStatus(Rect clippingRect, int idx, int clippedSoFar) {
UiThreadUtil.assertOnUiThread();
View child = Assertions.assertNotNull(mAllChildren)[idx];
sHelperRect.set(child.getLeft(), child.getTop(), child.getRight(), child.getBottom());
boolean intersects =
@@ -462,6 +465,8 @@ public class ReactViewGroup extends ViewGroup
@Override
public void removeView(View view) {
UiThreadUtil.assertOnUiThread();
mDrawingOrderHelper.handleRemoveView(view);
setChildrenDrawingOrderEnabled(mDrawingOrderHelper.shouldEnableCustomDrawingOrder());
@@ -470,6 +475,8 @@ public class ReactViewGroup extends ViewGroup
@Override
public void removeViewAt(int index) {
UiThreadUtil.assertOnUiThread();
mDrawingOrderHelper.handleRemoveView(getChildAt(index));
setChildrenDrawingOrderEnabled(mDrawingOrderHelper.shouldEnableCustomDrawingOrder());
@@ -543,6 +550,8 @@ public class ReactViewGroup extends ViewGroup
}
/*package*/ void removeViewWithSubviewClippingEnabled(View view) {
UiThreadUtil.assertOnUiThread();
Assertions.assertCondition(mRemoveClippedSubviews);
Assertions.assertNotNull(mClippingRect);
Assertions.assertNotNull(mAllChildren);