Browse Source

Ensure ReactView isn't created after it's destroyed (#2503)

* Ensure ReactView isn't created after it's destroyed

Fixes #2490

* Rebase fixes and renames
Guy Carmeli 6 years ago
parent
commit
8bf80cf82f
No account linked to committer's email address

+ 1
- 1
lib/android/app/src/main/java/com/reactnativenavigation/presentation/NavigationOptionsListener.java View File

@@ -4,5 +4,5 @@ package com.reactnativenavigation.presentation;
4 4
 import com.reactnativenavigation.parse.Options;
5 5
 
6 6
 public interface NavigationOptionsListener {
7
-	void mergeNavigationOptions(Options options);
7
+	void mergeOptions(Options options);
8 8
 }

+ 1
- 1
lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/BottomTabsController.java View File

@@ -98,7 +98,7 @@ public class BottomTabsController extends ParentController
98 98
 	}
99 99
 
100 100
 	@Override
101
-	public void mergeNavigationOptions(Options options) {
101
+	public void mergeOptions(Options options) {
102 102
 		if (options.bottomTabsOptions != null) {
103 103
 			if (options.bottomTabsOptions.currentTabIndex != NO_INT_VALUE) {
104 104
 				selectTabAtIndex(options.bottomTabsOptions.currentTabIndex);

+ 1
- 1
lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/ComponentViewController.java View File

@@ -86,7 +86,7 @@ public class ComponentViewController extends ViewController implements Navigatio
86 86
     }
87 87
 
88 88
     @Override
89
-    public void mergeNavigationOptions(Options options) {
89
+    public void mergeOptions(Options options) {
90 90
         this.options.mergeWith(options);
91 91
         component.applyOptions(this.options);
92 92
     }

+ 104
- 0
lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/ContainerViewController.java View File

@@ -0,0 +1,104 @@
1
+package com.reactnativenavigation.viewcontrollers;
2
+
3
+import android.app.Activity;
4
+import android.support.annotation.NonNull;
5
+import android.support.annotation.RestrictTo;
6
+import android.view.View;
7
+
8
+import com.reactnativenavigation.parse.Options;
9
+import com.reactnativenavigation.presentation.NavigationOptionsListener;
10
+import com.reactnativenavigation.views.ReactComponent;
11
+import com.reactnativenavigation.views.TopBar;
12
+
13
+public class ContainerViewController extends ViewController implements NavigationOptionsListener {
14
+
15
+    public interface ReactViewCreator {
16
+
17
+        IReactView create(Activity activity, String containerId, String containerName);
18
+    }
19
+
20
+    public interface IReactView {
21
+
22
+        boolean isReady();
23
+
24
+        View asView();
25
+
26
+        void destroy();
27
+
28
+        void sendComponentStart();
29
+
30
+        void sendComponentStop();
31
+
32
+        void sendOnNavigationButtonPressed(String buttonId);
33
+    }
34
+
35
+    private final String componentName;
36
+
37
+    private final ReactViewCreator viewCreator;
38
+    private Options options;
39
+    private ReactComponent component;
40
+
41
+    public ContainerViewController(final Activity activity,
42
+                                   final String id,
43
+                                   final String componentName,
44
+                                   final ReactViewCreator viewCreator,
45
+                                   final Options initialOptions) {
46
+        super(activity, id);
47
+        this.componentName = componentName;
48
+        this.viewCreator = viewCreator;
49
+        this.options = initialOptions;
50
+    }
51
+
52
+    @RestrictTo(RestrictTo.Scope.TESTS)
53
+    TopBar getTopBar() {
54
+        return component.getTopBar();
55
+    }
56
+
57
+    @RestrictTo(RestrictTo.Scope.TESTS)
58
+    ReactComponent getComponent() {
59
+        return component;
60
+    }
61
+
62
+    @Override
63
+    public void destroy() {
64
+        super.destroy();
65
+        if (component != null) component.destroy();
66
+        component = null;
67
+    }
68
+
69
+    @Override
70
+    public void onViewAppeared() {
71
+        super.onViewAppeared();
72
+        ensureViewIsCreated();
73
+        component.applyOptions(options);
74
+        component.sendComponentStart();
75
+    }
76
+
77
+    @Override
78
+    public void onViewDisappear() {
79
+        super.onViewDisappear();
80
+        component.sendComponentStop();
81
+    }
82
+
83
+    @Override
84
+    protected boolean isViewShown() {
85
+        return super.isViewShown() && component.isReady();
86
+    }
87
+
88
+    @NonNull
89
+    @Override
90
+    protected View createView() {
91
+        component = (ReactComponent) viewCreator.create(getActivity(), getId(), componentName);
92
+        return component.asView();
93
+    }
94
+
95
+    @Override
96
+    public void mergeOptions(Options options) {
97
+        this.options.mergeWith(options);
98
+        component.applyOptions(this.options);
99
+    }
100
+
101
+    Options getOptions() {
102
+        return options;
103
+    }
104
+}

+ 2
- 2
lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/Navigator.java View File

@@ -81,10 +81,10 @@ public class Navigator extends ParentController {
81 81
 	public void setOptions(final String componentId, Options options) {
82 82
 		ViewController target = findControllerById(componentId);
83 83
 		if (target instanceof NavigationOptionsListener) {
84
-			((NavigationOptionsListener) target).mergeNavigationOptions(options);
84
+			((NavigationOptionsListener) target).mergeOptions(options);
85 85
 		}
86 86
 		if (root instanceof NavigationOptionsListener) {
87
-			((NavigationOptionsListener) root).mergeNavigationOptions(options);
87
+			((NavigationOptionsListener) root).mergeOptions(options);
88 88
 		}
89 89
 	}
90 90
 

+ 6
- 2
lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/ViewController.java View File

@@ -23,6 +23,8 @@ public abstract class ViewController implements ViewTreeObserver.OnGlobalLayoutL
23 23
 	private ParentController parentController;
24 24
 	private boolean isShown = false;
25 25
 
26
+    private boolean isDestroyed;
27
+
26 28
 	public ViewController(Activity activity, String id) {
27 29
 		this.activity = activity;
28 30
 		this.id = id;
@@ -77,7 +79,8 @@ public abstract class ViewController implements ViewTreeObserver.OnGlobalLayoutL
77 79
 	@NonNull
78 80
 	public View getView() {
79 81
 		if (view == null) {
80
-			view = createView();
82
+		    if (isDestroyed) throw new RuntimeException("Tried to create view after it has already been destroyed");
83
+            view = createView();
81 84
 			view.setId(CompatUtils.generateViewId());
82 85
 			view.getViewTreeObserver().addOnGlobalLayoutListener(this);
83 86
 		}
@@ -120,6 +123,7 @@ public abstract class ViewController implements ViewTreeObserver.OnGlobalLayoutL
120 123
 				((ViewManager) view.getParent()).removeView(view);
121 124
 			}
122 125
 			view = null;
126
+            isDestroyed = true;
123 127
 		}
124 128
 	}
125 129
 
@@ -135,6 +139,6 @@ public abstract class ViewController implements ViewTreeObserver.OnGlobalLayoutL
135 139
 	}
136 140
 
137 141
 	protected boolean isViewShown() {
138
-		return getView().isShown();
142
+        return !isDestroyed && getView().isShown();
139 143
 	}
140 144
 }

+ 1
- 1
lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/toptabs/TopTabController.java View File

@@ -66,7 +66,7 @@ public class TopTabController extends ViewController implements NavigationOption
66 66
     }
67 67
 
68 68
     @Override
69
-    public void mergeNavigationOptions(Options options) {
69
+    public void mergeOptions(Options options) {
70 70
         this.options.mergeWith(options);
71 71
         applyOptions(this.options);
72 72
     }

+ 1
- 1
lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/toptabs/TopTabsController.java View File

@@ -62,7 +62,7 @@ public class TopTabsController extends ParentController implements NavigationOpt
62 62
     }
63 63
 
64 64
     @Override
65
-    public void mergeNavigationOptions(Options options) {
65
+    public void mergeOptions(Options options) {
66 66
 
67 67
     }
68 68
 

+ 7
- 7
lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/OptionsApplyingTest.java View File

@@ -61,7 +61,7 @@ public class OptionsApplyingTest extends BaseTest {
61 61
         assertThat(uut.getOptions().topBarOptions.title).isEmpty();
62 62
         Options options = new Options();
63 63
         options.topBarOptions.title = "new title";
64
-        uut.mergeNavigationOptions(options);
64
+        uut.mergeOptions(options);
65 65
         assertThat(uut.getOptions().topBarOptions.title).isEqualTo("new title");
66 66
         assertThat(uut.getOptions()).isSameAs(initialNavigationOptions);
67 67
     }
@@ -73,7 +73,7 @@ public class OptionsApplyingTest extends BaseTest {
73 73
 
74 74
         Options opts = new Options();
75 75
         opts.topBarOptions.title = "the new title";
76
-        uut.mergeNavigationOptions(opts);
76
+        uut.mergeOptions(opts);
77 77
 
78 78
         assertThat(uut.getTopBar().getTitle()).isEqualTo("the new title");
79 79
     }
@@ -86,7 +86,7 @@ public class OptionsApplyingTest extends BaseTest {
86 86
 
87 87
         Options opts = new Options();
88 88
         opts.topBarOptions.backgroundColor = Color.RED;
89
-        uut.mergeNavigationOptions(opts);
89
+        uut.mergeOptions(opts);
90 90
 
91 91
         assertThat(((ColorDrawable) uut.getTopBar().getTitleBar().getBackground()).getColor()).isEqualTo(Color.RED);
92 92
     }
@@ -101,7 +101,7 @@ public class OptionsApplyingTest extends BaseTest {
101 101
         Options opts = new Options();
102 102
         opts.topBarOptions.title = "the title";
103 103
         opts.topBarOptions.textColor = Color.RED;
104
-        uut.mergeNavigationOptions(opts);
104
+        uut.mergeOptions(opts);
105 105
 
106 106
         assertThat(uut.getTopBar().getTitleTextView()).isNotEqualTo(null);
107 107
         assertThat(uut.getTopBar().getTitleTextView().getCurrentTextColor()).isEqualTo(Color.RED);
@@ -117,7 +117,7 @@ public class OptionsApplyingTest extends BaseTest {
117 117
         Options opts = new Options();
118 118
         opts.topBarOptions.title = "the title";
119 119
         opts.topBarOptions.textFontSize = 18;
120
-        uut.mergeNavigationOptions(opts);
120
+        uut.mergeOptions(opts);
121 121
 
122 122
         assertThat(uut.getTopBar().getTitleTextView()).isNotEqualTo(null);
123 123
         assertThat(uut.getTopBar().getTitleTextView().getTextSize()).isEqualTo(18);
@@ -132,7 +132,7 @@ public class OptionsApplyingTest extends BaseTest {
132 132
 
133 133
         Options opts = new Options();
134 134
         opts.topBarOptions.hidden = Options.BooleanOptions.True;
135
-        uut.mergeNavigationOptions(opts);
135
+        uut.mergeOptions(opts);
136 136
 
137 137
         assertThat(uut.getTopBar().getVisibility()).isEqualTo(View.GONE);
138 138
     }
@@ -148,7 +148,7 @@ public class OptionsApplyingTest extends BaseTest {
148 148
 
149 149
         Options opts = new Options();
150 150
         opts.topBarOptions.drawBehind = Options.BooleanOptions.True;
151
-        uut.mergeNavigationOptions(opts);
151
+        uut.mergeOptions(opts);
152 152
 
153 153
         uutLayoutParams = (RelativeLayout.LayoutParams) ((ViewGroup) uut.getComponent().asView()).getChildAt(1).getLayoutParams();
154 154
         assertThat(uutLayoutParams.getRule(BELOW)).isEqualTo(0);

+ 7
- 4
lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/StackControllerTest.java View File

@@ -1,6 +1,7 @@
1 1
 package com.reactnativenavigation.viewcontrollers;
2 2
 
3 3
 import android.app.*;
4
+import android.view.View;
4 5
 
5 6
 import com.reactnativenavigation.*;
6 7
 import com.reactnativenavigation.mocks.*;
@@ -142,11 +143,13 @@ public class StackControllerTest extends BaseTest {
142 143
     public void popReplacesViewWithPrevious() throws Exception {
143 144
         uut.push(child1);
144 145
         uut.push(child2);
145
-        assertIsChildById(uut.getView(), child2.getView());
146
-        assertNotChildOf(uut.getView(), child1.getView());
146
+        final View child2View = child2.getView();
147
+        final View child1View = child1.getView();
148
+        assertIsChildById(uut.getView(), child2View);
149
+        assertNotChildOf(uut.getView(), child1View);
147 150
         uut.pop();
148
-        assertNotChildOf(uut.getView(), child2.getView());
149
-        assertIsChildById(uut.getView(), child1.getView());
151
+        assertNotChildOf(uut.getView(), child2View);
152
+        assertIsChildById(uut.getView(), child1View);
150 153
     }
151 154
 
152 155
     @Test

+ 1
- 1
lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/TopTabControllerTest.java View File

@@ -44,7 +44,7 @@ public class TopTabControllerTest extends BaseTest {
44 44
     public void styleIsAppliedOnParentControllerWhenOptionsAreSetDynamically() throws Exception {
45 45
         uut.ensureViewIsCreated();
46 46
         uut.onViewAppeared();
47
-        uut.mergeNavigationOptions(new Options());
47
+        uut.mergeOptions(new Options());
48 48
         verify(parentController, times(2)).applyOptions(initialOptions);
49 49
     }
50 50
 

+ 15
- 11
lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/ViewControllerTest.java View File

@@ -1,20 +1,24 @@
1 1
 package com.reactnativenavigation.viewcontrollers;
2 2
 
3
-import android.app.*;
4
-import android.view.*;
5
-import android.widget.*;
3
+import android.app.Activity;
4
+import android.view.View;
5
+import android.view.ViewParent;
6
+import android.widget.LinearLayout;
6 7
 
7
-import com.reactnativenavigation.*;
8
-import com.reactnativenavigation.mocks.*;
8
+import com.reactnativenavigation.BaseTest;
9
+import com.reactnativenavigation.mocks.SimpleViewController;
9 10
 
10
-import org.assertj.android.api.*;
11
-import org.junit.*;
12
-import org.robolectric.*;
11
+import org.assertj.android.api.Assertions;
12
+import org.junit.Test;
13
+import org.robolectric.Shadows;
13 14
 
14
-import java.lang.reflect.*;
15
+import java.lang.reflect.Field;
15 16
 
16
-import static org.assertj.core.api.Java6Assertions.*;
17
-import static org.mockito.Mockito.*;
17
+import static org.assertj.core.api.Java6Assertions.assertThat;
18
+import static org.mockito.Mockito.mock;
19
+import static org.mockito.Mockito.spy;
20
+import static org.mockito.Mockito.times;
21
+import static org.mockito.Mockito.verify;
18 22
 
19 23
 public class ViewControllerTest extends BaseTest {
20 24