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.

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<WebView> {
+  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<WebView> {
   // 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<WebView> {
 
     @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<Integer, AtomicReference<ShouldOverrideCallbackState>> lock = RNCWebViewModule.shouldOverrideUrlLoadingLock.getNewLock();
+        final int lockIdentifier = lock.first;
+        final AtomicReference<ShouldOverrideCallbackState> 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<WebView> {
      */
     public RNCWebView(ThemedReactContext reactContext) {
       super(reactContext);
+      this.createCatalystInstance();
       progressChangedFilter = new ProgressChangedFilter();
     }
 
@@ -1272,7 +1316,6 @@ public class RNCWebViewManager extends SimpleViewManager<WebView> {
 
       if (enabled) {
         addJavascriptInterface(createRNCWebViewBridge(this), JAVASCRIPT_INTERFACE);
-        this.createCatalystInstance();
       } else {
         removeJavascriptInterface(JAVASCRIPT_INTERFACE);
       }
@@ -1328,7 +1371,7 @@ public class RNCWebViewManager extends SimpleViewManager<WebView> {
             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<WebView> {
         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) {

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<Integer, AtomicReference<ShouldOverrideCallbackState>> shouldOverrideLocks = new HashMap<>();
+
+    public synchronized Pair<Integer, AtomicReference<ShouldOverrideCallbackState>> getNewLock() {
+      final int lockIdentifier = nextLockIdentifier++;
+      final AtomicReference<ShouldOverrideCallbackState> shouldOverride = new AtomicReference<>(ShouldOverrideCallbackState.UNDECIDED);
+      shouldOverrideLocks.put(lockIdentifier, shouldOverride);
+      return new Pair<>(lockIdentifier, shouldOverride);
+    }
+
+    @Nullable
+    public synchronized AtomicReference<ShouldOverrideCallbackState> 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<ShouldOverrideUrlLoadingLock.ShouldOverrideCallbackState> 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) {

src/WebView.android.tsx

@@ -77,6 +77,7 @@ class WebView extends React.Component<AndroidWebViewProps, State> {
     lastErrorEvent: null,
   };
 
+  onShouldStartLoadWithRequest: ReturnType<typeof createOnShouldStartLoadWithRequest> | null = null;
 
   webViewRef = React.createRef<NativeWebViewAndroid>();
 
@@ -280,8 +281,11 @@ class WebView extends React.Component<AndroidWebViewProps, State> {
   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<AndroidWebViewProps, State> {
     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<AndroidWebViewProps, State> {
         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)}