From 7c6eb1fecb1e3a74e382aba5dd2aba38b4cf3ec4 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Thu, 10 Jun 2021 16:45:35 -0700 Subject: [PATCH] Move handleReloadJS() from DevSupportManagerBase to BridgeDevSupportManager Summary: ## Rationale DevSupportManagerBase.handleReloadJS() implements reloads for the bridge. Therefore, it's best to move this method to BridgeDevSupportManager. Changelog: [Internal] Reviewed By: JoshuaGross Differential Revision: D29004541 fbshipit-source-id: f77244e9c44cd442e7e0ab2845f78d699b143e66 --- .../devsupport/BridgeDevSupportManager.java | 89 ++++++++++++++++ .../devsupport/DevSupportManagerBase.java | 100 +++--------------- 2 files changed, 105 insertions(+), 84 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/BridgeDevSupportManager.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/BridgeDevSupportManager.java index abe00a900a5601..462f064747cf15 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/BridgeDevSupportManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/BridgeDevSupportManager.java @@ -9,12 +9,26 @@ import android.content.Context; import androidx.annotation.Nullable; +import com.facebook.common.logging.FLog; +import com.facebook.debug.holder.PrinterHolder; +import com.facebook.debug.tags.ReactDebugOverlayTags; +import com.facebook.infer.annotation.Assertions; import com.facebook.react.bridge.CatalystInstance; import com.facebook.react.bridge.JSBundleLoader; +import com.facebook.react.bridge.JavaJSExecutor; +import com.facebook.react.bridge.ReactMarker; +import com.facebook.react.bridge.ReactMarkerConstants; +import com.facebook.react.bridge.UiThreadUtil; +import com.facebook.react.common.ReactConstants; +import com.facebook.react.common.futures.SimpleSettableFuture; import com.facebook.react.devsupport.interfaces.DevBundleDownloadListener; import com.facebook.react.devsupport.interfaces.DevSplitBundleCallback; import com.facebook.react.packagerconnection.RequestHandler; +import java.io.IOException; import java.util.Map; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; /** * Interface for accessing and interacting with development features. Following features @@ -103,4 +117,79 @@ public void onError(String url, Throwable cause) { } }); } + + private WebsocketJavaScriptExecutor.JSExecutorConnectCallback getExecutorConnectCallback( + final SimpleSettableFuture future) { + return new WebsocketJavaScriptExecutor.JSExecutorConnectCallback() { + @Override + public void onSuccess() { + future.set(true); + hideDevLoadingView(); + } + + @Override + public void onFailure(final Throwable cause) { + hideDevLoadingView(); + FLog.e(ReactConstants.TAG, "Failed to connect to debugger!", cause); + future.setException( + new IOException( + getApplicationContext().getString(com.facebook.react.R.string.catalyst_debug_error), + cause)); + } + }; + } + + private void reloadJSInProxyMode() { + // When using js proxy, there is no need to fetch JS bundle as proxy executor will do that + // anyway + getDevServerHelper().launchJSDevtools(); + + JavaJSExecutor.Factory factory = + new JavaJSExecutor.Factory() { + @Override + public JavaJSExecutor create() throws Exception { + WebsocketJavaScriptExecutor executor = new WebsocketJavaScriptExecutor(); + SimpleSettableFuture future = new SimpleSettableFuture<>(); + executor.connect( + getDevServerHelper().getWebsocketProxyURL(), getExecutorConnectCallback(future)); + // TODO(t9349129) Don't use timeout + try { + future.get(90, TimeUnit.SECONDS); + return executor; + } catch (ExecutionException e) { + throw (Exception) e.getCause(); + } catch (InterruptedException | TimeoutException e) { + throw new RuntimeException(e); + } + } + }; + getReactInstanceDevHelper().onReloadWithJSDebugger(factory); + } + + @Override + public void handleReloadJS() { + + UiThreadUtil.assertOnUiThread(); + + ReactMarker.logMarker( + ReactMarkerConstants.RELOAD, + getDevSettings().getPackagerConnectionSettings().getDebugServerHost()); + + // dismiss redbox if exists + hideRedboxDialog(); + + if (getDevSettings().isRemoteJSDebugEnabled()) { + PrinterHolder.getPrinter() + .logMessage(ReactDebugOverlayTags.RN_CORE, "RNCore: load from Proxy"); + showDevLoadingViewForRemoteJSEnabled(); + reloadJSInProxyMode(); + } else { + PrinterHolder.getPrinter() + .logMessage(ReactDebugOverlayTags.RN_CORE, "RNCore: load from Server"); + String bundleURL = + getDevServerHelper() + .getDevServerBundleURL(Assertions.assertNotNull(getJSAppBundleName())); + reloadJSFromServer(bundleURL); + } + } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java index 07c371e15aa40b..79c049e11e8627 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java @@ -25,13 +25,10 @@ import androidx.annotation.Nullable; import androidx.annotation.UiThread; import com.facebook.common.logging.FLog; -import com.facebook.debug.holder.PrinterHolder; -import com.facebook.debug.tags.ReactDebugOverlayTags; import com.facebook.infer.annotation.Assertions; import com.facebook.react.R; import com.facebook.react.bridge.DefaultNativeModuleCallExceptionHandler; import com.facebook.react.bridge.JSBundleLoader; -import com.facebook.react.bridge.JavaJSExecutor; import com.facebook.react.bridge.JavaScriptExecutorFactory; import com.facebook.react.bridge.ReactContext; import com.facebook.react.bridge.ReactMarker; @@ -41,7 +38,6 @@ import com.facebook.react.common.DebugServerException; import com.facebook.react.common.ReactConstants; import com.facebook.react.common.ShakeDetector; -import com.facebook.react.common.futures.SimpleSettableFuture; import com.facebook.react.devsupport.DevServerHelper.PackagerCommandListener; import com.facebook.react.devsupport.interfaces.BundleLoadCallback; import com.facebook.react.devsupport.interfaces.DevBundleDownloadListener; @@ -52,7 +48,6 @@ import com.facebook.react.devsupport.interfaces.PackagerStatusCallback; import com.facebook.react.devsupport.interfaces.StackFrame; import com.facebook.react.modules.core.RCTNativeAppEventEmitter; -import com.facebook.react.modules.debug.interfaces.DeveloperSettings; import com.facebook.react.packagerconnection.RequestHandler; import com.facebook.react.packagerconnection.Responder; import java.io.File; @@ -64,9 +59,6 @@ import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; public abstract class DevSupportManagerBase implements DevSupportManager, PackagerCommandListener, DevInternalSettings.Listener { @@ -711,7 +703,7 @@ public boolean getDevSupportEnabled() { } @Override - public DeveloperSettings getDevSettings() { + public DevInternalSettings getDevSettings() { return mDevSettings; } @@ -841,34 +833,24 @@ public void onInternalSettingsChanged() { reloadSettings(); } - @Override - public void handleReloadJS() { - - UiThreadUtil.assertOnUiThread(); + protected @Nullable ReactContext getCurrentContext() { + return mCurrentContext; + } - ReactMarker.logMarker( - ReactMarkerConstants.RELOAD, - mDevSettings.getPackagerConnectionSettings().getDebugServerHost()); + protected @Nullable String getJSAppBundleName() { + return mJSAppBundleName; + } - // dismiss redbox if exists - hideRedboxDialog(); + protected Context getApplicationContext() { + return mApplicationContext; + } - if (mDevSettings.isRemoteJSDebugEnabled()) { - PrinterHolder.getPrinter() - .logMessage(ReactDebugOverlayTags.RN_CORE, "RNCore: load from Proxy"); - showDevLoadingViewForRemoteJSEnabled(); - reloadJSInProxyMode(); - } else { - PrinterHolder.getPrinter() - .logMessage(ReactDebugOverlayTags.RN_CORE, "RNCore: load from Server"); - String bundleURL = - mDevServerHelper.getDevServerBundleURL(Assertions.assertNotNull(mJSAppBundleName)); - reloadJSFromServer(bundleURL); - } + protected DevServerHelper getDevServerHelper() { + return mDevServerHelper; } - protected @Nullable ReactContext getCurrentContext() { - return mCurrentContext; + protected ReactInstanceDevHelper getReactInstanceDevHelper() { + return mReactInstanceDevHelper; } @UiThread @@ -878,21 +860,17 @@ private void showDevLoadingViewForUrl(String bundleUrl) { } @UiThread - private void showDevLoadingViewForRemoteJSEnabled() { + protected void showDevLoadingViewForRemoteJSEnabled() { mDevLoadingViewController.showForRemoteJSEnabled(); mDevLoadingViewVisible = true; } @UiThread - private void hideDevLoadingView() { + protected void hideDevLoadingView() { mDevLoadingViewController.hide(); mDevLoadingViewVisible = false; } - protected DevServerHelper getDevServerHelper() { - return mDevServerHelper; - } - public void fetchSplitBundleAndCreateBundleLoader( String bundlePath, final CallbackWithBundleLoader callback) { final String bundleUrl = mDevServerHelper.getDevServerSplitBundleURL(bundlePath); @@ -1087,52 +1065,6 @@ private void updateLastErrorInfo( mLastErrorType = errorType; } - private void reloadJSInProxyMode() { - // When using js proxy, there is no need to fetch JS bundle as proxy executor will do that - // anyway - mDevServerHelper.launchJSDevtools(); - - JavaJSExecutor.Factory factory = - new JavaJSExecutor.Factory() { - @Override - public JavaJSExecutor create() throws Exception { - WebsocketJavaScriptExecutor executor = new WebsocketJavaScriptExecutor(); - SimpleSettableFuture future = new SimpleSettableFuture<>(); - executor.connect( - mDevServerHelper.getWebsocketProxyURL(), getExecutorConnectCallback(future)); - // TODO(t9349129) Don't use timeout - try { - future.get(90, TimeUnit.SECONDS); - return executor; - } catch (ExecutionException e) { - throw (Exception) e.getCause(); - } catch (InterruptedException | TimeoutException e) { - throw new RuntimeException(e); - } - } - }; - mReactInstanceDevHelper.onReloadWithJSDebugger(factory); - } - - private WebsocketJavaScriptExecutor.JSExecutorConnectCallback getExecutorConnectCallback( - final SimpleSettableFuture future) { - return new WebsocketJavaScriptExecutor.JSExecutorConnectCallback() { - @Override - public void onSuccess() { - future.set(true); - hideDevLoadingView(); - } - - @Override - public void onFailure(final Throwable cause) { - hideDevLoadingView(); - FLog.e(ReactConstants.TAG, "Failed to connect to debugger!", cause); - future.setException( - new IOException(mApplicationContext.getString(R.string.catalyst_debug_error), cause)); - } - }; - } - public void reloadJSFromServer(final String bundleURL) { reloadJSFromServer( bundleURL,