feat: support async onShouldStartLoad requests #49

kbulgakov-exo · 2026-01-18T19:09:56Z

The existing approach with locking the main thread to make a synchronous call asking JS "can I proceed with this URL" isn't reliable. Even the recent timeout bump to 500ms #47 didn't help, we still experience the loading issues. Bumping the timeout even more (for example with #48) would inevitably lead to performance degradation because of the locked thread. We should come up with a more reliable solution

The bottleneck

I've put some logs to diagnose what exactly is the bottleneck. The JS callback works super fast, but the problem is that sometimes it seems to be called after Native already makes the decision after the 500ms default timeout:

 Timeline:
  0ms      Native dispatches event
           |
           |  (JS thread busy)
           |
  501ms    Native TIMEOUT → blocks navigation
           |
 >501ms   JS receives event, responds "allow"
           |
           ✗ Response ignored - decision already made

The solution

Let's mirror the async architecture of the onShouldStartLoad calls from https://github.com/react-native-webview/react-native-webview. We can easily backport most of the code.

I recommend reviewing commit by commit

Notes:

the first commit cherry-picks WebViewDecisionManager from upstream with minor formatting changes
the other commits rely on the upstream code as well, but with a few minor changes to conform to our codebase. I’m going to leave the code references below.

Testing

WebView loads content normally when JS responds with true
WebView rejects loading when JS responds with false

Co-authored-by: Thibault Malbranche <malbranche.thibault@gmail.com>

kbulgakov-exo · 2026-01-18T19:15:08Z

apple/RNCWebView.m

+    int lockIdentifier = [[RNCWebViewDecisionManager getInstance] setDecisionHandler:^(BOOL shouldStart) {
+      dispatch_async(dispatch_get_main_queue(), ^{
+        if (!shouldStart) {
+          decisionHandler(WKNavigationActionPolicyCancel);
+          return;
+        }
+        allowNavigation();
+      });
+    }];
+


Reference: https://github.com/react-native-webview/react-native-webview/blob/5bc526fce5b9d6225df183bdf3d8cf542007d90a/apple/RNCWebViewImpl.m#L1366-L1386

The allowing code is moved into the allowNavigation helper to be reused below to keep the old fallback behavior

kbulgakov-exo · 2026-01-18T19:22:19Z

apple/RNCWebViewManager.m

-    RCTLogWarn(@"startLoadWithResult invoked with invalid lockIdentifier: "
-               "got %lld, expected %lld", (long long)lockIdentifier, (long long)_shouldStartLoadLock.condition);
-  }
+  [[RNCWebViewDecisionManager getInstance] setResult:result forLockIdentifier:(int)lockIdentifier];


Reference: https://github.com/react-native-webview/react-native-webview/blob/5bc526fce5b9d6225df183bdf3d8cf542007d90a/apple/RNCWebViewModule.mm#L21

kbulgakov-exo

tACK

WebView loads content normally when JS responds with true
WebView rejects loading when JS responds with false

guten-exodus

utACK nice

kbulgakov-exo · 2026-01-19T20:17:50Z

@guten-exodus Could you check whether this also addresses the pain points you had in Pay?

timlanahan

@ChALkeR please review

apple/RNCWebView.m

guten-exodus · 2026-01-20T02:51:58Z

Could you check whether this also addresses the pain points you had in Pay?

Tested, worked great!

apple/RNCWebViewDecisionManager.m

kewdex

utACK

ChALkeR

@timlanahan This should be tested in an emulator by someone actively trying to bypass that, including (but not limited to) in the following conditions:

main thread hanged
main thread is slow and does not respond in time
there are several requests queued some of which should pass and some should not - we should not load the non-passing one
(3) but main thread is slow
handler was not even installed in time before the load happened
All of the above but the load requests are spammed, mixing allowed and non-allowed ones

The current test plan is unsatisfactory

Copilot

Pull request overview

This PR migrates from a synchronous blocking approach to an asynchronous architecture for handling onShouldStartLoad navigation decisions in iOS/macOS WebView. The synchronous approach was causing reliability issues when the JavaScript thread couldn't respond within the 500ms timeout, leading to blocked navigations even when JS eventually responded with "allow".

