Skip to content

Commit

Permalink
Synchronously decide if Android WebView should load a URL or not.
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@dvicory
dvicory committed Aug 24, 2020
1 parent 6598325 commit 773a3a4
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 21 deletions.
79 changes: 61 additions & 18 deletions android/src/main/java/com/reactnativecommunity/webview/RNCWebViewManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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}
Expand Down Expand Up @@ -113,6 +117,7 @@
*/
@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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<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
Expand Down Expand Up @@ -1164,6 +1207,7 @@ protected static class RNCWebView extends WebView implements LifecycleEventListe
*/
public RNCWebView(ThemedReactContext reactContext) {
super(reactContext);
this.createCatalystInstance();
progressChangedFilter = new ProgressChangedFilter();
}

Expand Down Expand Up @@ -1272,7 +1316,6 @@ public void setMessagingEnabled(boolean enabled) {

if (enabled) {
addJavascriptInterface(createRNCWebViewBridge(this), JAVASCRIPT_INTERFACE);
this.createCatalystInstance();
} else {
removeJavascriptInterface(JAVASCRIPT_INTERFACE);
}
Expand Down Expand Up @@ -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));
}
Expand All @@ -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) {
Expand Down
44 changes: 44 additions & 0 deletions android/src/main/java/com/reactnativecommunity/webview/RNCWebViewModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand All @@ -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"),
Expand Down Expand Up @@ -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<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) {
Expand Down
10 changes: 7 additions & 3 deletions src/WebView.android.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ class WebView extends React.Component<AndroidWebViewProps, State> {
lastErrorEvent: null,
};

onShouldStartLoadWithRequest: ReturnType<typeof createOnShouldStartLoadWithRequest> | null = null;

webViewRef = React.createRef<NativeWebViewAndroid>();

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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[],
Expand All @@ -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)}
Expand Down

0 comments on commit 773a3a4

Please sign in to comment.