From 4d4b5e23871f6dcc699f062b875729b267be09a9 Mon Sep 17 00:00:00 2001 From: Daniel Vicory Date: Tue, 25 Aug 2020 23:59:51 -0700 Subject: [PATCH] Synchronously decide if Android WebView should load a URL or not. (#1590) This solves a class of issues when the WebView loses "context" that a subsequent page load is the same as what was attempted to be loaded previously. This solves a bug where a HTTP redirect in combination with history manipulations causes a user to be stuck and prevented from going back. Since WebView requests are allowed to happen normally, debugging the WebView and tracking redirects and page load initiators is more accurate and easier. This will also bypass bridge latency and provide a faster navigation. To do this, we must lock in the shouldOverrideUrlLoading callback and send an event to JS. Currently, this callback is ran on the main UI thread, of which we have no control over. This is problematic as using the bridge in most ways seems to require the main UI thread, which will cause a deadlock. However, using BatchedBridge for Java->JS and a synchronous method for JS->Java doesn't cause any problems. Additionally, it's been designed so that if WebView suddenly runs the callback on a different thread allowing for concurrency, it will continue to work. --- .../webview/RNCWebViewManager.java | 79 ++++++++++++++----- .../webview/RNCWebViewModule.java | 44 +++++++++++ src/WebView.android.tsx | 10 ++- 3 files changed, 112 insertions(+), 21 deletions(-) diff --git a/android/src/main/java/com/reactnativecommunity/webview/RNCWebViewManager.java b/android/src/main/java/com/reactnativecommunity/webview/RNCWebViewManager.java index 0461a0c..8d2cdc2 100644 --- a/android/src/main/java/com/reactnativecommunity/webview/RNCWebViewManager.java +++ b/android/src/main/java/com/reactnativecommunity/webview/RNCWebViewManager.java @@ -4,7 +4,6 @@ import android.annotation.SuppressLint; import android.annotation.TargetApi; import android.app.DownloadManager; import android.content.Context; -import android.content.Intent; import android.content.pm.ActivityInfo; import android.content.pm.PackageManager; import android.graphics.Bitmap; @@ -14,8 +13,7 @@ import android.net.http.SslError; import android.net.Uri; import android.os.Build; import android.os.Environment; -import androidx.annotation.RequiresApi; -import androidx.core.content.ContextCompat; +import android.os.SystemClock; import android.text.TextUtils; import android.util.Log; import android.view.Gravity; @@ -41,6 +39,12 @@ import android.webkit.WebView; import android.webkit.WebViewClient; import android.widget.FrameLayout; +import androidx.annotation.Nullable; +import androidx.annotation.RequiresApi; +import androidx.core.content.ContextCompat; +import androidx.core.util.Pair; + +import com.facebook.common.logging.FLog; import com.facebook.react.views.scroll.ScrollEvent; import com.facebook.react.views.scroll.ScrollEventType; import com.facebook.react.views.scroll.OnScrollDispatchHelper; @@ -64,6 +68,7 @@ import com.facebook.react.uimanager.annotations.ReactProp; import com.facebook.react.uimanager.events.ContentSizeChangeEvent; import com.facebook.react.uimanager.events.Event; import com.facebook.react.uimanager.events.EventDispatcher; +import com.reactnativecommunity.webview.RNCWebViewModule.ShouldOverrideUrlLoadingLock.ShouldOverrideCallbackState; import com.reactnativecommunity.webview.events.TopLoadingErrorEvent; import com.reactnativecommunity.webview.events.TopHttpErrorEvent; import com.reactnativecommunity.webview.events.TopLoadingFinishEvent; @@ -84,8 +89,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.Locale; import java.util.Map; - -import javax.annotation.Nullable; +import java.util.concurrent.atomic.AtomicReference; /** * Manages instances of {@link WebView} @@ -113,6 +117,7 @@ import javax.annotation.Nullable; */ @ReactModule(name = RNCWebViewManager.REACT_CLASS) public class RNCWebViewManager extends SimpleViewManager { + private static final String TAG = "RNCWebViewManager"; public static final int COMMAND_GO_BACK = 1; public static final int COMMAND_GO_FORWARD = 2; @@ -136,6 +141,7 @@ public class RNCWebViewManager extends SimpleViewManager { // Use `webView.loadUrl("about:blank")` to reliably reset the view // state and release page resources (including any running JavaScript). protected static final String BLANK_URL = "about:blank"; + protected static final int SHOULD_OVERRIDE_URL_LOADING_TIMEOUT = 250; protected WebViewConfig mWebViewConfig; protected RNCWebChromeClient mWebChromeClient = null; @@ -806,15 +812,52 @@ public class RNCWebViewManager extends SimpleViewManager { @Override public boolean shouldOverrideUrlLoading(WebView view, String url) { - progressChangedFilter.setWaitingForCommandLoadUrl(true); - dispatchEvent( - view, - new TopShouldStartLoadWithRequestEvent( - view.getId(), - createWebViewEvent(view, url))); - return true; - } + final RNCWebView rncWebView = (RNCWebView) view; + final boolean isJsDebugging = ((ReactContext) view.getContext()).getJavaScriptContextHolder().get() == 0; + if (!isJsDebugging && rncWebView.mCatalystInstance != null) { + final Pair> lock = RNCWebViewModule.shouldOverrideUrlLoadingLock.getNewLock(); + final int lockIdentifier = lock.first; + final AtomicReference lockObject = lock.second; + + final WritableMap event = createWebViewEvent(view, url); + event.putInt("lockIdentifier", lockIdentifier); + rncWebView.sendDirectMessage("onShouldStartLoadWithRequest", event); + + try { + assert lockObject != null; + synchronized (lockObject) { + final long startTime = SystemClock.elapsedRealtime(); + while (lockObject.get() == ShouldOverrideCallbackState.UNDECIDED) { + if (SystemClock.elapsedRealtime() - startTime > SHOULD_OVERRIDE_URL_LOADING_TIMEOUT) { + FLog.w(TAG, "Did not receive response to shouldOverrideUrlLoading in time, defaulting to allow loading."); + RNCWebViewModule.shouldOverrideUrlLoadingLock.removeLock(lockIdentifier); + return false; + } + lockObject.wait(SHOULD_OVERRIDE_URL_LOADING_TIMEOUT); + } + } + } catch (InterruptedException e) { + FLog.e(TAG, "shouldOverrideUrlLoading was interrupted while waiting for result.", e); + RNCWebViewModule.shouldOverrideUrlLoadingLock.removeLock(lockIdentifier); + return false; + } + + final boolean shouldOverride = lockObject.get() == ShouldOverrideCallbackState.SHOULD_OVERRIDE; + RNCWebViewModule.shouldOverrideUrlLoadingLock.removeLock(lockIdentifier); + + return shouldOverride; + } else { + FLog.w(TAG, "Couldn't use blocking synchronous call for onShouldStartLoadWithRequest due to debugging or missing Catalyst instance, falling back to old event-and-load."); + progressChangedFilter.setWaitingForCommandLoadUrl(true); + dispatchEvent( + view, + new TopShouldStartLoadWithRequestEvent( + view.getId(), + createWebViewEvent(view, url))); + return true; + } + } @TargetApi(Build.VERSION_CODES.N) @Override @@ -1164,6 +1207,7 @@ public class RNCWebViewManager extends SimpleViewManager { */ public RNCWebView(ThemedReactContext reactContext) { super(reactContext); + this.createCatalystInstance(); progressChangedFilter = new ProgressChangedFilter(); } @@ -1272,7 +1316,6 @@ public class RNCWebViewManager extends SimpleViewManager { if (enabled) { addJavascriptInterface(createRNCWebViewBridge(this), JAVASCRIPT_INTERFACE); - this.createCatalystInstance(); } else { removeJavascriptInterface(JAVASCRIPT_INTERFACE); } @@ -1328,7 +1371,7 @@ public class RNCWebViewManager extends SimpleViewManager { data.putString("data", message); if (mCatalystInstance != null) { - mContext.sendDirectMessage(data); + mContext.sendDirectMessage("onMessage", data); } else { dispatchEvent(webView, new TopMessageEvent(webView.getId(), data)); } @@ -1339,21 +1382,21 @@ public class RNCWebViewManager extends SimpleViewManager { eventData.putString("data", message); if (mCatalystInstance != null) { - this.sendDirectMessage(eventData); + this.sendDirectMessage("onMessage", eventData); } else { dispatchEvent(this, new TopMessageEvent(this.getId(), eventData)); } } } - protected void sendDirectMessage(WritableMap data) { + protected void sendDirectMessage(final String method, WritableMap data) { WritableNativeMap event = new WritableNativeMap(); event.putMap("nativeEvent", data); WritableNativeArray params = new WritableNativeArray(); params.pushMap(event); - mCatalystInstance.callFunction(messagingModuleName, "onMessage", params); + mCatalystInstance.callFunction(messagingModuleName, method, params); } protected void onScrollChanged(int x, int y, int oldX, int oldY) { diff --git a/android/src/main/java/com/reactnativecommunity/webview/RNCWebViewModule.java b/android/src/main/java/com/reactnativecommunity/webview/RNCWebViewModule.java index 465353d..d0e7fb3 100644 --- a/android/src/main/java/com/reactnativecommunity/webview/RNCWebViewModule.java +++ b/android/src/main/java/com/reactnativecommunity/webview/RNCWebViewModule.java @@ -12,9 +12,11 @@ import android.os.Environment; import android.os.Parcelable; import android.provider.MediaStore; +import androidx.annotation.Nullable; import androidx.annotation.RequiresApi; import androidx.core.content.ContextCompat; import androidx.core.content.FileProvider; +import androidx.core.util.Pair; import android.util.Log; import android.webkit.MimeTypeMap; @@ -35,6 +37,8 @@ import java.io.File; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; +import java.util.concurrent.atomic.AtomicReference; import static android.app.Activity.RESULT_OK; @@ -50,6 +54,35 @@ public class RNCWebViewModule extends ReactContextBaseJavaModule implements Acti private File outputVideo; private DownloadManager.Request downloadRequest; + protected static class ShouldOverrideUrlLoadingLock { + protected enum ShouldOverrideCallbackState { + UNDECIDED, + SHOULD_OVERRIDE, + DO_NOT_OVERRIDE, + } + + private int nextLockIdentifier = 0; + private final HashMap> shouldOverrideLocks = new HashMap<>(); + + public synchronized Pair> getNewLock() { + final int lockIdentifier = nextLockIdentifier++; + final AtomicReference shouldOverride = new AtomicReference<>(ShouldOverrideCallbackState.UNDECIDED); + shouldOverrideLocks.put(lockIdentifier, shouldOverride); + return new Pair<>(lockIdentifier, shouldOverride); + } + + @Nullable + public synchronized AtomicReference getLock(Integer lockIdentifier) { + return shouldOverrideLocks.get(lockIdentifier); + } + + public synchronized void removeLock(Integer lockIdentifier) { + shouldOverrideLocks.remove(lockIdentifier); + } + } + + protected static final ShouldOverrideUrlLoadingLock shouldOverrideUrlLoadingLock = new ShouldOverrideUrlLoadingLock(); + private enum MimeType { DEFAULT("*/*"), IMAGE("image"), @@ -105,6 +138,17 @@ public class RNCWebViewModule extends ReactContextBaseJavaModule implements Acti promise.resolve(result); } + @ReactMethod(isBlockingSynchronousMethod = true) + public void onShouldStartLoadWithRequestCallback(final boolean shouldStart, final int lockIdentifier) { + final AtomicReference lockObject = shouldOverrideUrlLoadingLock.getLock(lockIdentifier); + if (lockObject != null) { + synchronized (lockObject) { + lockObject.set(shouldStart ? ShouldOverrideUrlLoadingLock.ShouldOverrideCallbackState.DO_NOT_OVERRIDE : ShouldOverrideUrlLoadingLock.ShouldOverrideCallbackState.SHOULD_OVERRIDE); + lockObject.notify(); + } + } + } + public void onActivityResult(Activity activity, int requestCode, int resultCode, Intent data) { if (filePathCallback == null && filePathCallbackLegacy == null) { diff --git a/src/WebView.android.tsx b/src/WebView.android.tsx index f20d2cb..bdc8027 100644 --- a/src/WebView.android.tsx +++ b/src/WebView.android.tsx @@ -77,6 +77,7 @@ class WebView extends React.Component { lastErrorEvent: null, }; + onShouldStartLoadWithRequest: ReturnType | null = null; webViewRef = React.createRef(); @@ -280,8 +281,11 @@ class WebView extends React.Component { onShouldStartLoadWithRequestCallback = ( shouldStart: boolean, url: string, + lockIdentifier?: number, ) => { - if (shouldStart) { + if (lockIdentifier) { + NativeModules.RNCWebView.onShouldStartLoadWithRequestCallback(shouldStart, lockIdentifier); + } else if (shouldStart) { UIManager.dispatchViewManagerCommand( this.getWebViewHandle(), this.getCommands().loadUrl, @@ -338,7 +342,7 @@ class WebView extends React.Component { const NativeWebView = (nativeConfig.component as typeof NativeWebViewAndroid) || RNCWebView; - const onShouldStartLoadWithRequest = createOnShouldStartLoadWithRequest( + this.onShouldStartLoadWithRequest = createOnShouldStartLoadWithRequest( this.onShouldStartLoadWithRequestCallback, // casting cause it's in the default props originWhitelist as readonly string[], @@ -358,7 +362,7 @@ class WebView extends React.Component { onHttpError={this.onHttpError} onRenderProcessGone={this.onRenderProcessGone} onMessage={this.onMessage} - onShouldStartLoadWithRequest={onShouldStartLoadWithRequest} + onShouldStartLoadWithRequest={this.onShouldStartLoadWithRequest} ref={this.webViewRef} // TODO: find a better way to type this. source={resolveAssetSource(source as ImageSourcePropType)}