Skip to content

Commit 2a4338d

Browse files
Dark Knightfacebook-github-bot
authored andcommitted
Revert D51346658: Multisect successfully blamed "D51346658: [RN][Android] Handle all incoming Inspector messages on main thread, downgrade some errors to logs" for otest failure
Summary: This diff is reverting D51346658 D51346658: [RN][Android] Handle all incoming Inspector messages on main thread, downgrade some errors to logs by motiz88 has been identified to be causing the following test failure: Tests affected: - [xplat/endtoend/jest-e2e/apps/facebook_xplat/ReactNativeTTRCTester/__tests__/ReactNativeTTRCTester-storeOrNetworkWithoutCachedContent-android-e2e.js](https://www.internalfb.com/intern/test/281475019301167/) Here's the Multisect link: https://www.internalfb.com/multisect/3539088 Here are the tasks that are relevant to this breakage: We're generating a revert to back out the changes in this diff, please note the backout may land if someone accepts it. If you believe this diff has been generated in error you may Commandeer and Abandon it. bypass-github-export-checks Changelog: [Internal] Reviewed By: motiz88 Differential Revision: D51512872 fbshipit-source-id: 8bc8e12b651f91a6f74243a0a85fca7fd1953bdb
1 parent 08f89eb commit 2a4338d

File tree

3 files changed

+54
-64
lines changed

3 files changed

+54
-64
lines changed

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -210,10 +210,16 @@ public void openInspectorConnection() {
210210
FLog.w(ReactConstants.TAG, "Inspector connection already open, nooping.");
211211
return;
212212
}
213-
mInspectorPackagerConnection =
214-
new InspectorPackagerConnection(
215-
getInspectorDeviceUrl(), mPackageName, mBundlerStatusProvider);
216-
mInspectorPackagerConnection.connect();
213+
new AsyncTask<Void, Void, Void>() {
214+
@Override
215+
protected Void doInBackground(Void... params) {
216+
mInspectorPackagerConnection =
217+
new InspectorPackagerConnection(
218+
getInspectorDeviceUrl(), mPackageName, mBundlerStatusProvider);
219+
mInspectorPackagerConnection.connect();
220+
return null;
221+
}
222+
}.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
217223
}
218224