Changes:

Introduced RNCWebViewDecisionManager singleton to manage async navigation decisions across threads
Removed synchronous RNCWebViewDelegate protocol and blocking timeout logic
Updated navigation decision flow to store handlers and resolve them asynchronously when JS responds

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
apple/RNCWebViewDecisionManager.h	New header defining the decision manager interface with thread-safe handler storage
apple/RNCWebViewDecisionManager.m	Implementation of singleton pattern for managing async navigation decision handlers
apple/RNCWebViewManager.m	Simplified to delegate decision resolution to RNCWebViewDecisionManager, removed blocking logic
apple/RNCWebView.h	Removed RNCWebViewDelegate protocol definition
apple/RNCWebView.m	Updated decidePolicyForNavigationAction to use async decision handlers instead of synchronous delegate calls

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-23T10:58:56Z

apple/RNCWebViewDecisionManager.m

+ */
+- (int)setDecisionHandler:(DecisionBlock)decisionHandler {
+    @synchronized (self) {
+        int lockIdentifier = self.nextLockIdentifier++;


The nextLockIdentifier counter could theoretically overflow after 2^31-1 increments (about 2.1 billion). While unlikely in normal usage, if this happens, the identifier will wrap around to negative values or repeat, potentially causing collisions with pending navigation decisions. Consider using a more robust identifier generation strategy, such as checking for existing keys before assignment, or using a larger integer type.

apple/RNCWebViewDecisionManager.m

Copilot · 2026-01-23T10:58:57Z

apple/RNCWebViewDecisionManager.h

+@interface RNCWebViewDecisionManager : NSObject {
+    int nextLockIdentifier;
+    NSMutableDictionary *decisionHandlers;
+}
+
+@property (nonatomic) int nextLockIdentifier;
+@property (nonatomic, retain) NSMutableDictionary *decisionHandlers;
+


The instance variables in the @interface block are redundant when @Property declarations are present, as properties automatically synthesize their backing instance variables. The duplicate declarations of nextLockIdentifier and decisionHandlers in lines 6-7 should be removed to follow modern Objective-C conventions.

Suggested change

@interface RNCWebViewDecisionManager : NSObject {

int nextLockIdentifier;

NSMutableDictionary *decisionHandlers;

}

@property (nonatomic) int nextLockIdentifier;

@property (nonatomic, retain) NSMutableDictionary *decisionHandlers;

@interface RNCWebViewDecisionManager : NSObject

@property (nonatomic) int nextLockIdentifier;

@property (nonatomic, retain) NSMutableDictionary *decisionHandlers;

apple/RNCWebViewDecisionManager.m

Copilot · 2026-01-23T10:58:57Z

apple/RNCWebViewDecisionManager.h

+}
+
+@property (nonatomic) int nextLockIdentifier;
+@property (nonatomic, retain) NSMutableDictionary *decisionHandlers;


The decisionHandlers property should use 'copy' semantic instead of 'retain' for storing blocks. In Objective-C, blocks need to be copied to move them from the stack to the heap to ensure they remain valid after the scope in which they were created ends. This is a critical memory management issue that could lead to crashes when the block is invoked after the original stack frame is destroyed.

Copilot · 2026-01-23T10:58:58Z

apple/RNCWebView.m

+    int lockIdentifier = [[RNCWebViewDecisionManager getInstance] setDecisionHandler:^(BOOL shouldStart) {
+      dispatch_async(dispatch_get_main_queue(), ^{
+        if (!shouldStart) {
+          decisionHandler(WKNavigationActionPolicyCancel);
+          return;
+        }
+        allowNavigation();
+      });
+    }];


When the WKWebView is deallocated while there are pending navigation decisions, the stored decision handlers in the singleton RNCWebViewDecisionManager will never be called, leading to a memory leak. The decisionHandler closure from WKWebView's decidePolicyForNavigationAction captures references that won't be released until the handler is invoked. Consider adding a mechanism to cancel pending decisions when the webview is deallocated, or add a timeout mechanism to automatically clean up stale handlers.

Copilot · 2026-01-23T10:58:58Z

apple/RNCWebViewDecisionManager.m

+- (void) setResult:(BOOL)shouldStart
+ forLockIdentifier:(int)lockIdentifier {
+    DecisionBlock handler;
+    @synchronized (self) {
+        handler = [self.decisionHandlers objectForKey:@(lockIdentifier)];
+        if (handler == nil) {
+            RCTLogWarn(@"Lock not found");
+            return;
+        }
+        [self.decisionHandlers removeObjectForKey:@(lockIdentifier)];
+    }
+    handler(shouldStart);


The WKWebView decisionHandler must be called exactly once. If JS calls startLoadWithResult multiple times with the same lockIdentifier (either due to a bug or race condition), the decisionHandler will be invoked multiple times, which will cause a crash with the error "Completion handler passed to -[WKWebView decidePolicyForNavigationAction:decisionHandler:] was called more than once". Consider adding a guard to track whether a decision has already been made for a given lockIdentifier to prevent double invocation.

kbulgakov-exo · 2026-01-23T14:00:23Z

There’s an ongoing effort to upgrade @exodus/react-native-webview to the latest upstream version, which already includes the changes I’m backporting in this PR. I was going to backport them specifically for the WC case, but since it’s not going to Prod anytime soon, I think it’s better to wait for the proper upgrade instead. I’m going to keep this PR open for future reference in case it’s helpful.

kbulgakov-exo and others added 4 commits January 17, 2026 21:59

feat: implement WebViewDecisionManager

1e6070c

Co-authored-by: Thibault Malbranche <malbranche.thibault@gmail.com>

refactor: WebView manager should rely on WebViewDecisionManager

adfe5ff

refactor: make reusable allowNavigation block

8a11f80

feat: support async navigation decision handling

4b0427b

kbulgakov-exo requested review from a team and 633kh4ck January 18, 2026 19:09

kbulgakov-exo self-assigned this Jan 18, 2026

kbulgakov-exo commented Jan 18, 2026

View reviewed changes

kbulgakov-exo added 2 commits January 18, 2026 22:27

chore: remove unnecessary pragma mark

20b4976

refactor: use consistent int type for lockIdentifier

33f9051

kbulgakov-exo commented Jan 18, 2026

View reviewed changes

kbulgakov-exo mentioned this pull request Jan 18, 2026

feat: support shouldStartLoadTimeout parameter (iOS) #48

Closed

3 tasks

guten-exodus approved these changes Jan 19, 2026

View reviewed changes

timlanahan reviewed Jan 20, 2026

View reviewed changes

apple/RNCWebView.m Show resolved Hide resolved

kewdex reviewed Jan 22, 2026

View reviewed changes

apple/RNCWebViewDecisionManager.m Outdated Show resolved Hide resolved

fix: thread safe decision manager

b6cad2e

kewdex approved these changes Jan 22, 2026

View reviewed changes

ChALkeR reviewed Jan 23, 2026

View reviewed changes

ChALkeR requested a review from Copilot January 23, 2026 10:51

Copilot started reviewing on behalf of ChALkeR January 23, 2026 10:51 View session

Copilot AI reviewed Jan 23, 2026

View reviewed changes

kbulgakov-exo marked this pull request as draft January 23, 2026 13:56

feat: support async onShouldStartLoad requests #49

Are you sure you want to change the base?

feat: support async onShouldStartLoad requests #49

Uh oh!

Conversation

kbulgakov-exo commented Jan 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The bottleneck

The solution

Testing

Uh oh!

kbulgakov-exo Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

kbulgakov-exo Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

kbulgakov-exo left a comment

Choose a reason for hiding this comment

Uh oh!

guten-exodus left a comment

Choose a reason for hiding this comment

Uh oh!

kbulgakov-exo commented Jan 19, 2026

Uh oh!

timlanahan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

guten-exodus commented Jan 20, 2026

Uh oh!

Uh oh!

kewdex left a comment

Choose a reason for hiding this comment

Uh oh!

ChALkeR left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

kbulgakov-exo commented Jan 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

kbulgakov-exo commented Jan 18, 2026 •

edited

Loading

ChALkeR left a comment •

edited

Loading