Skip to content

Commit

Permalink
Add UI-Thread assertions in methods that mutate Views
Browse files Browse the repository at this point in the history
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
  • Loading branch information
JoshuaGross authored and facebook-github-bot committed Nov 5, 2019
1 parent 3281714 commit a7f56e7
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
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;

Expand Down Expand Up @@ -44,6 +45,8 @@ public void addView(T parent, View child, int index) {
* @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);
}
Expand All @@ -66,10 +69,14 @@ public View getChildAt(T parent, int index) {
}

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);
Expand All @@ -79,6 +86,8 @@ public void removeView(T parent, View view) {
}

public void removeAllViews(T parent) {
UiThreadUtil.assertOnUiThread();

for (int i = getChildCount(parent) - 1; i >= 0; i--) {
removeViewAt(parent, i);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
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;
Expand Down Expand Up @@ -97,6 +98,8 @@ protected void onLayout(boolean changed, int l, int t, int r, int b) {

@Override
public void addView(View child, int index) {
UiThreadUtil.assertOnUiThread();

mHostView.addView(child, index);
}

Expand All @@ -112,11 +115,15 @@ public View getChildAt(int index) {

@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);
}
Expand All @@ -140,6 +147,8 @@ public void onDropInstance() {
}

private void dismiss() {
UiThreadUtil.assertOnUiThread();

if (mDialog != null) {
if (mDialog.isShowing()) {
Activity dialogContext =
Expand Down Expand Up @@ -217,6 +226,8 @@ public void onHostDestroy() {
* 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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
Expand Down Expand Up @@ -56,6 +61,8 @@ public View getChildAt(T parent, int index) {

@Override
public void removeViewAt(T parent, int index) {
UiThreadUtil.assertOnUiThread();

boolean removeClippedSubviews = parent.getRemoveClippedSubviews();
if (removeClippedSubviews) {
View child = getChildAt(parent, index);
Expand All @@ -70,6 +77,8 @@ public void removeViewAt(T parent, int index) {

@Override
public void removeAllViews(T parent) {
UiThreadUtil.assertOnUiThread();

boolean removeClippedSubviews = parent.getRemoveClippedSubviews();
if (removeClippedSubviews) {
parent.removeAllViewsWithSubviewClippingEnabled();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
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;
Expand Down Expand Up @@ -358,6 +359,8 @@ private void updateClippingToRect(Rect clippingRect) {
}

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 =
Expand Down Expand Up @@ -462,6 +465,8 @@ public void addView(View child, int index, ViewGroup.LayoutParams params) {

@Override
public void removeView(View view) {
UiThreadUtil.assertOnUiThread();

mDrawingOrderHelper.handleRemoveView(view);
setChildrenDrawingOrderEnabled(mDrawingOrderHelper.shouldEnableCustomDrawingOrder());

Expand All @@ -470,6 +475,8 @@ public void removeView(View view) {

@Override
public void removeViewAt(int index) {
UiThreadUtil.assertOnUiThread();

mDrawingOrderHelper.handleRemoveView(getChildAt(index));
setChildrenDrawingOrderEnabled(mDrawingOrderHelper.shouldEnableCustomDrawingOrder());

Expand Down Expand Up @@ -543,6 +550,8 @@ protected void dispatchSetPressed(boolean pressed) {
}

/*package*/ void removeViewWithSubviewClippingEnabled(View view) {
UiThreadUtil.assertOnUiThread();

Assertions.assertCondition(mRemoveClippedSubviews);
Assertions.assertNotNull(mClippingRect);
Assertions.assertNotNull(mAllChildren);
Expand Down

0 comments on commit a7f56e7

Please sign in to comment.