219225
public void disableDebugger() {
@@ -223,10 +229,16 @@ public void disableDebugger() {
223229
}
224230

225231
public void closeInspectorConnection() {
226-
if (mInspectorPackagerConnection != null) {
227-
mInspectorPackagerConnection.closeQuietly();
228-
mInspectorPackagerConnection = null;
229-
}
232+
new AsyncTask<Void, Void, Void>() {
233+
@Override
234+
protected Void doInBackground(Void... params) {
235+
if (mInspectorPackagerConnection != null) {
236+
mInspectorPackagerConnection.closeQuietly();
237+
mInspectorPackagerConnection = null;
238+
}
239+
return null;
240+
}
241+
}.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
230242
}
231243

232244
public String getWebsocketProxyURL() {

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -942,15 +942,13 @@ private void reportBundleLoadingFailure(final Exception cause) {
942942

943943
@Override
944944
public void startInspector() {
945-
UiThreadUtil.assertOnUiThread();
946945
if (mIsDevSupportEnabled) {
947946
mDevServerHelper.openInspectorConnection();
948947
}
949948
}
950949

951950
@Override
952951
public void stopInspector() {
953-
UiThreadUtil.assertOnUiThread();
954952
mDevServerHelper.closeInspectorConnection();
955953
}
956954

@@ -1047,12 +1045,9 @@ public void onPackagerDisconnected() {
10471045

10481046
@Override
10491047
public void onPackagerReloadCommand() {
1050-
UiThreadUtil.runOnUiThread(
1051-
() -> {
1052-
// Disable debugger to resume the JsVM & avoid thread locks while reloading
1053-
mDevServerHelper.disableDebugger();
1054-
handleReloadJS();
1055-
});
1048+
// Disable debugger to resume the JsVM & avoid thread locks while reloading
1049+
mDevServerHelper.disableDebugger();
1050+
UiThreadUtil.runOnUiThread(() -> handleReloadJS());
10561051
}
10571052

10581053
@Override

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/InspectorPackagerConnection.java

Lines changed: 31 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,12 @@
88
package com.facebook.react.devsupport;
99

1010
import android.os.AsyncTask;
11+
import android.os.Handler;
12+
import android.os.Looper;
1113
import androidx.annotation.Nullable;
1214
import com.facebook.common.logging.FLog;
1315
import com.facebook.react.bridge.Inspector;
14-
import com.facebook.react.bridge.UiThreadUtil;
16+
import java.io.IOException;
1517
import java.util.HashMap;
1618
import java.util.List;
1719
import java.util.Map;
@@ -35,20 +37,17 @@ public class InspectorPackagerConnection {
3537

3638
public InspectorPackagerConnection(
3739
String url, String packageName, BundleStatusProvider bundleStatusProvider) {
38-
UiThreadUtil.assertOnUiThread();
3940
mConnection = new Connection(url);
4041
mInspectorConnections = new HashMap<>();
4142
mPackageName = packageName;
4243
mBundleStatusProvider = bundleStatusProvider;
4344
}
4445

4546
public void connect() {
46-
UiThreadUtil.assertOnUiThread();
4747
mConnection.connect();
4848
}
4949

5050
public void closeQuietly() {
51-
UiThreadUtil.assertOnUiThread();
5251
mConnection.close();
5352
}
5453

@@ -60,7 +59,7 @@ public void sendEventToAllConnections(String event) {
6059
}
6160
}
6261

63-
void handleProxyMessage(JSONObject message) throws JSONException {
62+
void handleProxyMessage(JSONObject message) throws JSONException, IOException {
6463
String event = message.getString("event");
6564
switch (event) {
6665
case "getPages":
@@ -76,8 +75,7 @@ void handleProxyMessage(JSONObject message) throws JSONException {
7675
handleDisconnect(message.getJSONObject("payload"));
7776
break;
7877
default:
79-
FLog.e(TAG, "Unknown event: " + event);
80-
break;
78+
throw new IllegalArgumentException("Unknown event: " + event);
8179
}
8280
}
8381

@@ -92,8 +90,7 @@ private void handleConnect(JSONObject payload) throws JSONException {
9290
final String pageId = payload.getString("pageId");
9391
Inspector.LocalConnection inspectorConnection = mInspectorConnections.remove(pageId);
9492
if (inspectorConnection != null) {
95-
FLog.w(TAG, "Already connected: " + pageId);
96-
return;
93+
throw new IllegalStateException("Already connected: " + pageId);
9794
}
9895

9996
try {
@@ -194,65 +191,49 @@ private class Connection extends WebSocketListener {
194191

195192
private OkHttpClient mHttpClient;
196193
private @Nullable WebSocket mWebSocket;
194+
private final Handler mHandler;
197195
private boolean mClosed;
198196
private boolean mSuppressConnectionErrors;
199197

200198
public Connection(String url) {
201199
mUrl = url;
200+
mHandler = new Handler(Looper.getMainLooper());
202201
}
203202

204203
@Override
205204
public void onOpen(WebSocket webSocket, Response response) {
206-
UiThreadUtil.runOnUiThread(
207-
() -> {
208-
mWebSocket = webSocket;
209-
});
205+
mWebSocket = webSocket;
210206
}
211207

212208
@Override
213209
public void onFailure(WebSocket webSocket, Throwable t, Response response) {
214-
UiThreadUtil.runOnUiThread(
215-
() -> {
216-
if (mWebSocket != null) {
217-
abort("Websocket exception", t);
218-
}
219-
if (!mClosed) {
220-
reconnect();
221-
}
222-
});
210+
if (mWebSocket != null) {
211+
abort("Websocket exception", t);
212+
}
213+
if (!mClosed) {
214+
reconnect();
215+
}
223216
}
224217

225218
@Override
226219
public void onMessage(WebSocket webSocket, String text) {
227220
try {
228-
JSONObject parsed = new JSONObject(text);
229-
UiThreadUtil.runOnUiThread(
230-
() -> {
231-
try {
232-
handleProxyMessage(parsed);
233-
} catch (JSONException e) {
234-
FLog.w(TAG, "Error handling inspector message", e);
235-
}
236-
});
237-
} catch (JSONException e) {
238-
FLog.w(TAG, "Unrecognized inspector message, string was not valid JSON: " + text, e);
221+
handleProxyMessage(new JSONObject(text));
222+
} catch (Exception e) {
223+
throw new RuntimeException(e);
239224
}
240225
}
241226

242227
@Override
243228
public void onClosed(WebSocket webSocket, int code, String reason) {
244-
UiThreadUtil.runOnUiThread(
245-
() -> {
246-
mWebSocket = null;
247-
closeAllConnections();
248-
if (!mClosed) {
249-
reconnect();
250-
}
251-
});
229+
mWebSocket = null;
230+
closeAllConnections();
231+
if (!mClosed) {
232+
reconnect();
233+
}
252234
}
253235

254236
public void connect() {
255-
UiThreadUtil.assertOnUiThread();
256237
if (mClosed) {
257238
throw new IllegalStateException("Can't connect closed client");
258239
}
@@ -277,18 +258,20 @@ private void reconnect() {
277258
FLog.w(TAG, "Couldn't connect to packager, will silently retry");
278259
mSuppressConnectionErrors = true;
279260
}
280-
UiThreadUtil.runOnUiThread(
281-
() -> {
282-
// check that we haven't been closed in the meantime
283-
if (!mClosed) {
284-
connect();
261+
mHandler.postDelayed(
262+
new Runnable() {
263+
@Override
264+
public void run() {
265+
// check that we haven't been closed in the meantime
266+
if (!mClosed) {
267+
connect();
268+
}
285269
}
286270
},
287271
RECONNECT_DELAY_MS);
288272
}
289273

290274
public void close() {
291-
UiThreadUtil.assertOnUiThread();
292275
mClosed = true;
293276
if (mWebSocket != null) {
294277
try {

0 commit comments

Comments
 (0)