From 773a3a4318e61f79589ca53b85a026006b1891af Mon Sep 17 00:00:00 2001 From: Daniel Vicory Date: Mon, 17 Aug 2020 22:57:09 -0700 Subject: [PATCH] Synchronously decide if Android WebView should load a URL or not. 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 0461a0c48..8d2cdc271 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.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.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.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.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.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 @@ */ @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 void onPageStarted(WebView webView, String url, Bitmap favicon) { @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 @@ protected static class RNCWebView extends WebView implements LifecycleEventListe */ public RNCWebView(ThemedReactContext reactContext) { super(reactContext); + this.createCatalystInstance(); progressChangedFilter = new ProgressChangedFilter(); } @@ -1272,7 +1316,6 @@ public void setMessagingEnabled(boolean enabled) { if (enabled) { addJavascriptInterface(createRNCWebViewBridge(this), JAVASCRIPT_INTERFACE); - this.createCatalystInstance(); } else { removeJavascriptInterface(JAVASCRIPT_INTERFACE); } @@ -1328,7 +1371,7 @@ public void run() { 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 void run() { 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 465353d3d..d0e7fb367 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.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.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 void isFileUploadSupported(final Promise promise) { 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 f20d2cbd8..bdc802769 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)}