Exposing potentially-nullable NavigationHandle in new Android WebView API

[Rakina Zata Amni]

Hi content-owners,

(cc-ing people from the WebView side and also some //content owners I talked to about this earlier today)

I've been reviewing a CL for a new Android WebView API (WebViewCompat#navigate) to provide a better way to start navigations in WebView, including returning a Navigation object (see tracking bug). The Navigation object itself isn't new to this specific change, it's already exposed as part of the (also quite new) navigation callback APIs that closely resemble the current //content public APIs: onNavigationStarted/Redirected/Completed (see internal doc).

One of the more interesting additions from WebViewCompat#navigate is the fact that it will return a Navigation object created when calling NavigationController::LoadURLWithParams(), so that the callers can track the navigation it initiated. But there are some interesting edge cases:

• This navigation might not have started yet (e.g. if we trigger running beforeunload, which will introduce an async step and delay/potentially even not trigger onNavigationStarted)
• The navigation might not even get created (if we return early, e.g. if the URL is a renderer debug URL like javascript: URL, or fails certain checks) and we return null.

For the former, it is probably ok to expose a non-started Navigation object, the user of the API just needs to be aware that a navigation might not be started yet / might never start, so don't expect onNavigationStarted/Redirected/Completed calls associated with it to always happen. (This needs to be documented clearly in the API)

For the latter, I think it makes sense to make the API return null in those cases, since NavigationController::LoadURLWithParams() API has no guarantees on it not returning null (and in fact has many cases where that's possible already). However, from the WebView side, there is pushback about making this API nullable. peconn@ can explain the rationale better, but from the thread:

> If we sometimes return null, then most developers will either ignore it (which would be the same as putting logic in an onNavigationStarted that never gets called) or if the null values are rare, they'll probably not deal with them and get a NullPointerException.

> I dislike the idea that this API would return null - because we're going to have to explain to developers why and when that could happen, and if it's not something that happens now developers will assume it never will and then when //content changes how loadURL works, we're just going to pass the null through and it will break developer's apps.

The workaround proposed by WebView is to ensure that for the cases this API can encounter, we will never return null. The current cases for returning null are:

• The URL is a debug URL or renderer debug URL
• IsValidURLForNavigation returns false
• Violating fenced frames, which won't happen because the navigation is not initiated by fenced frames
• renderer_manager()->is_attaching_inner_delegate(), is for GuestViews which don't exist for Android WebView.

The argument is that the URL validity check can be done on the WebView side and we throw an exception there (instead of returning null), and the other cases don't apply to WebView, thus there are no more cases that return null. However I don't think that invariant can be guaranteed in the long run. Nothing can prevent a new web platform feature or some type of internal policy to prevent creation of some navigations, e.g. like the fenced frames violation check but it applies to main frames / cases that can happen in WebView as well. Additionally even the URL validity check needs to be maintained to not diverge between WebView and //content-internal checks. If any of those changes happen, we can't change the WebView API to be nullable after that as that will be significantly riskier than making it nullable from the start.

I want to ask the opinion from //content owners and WebView folks on how to best handle this null possibility in a way that's not going to put burden within //content to maintain the invariant of not returning null for the cases WebView cares about but is also not harmful for users of the WebView API. 

From the WebView side it seems like to make the API nullable the requirement is to enumerate / be clear on the cases that can return null, but since that is not happening right now, I wonder if it's possible to just have a generic but explicit explanation like "if the browser determines that the requested navigation cannot be initiated, the API can return null", which we generally do in web platform standardization for carveouts like this? (or make the invalid URL cases return null, although that's probably not as useful as an actual exception).

(The other alternative for the problem is exposing a "Fake Navigation" for these navigations that don't actually get created which after some more thinking is probably a bad idea, requiring inventing values to be set for the navigation properties and will need ongoing maintenance as new Navigation properties get exposed.)

Looking forward to your thoughts. Right now the plan is to unblock peconn@'s CL by landing the API as nullable to start with to unblock further work, but this API is not shipped yet in that state. If we reach a different conclusion or find a better solution for this, we can still change the API before shipping it.

[Peter Conn]

Hello,

Thanks Rakina for kicking off this conversation. To add a few more words to my position:

There are two separate parts to this question:
- Should the WebViewCompat#navigate API return something nullable?
- Should the WebViewCompat#navigate API be simple mapping onto NavigationController::LoadURLWithParams?

For the first question, I strongly feel that navigate should never return null (and throw some Exception for invalid URLs - the only situation where the WIP implementation would encounter a null for WebView). Fundamentally, the point of a WebView is to load URLs into it, and if we cannot guarantee the semantics of that it's going to cause pain for developers. Another way of thinking about this - what do we expect a developer to do if navigate returns null?

This applies not just to making the API return null, but to expanding the situations in which it could return null. If a call to navigate returns something today, and returns null tomorrow, we have broken someone's app (whether by causing a NullPointerException or by just not navigating where we previously did).

I could imagine some future security change that expands the situations in which NavigationController::LoadURLParams returned null and we would want to bring those benefits to WebView - but that would need to be an opt-in, we couldn't just change the behaviour by default.


For the second question, there would be a lot of overhead in creating some WebViewNavigationHandle and managing how that mapped to real NavigationHandles - and we still wouldn't fix the problem of "the user called navigate (with a well formed URL), they expect a navigation to occur but for some reason it doesn't".


Ultimately what we're doing here is exposing part of the semantics of NavigationController::LoadURLWithParams as part of WebView's API. I think that for the API to make sense, we need to do that, but I understand the burden that places on the content owners. To some extent though this already exists today - if //content/ made a change that stopped the existing WebView#loadUrl from working, that would break WebView and would definitely be a bug.

Thanks,

Peter C

[Charlie Reis]

Yes, thanks Rakina for raising the question, and for providing so much useful context!

Fundamentally, the point of a WebView is to load URLs into it, and if we cannot guarantee the semantics of that it's going to cause pain for developers. Another way of thinking about this - what do we expect a developer to do if navigate returns null?

... wouldn't fix the problem of  "the user called navigate (with a well formed URL), they expect a navigation to occur but for some reason it doesn't". 

I think these statements vastly oversimplify the concept of navigation, and they would cause fundamental problems if we encoded them into an API that can't meet those expectations.

There cannot be a guarantee that a request to navigate to a URL will succeed.  Developers can't ever rely on that expectation, and they already have to handle failure cases like the server being down (or throttles canceling it, etc, etc), though in most of those cases the NavigationHandle does get returned but later fails.  There's still a whole class of cases that Rakina mentions where the navigation doesn't get that far and we return null from LoadURLWithParams, and there is no expectation that this class won't expand over time.  In fact, I'm reviewing a CL right now that expands it, by allowing enterprises to block additional debug URLs.

Importantly, it is our responsibility to only expose APIs whose invariants can be met, or else we end up stuck in an intractable state where problems occur and we can't fix them.  We've already seen this with other Android WebView APIs, and we face long-term hacks within //content because apps do things like requesting re-entrant navigations (which can't be supported).  This means that we should be very careful in exposing new navigation APIs, and discuss what can and can't be supported.  I'm glad this is coming up while the API is still malleable enough to improve.

I do agree that we should aim to expose APIs that are friendly to developers, and that means both (1) being ergonomic and having consistent ways to handle failures, and (2) not hiding issues that actually occur in practice.  The fact that LoadURLWithParams has different ways of exposing different types of failures is admittedly not perfect.  I'm open to discussions about how we expose these types of failures to developers (e.g., null from LoadURLWithParams always leads to an exception, or even having some type of failed NavigationHandle returned in those cases), but we can't expose an API that assumes the failures won't happen or even that they won't change over time.

In practice, I suspect Rakina might be right that returning some kind of failed NavigationHandle instead of null might be difficult, though I'm not opposed to discussing what the tradeoffs of it are.  However, would simply throwing an exception in all null cases be easier?

Thanks,

Charlie

[Torne (Richard Coles)]

On Thu, 15 May 2025 at 14:32, Charlie Reis <cr...@chromium.org> wrote:

Yes, thanks Rakina for raising the question, and for providing so much useful context!

Fundamentally, the point of a WebView is to load URLs into it, and if we cannot guarantee the semantics of that it's going to cause pain for developers. Another way of thinking about this - what do we expect a developer to do if navigate returns null?

... wouldn't fix the problem of  "the user called navigate (with a well formed URL), they expect a navigation to occur but for some reason it doesn't". 

I think these statements vastly oversimplify the concept of navigation, and they would cause fundamental problems if we encoded them into an API that can't meet those expectations.

There cannot be a guarantee that a request to navigate to a URL will succeed.  Developers can't ever rely on that expectation, and they already have to handle failure cases like the server being down (or throttles canceling it, etc, etc), though in most of those cases the NavigationHandle does get returned but later fails.

Right, when there's a NavigationHandle returned which later fails this is fine - the API we are exposing can communicate navigation failures and also deals with navigations that succeed but do not commit. The navigation handle tells us a bunch about what happened, and many cases give us net::Error codes or other specific information about the failure that we can communicate to the app.

 

  There's still a whole class of cases that Rakina mentions where the navigation doesn't get that far and we return null from LoadURLWithParams, and there is no expectation that this class won't expand over time.  In fact, I'm reviewing a CL right now that expands it, by allowing enterprises to block additional debug URLs.

I wouldn't really have a problem with blocking all debug URLs in this API if that solves things for us. The case that seems important here is when we have a normal web URL that we expect to start a navigation, but LoadURLWithParams returns null for some reason we can't predict in the WebView implementation code.

 

Importantly, it is our responsibility to only expose APIs whose invariants can be met, or else we end up stuck in an intractable state where problems occur and we can't fix them.  We've already seen this with other Android WebView APIs, and we face long-term hacks within //content because apps do things like requesting re-entrant navigations (which can't be supported).  This means that we should be very careful in exposing new navigation APIs, and discuss what can and can't be supported.  I'm glad this is coming up while the API is still malleable enough to improve.

I do agree that we should aim to expose APIs that are friendly to developers, and that means both (1) being ergonomic and having consistent ways to handle failures, and (2) not hiding issues that actually occur in practice.  The fact that LoadURLWithParams has different ways of exposing different types of failures is admittedly not perfect.  I'm open to discussions about how we expose these types of failures to developers (e.g., null from LoadURLWithParams always leads to an exception, or even having some type of failed NavigationHandle returned in those cases), but we can't expose an API that assumes the failures won't happen or even that they won't change over time.

It's okay for there to be failures, but it's extremely hard to use an API that can fail without giving any reason. How we communicate that reason to WebView developers is an API design problem for us - a special navigation handle, an exception, we can figure that out ourselves.

It seems like we only have three general options here:

1. Enumerate all the reasons that LoadURLWithParams might return null and handle them all explicitly in our implementation, and //content folks promise not to intentionally create any new reasons without first having a compatibility conversation with WebView about it.

2. Change the interface of LoadURLWithParams so that if it doesn't return a navigation handle it instead returns some kind of more-specific error code/message which WebView can communicate to the developer in whatever way makes sense for us.

3. Leave //content alone and just give the developer a completely generic "oops, guess this navigation didn't happen" error if LoadURLWithParams returns null.

 

I don't think 3 leaves us with a reasonable public API: either the cases where this happens turn out to be common enough that apps *do* have to handle it but can't handle it in a particularly useful way, or the cases where it happens end up being really rare in which case apps just *won't* handle it and will crash or do the wrong thing or whatever.

But, doing anything else means we need *some* level of commitment from the //content API about this.

[Rakina Zata Amni]

On Fri, May 16, 2025 at 2:51 AM Torne (Richard Coles) <to...@chromium.org> wrote:

On Thu, 15 May 2025 at 14:32, Charlie Reis <cr...@chromium.org> wrote:

Yes, thanks Rakina for raising the question, and for providing so much useful context!

Fundamentally, the point of a WebView is to load URLs into it, and if we cannot guarantee the semantics of that it's going to cause pain for developers. Another way of thinking about this - what do we expect a developer to do if navigate returns null?

... wouldn't fix the problem of  "the user called navigate (with a well formed URL), they expect a navigation to occur but for some reason it doesn't". 

I think these statements vastly oversimplify the concept of navigation, and they would cause fundamental problems if we encoded them into an API that can't meet those expectations.

There cannot be a guarantee that a request to navigate to a URL will succeed.  Developers can't ever rely on that expectation, and they already have to handle failure cases like the server being down (or throttles canceling it, etc, etc), though in most of those cases the NavigationHandle does get returned but later fails.

Right, when there's a NavigationHandle returned which later fails this is fine - the API we are exposing can communicate navigation failures and also deals with navigations that succeed but do not commit. The navigation handle tells us a bunch about what happened, and many cases give us net::Error codes or other specific information about the failure that we can communicate to the app.

 

  There's still a whole class of cases that Rakina mentions where the navigation doesn't get that far and we return null from LoadURLWithParams, and there is no expectation that this class won't expand over time.  In fact, I'm reviewing a CL right now that expands it, by allowing enterprises to block additional debug URLs.

I wouldn't really have a problem with blocking all debug URLs in this API if that solves things for us. The case that seems important here is when we have a normal web URL that we expect to start a navigation, but LoadURLWithParams returns null for some reason we can't predict in the WebView implementation code.

 

Importantly, it is our responsibility to only expose APIs whose invariants can be met, or else we end up stuck in an intractable state where problems occur and we can't fix them.  We've already seen this with other Android WebView APIs, and we face long-term hacks within //content because apps do things like requesting re-entrant navigations (which can't be supported).  This means that we should be very careful in exposing new navigation APIs, and discuss what can and can't be supported.  I'm glad this is coming up while the API is still malleable enough to improve.

I do agree that we should aim to expose APIs that are friendly to developers, and that means both (1) being ergonomic and having consistent ways to handle failures, and (2) not hiding issues that actually occur in practice.  The fact that LoadURLWithParams has different ways of exposing different types of failures is admittedly not perfect.  I'm open to discussions about how we expose these types of failures to developers (e.g., null from LoadURLWithParams always leads to an exception, or even having some type of failed NavigationHandle returned in those cases), but we can't expose an API that assumes the failures won't happen or even that they won't change over time.

It's okay for there to be failures, but it's extremely hard to use an API that can fail without giving any reason. How we communicate that reason to WebView developers is an API design problem for us - a special navigation handle, an exception, we can figure that out ourselves.

It seems like we only have three general options here:

1. Enumerate all the reasons that LoadURLWithParams might return null and handle them all explicitly in our implementation, and //content folks promise not to intentionally create any new reasons without first having a compatibility conversation with WebView about it.

2. Change the interface of LoadURLWithParams so that if it doesn't return a navigation handle it instead returns some kind of more-specific error code/message which WebView can communicate to the developer in whatever way makes sense for us.

This might be workable. Do you think making LoadURLWithParams return a string reason if it's nullptr is enough for WebView, and that can be surfaced as an exception or something from the WebView API and that's easier to handle by the users? And there's no need to have the reasons enumerated (or at least easy to add new / arbitrary reasons after shipping)?

[Torne (Richard Coles)]

On Fri, 16 May 2025 at 11:05, Rakina Zata Amni <rak...@chromium.org> wrote:

On Fri, May 16, 2025 at 2:51 AM Torne (Richard Coles) <to...@chromium.org> wrote:

On Thu, 15 May 2025 at 14:32, Charlie Reis <cr...@chromium.org> wrote:

Yes, thanks Rakina for raising the question, and for providing so much useful context!

Fundamentally, the point of a WebView is to load URLs into it, and if we cannot guarantee the semantics of that it's going to cause pain for developers. Another way of thinking about this - what do we expect a developer to do if navigate returns null?

... wouldn't fix the problem of  "the user called navigate (with a well formed URL), they expect a navigation to occur but for some reason it doesn't". 

I think these statements vastly oversimplify the concept of navigation, and they would cause fundamental problems if we encoded them into an API that can't meet those expectations.

There cannot be a guarantee that a request to navigate to a URL will succeed.  Developers can't ever rely on that expectation, and they already have to handle failure cases like the server being down (or throttles canceling it, etc, etc), though in most of those cases the NavigationHandle does get returned but later fails.

Right, when there's a NavigationHandle returned which later fails this is fine - the API we are exposing can communicate navigation failures and also deals with navigations that succeed but do not commit. The navigation handle tells us a bunch about what happened, and many cases give us net::Error codes or other specific information about the failure that we can communicate to the app.

 

  There's still a whole class of cases that Rakina mentions where the navigation doesn't get that far and we return null from LoadURLWithParams, and there is no expectation that this class won't expand over time.  In fact, I'm reviewing a CL right now that expands it, by allowing enterprises to block additional debug URLs.

I wouldn't really have a problem with blocking all debug URLs in this API if that solves things for us. The case that seems important here is when we have a normal web URL that we expect to start a navigation, but LoadURLWithParams returns null for some reason we can't predict in the WebView implementation code.

 

Importantly, it is our responsibility to only expose APIs whose invariants can be met, or else we end up stuck in an intractable state where problems occur and we can't fix them.  We've already seen this with other Android WebView APIs, and we face long-term hacks within //content because apps do things like requesting re-entrant navigations (which can't be supported).  This means that we should be very careful in exposing new navigation APIs, and discuss what can and can't be supported.  I'm glad this is coming up while the API is still malleable enough to improve.

I do agree that we should aim to expose APIs that are friendly to developers, and that means both (1) being ergonomic and having consistent ways to handle failures, and (2) not hiding issues that actually occur in practice.  The fact that LoadURLWithParams has different ways of exposing different types of failures is admittedly not perfect.  I'm open to discussions about how we expose these types of failures to developers (e.g., null from LoadURLWithParams always leads to an exception, or even having some type of failed NavigationHandle returned in those cases), but we can't expose an API that assumes the failures won't happen or even that they won't change over time.

It's okay for there to be failures, but it's extremely hard to use an API that can fail without giving any reason. How we communicate that reason to WebView developers is an API design problem for us - a special navigation handle, an exception, we can figure that out ourselves.

It seems like we only have three general options here:

1. Enumerate all the reasons that LoadURLWithParams might return null and handle them all explicitly in our implementation, and //content folks promise not to intentionally create any new reasons without first having a compatibility conversation with WebView about it.

2. Change the interface of LoadURLWithParams so that if it doesn't return a navigation handle it instead returns some kind of more-specific error code/message which WebView can communicate to the developer in whatever way makes sense for us.

This might be workable. Do you think making LoadURLWithParams return a string reason if it's nullptr is enough for WebView, and that can be surfaced as an exception or something from the WebView API and that's easier to handle by the users? And there's no need to have the reasons enumerated (or at least easy to add new / arbitrary reasons after shipping)?

If we don't expect this to actually be hit by an app that's doing normal, reasonable things and this is just to handle unforeseen edge cases (i.e. we carve out the known cases that we don't want to allow anyway like loading javascript: URLs and handle them ourselves), then a string reason for any remaining unusual cases seems fine as long as it's meaningful enough for the app developer to understand when they see it, or at least specific enough for *us* to understand when they file a bug about it.

We can still carve out known cases that we don't want to allow like loading javascript: URLs and check for those ourselves without even calling LoadURLWithParams, so that we can make more specific decisions about how to raise those errors.

[Peter Conn]

It's worth pulling apart two questions here:

1. Will a certain call to LoadURLWithParams trigger a navigation?
2. If a call to LoadURLWithParams triggers a navigation, will it return a NavigationHandle?

IIUC the //content/ owners would like to be able to change the answer to #1 in the future. Given that the existing WebView#loadUrl is also based on LoadUrlWithParams, if the change touches a situation possible in WebView, this would be a breaking Android change (touching every version of Android that WebView supports).

I think ultimately it comes down to what kind of changes you think you'll be making in the future. If they don't affect WebView apps doing "normal, reasonable things" then this is a good solution. If they could, then we need to figure out how not to cause an Android AppCompat bug when that happens.

[Rakina Zata Amni]

(Sorry for the delay in replying, was out unexpectedly)

There's two types of things I can imagine potentially being added in the future:

- A new web platform API is introduced that has certain rules on when a navigation can be triggered or not when that new API is involved.

- A new internal policy is introduced to prevent existing ways to navigate, e.g. due to newly found security concerns.

A generic WebView app that uses navigate() to go to a user-supplied URL, for example, can hypothetically go to sites using those new APIs, or be in a situation that falls under the new navigation-blocking policy. Would having those return null with "X API / policy blocks navigation from being navigated" as a reason be enough? (not sure if it's normal/reasonable for such an app to not handle the null case already)

[Torne (Richard Coles)]

On Tue, 20 May 2025 at 06:53, Rakina Zata Amni <rak...@chromium.org> wrote:

(Sorry for the delay in replying, was out unexpectedly)

There's two types of things I can imagine potentially being added in the future:

- A new web platform API is introduced that has certain rules on when a navigation can be triggered or not when that new API is involved.

- A new internal policy is introduced to prevent existing ways to navigate, e.g. due to newly found security concerns.

A generic WebView app that uses navigate() to go to a user-supplied URL, for example, can hypothetically go to sites using those new APIs, or be in a situation that falls under the new navigation-blocking policy. Would having those return null with "X API / policy blocks navigation from being navigated" as a reason be enough? (not sure if it's normal/reasonable for such an app to not handle the null case already)

That seems reasonable to me, though ideally we'd still have a discussion about the compatibility impact of these things before shipping them.

[Peter Conn]

IIUC, the API shape we're aligning towards is:

• NavigationController::LoadUrl returning something like Result<NavigationHandle, ErrorString>.
  
• WebViewCompat#navigate returning a @NonNull Navigation (based on that NavigationHandle) or throwing some Exception.

Throwing the exception is going to be more informative to the developer than just returning null, and practically they're both going to be as intrusive.

Yeah, I think that's reasonable, provided that the WebView team is involved in the discussion before shipping.

[Rakina Zata Amni]

On Tue, May 20, 2025 at 11:40 PM Peter Conn <pec...@google.com> wrote:

IIUC, the API shape we're aligning towards is:

• NavigationController::LoadUrl returning something like Result<NavigationHandle, ErrorString>.
  
• WebViewCompat#navigate returning a @NonNull Navigation (based on that NavigationHandle) or throwing some Exception.

Throwing the exception is going to be more informative to the developer than just returning null, and practically they're both going to be as intrusive.

Yeah, I think that's reasonable, provided that the WebView team is involved in the discussion before shipping.

Some of the content-owners (the ones cc-ed in the thread) discussed this internally and the shape seems reasonable. Can you clarify what kind of involvement you think should happen? What we imagine this to look like is //content expose base::expected<NavigationHandle, ErrorReason>  as the return value. The ErrorReason will be an enum that WebView handles all the cases of, and if we add a new reason that will automatically require an update on the WebView handling code, at which point WebView reviewers can decide what to do (piggyback an existing exception, create a new one, etc). (With this I think WebView can also avoid having to maintain URL checks on the WebView side, and just depend on the enum reason to tell you that the URL is invalid, etc)

Also, a somewhat orthogonal question (but related to this new API) came from creis@, who recently looked into re-entrancy problems encountered by WebView apps when removing the beforeunload legacy PostTask (here, which was linked a bit earlier in this thread as well). The main problem there is we currently need to do a "fake" asynchronous step between creating a NavigationHandle and triggering some throttles/callbacks firing in the WebView code, because the WebView code can trigger navigations synchronously, deleting the NavigationHandle that triggered those callbacks. This navigation re-entrancy is not allowed to happen within //content code, thus a fake PostTask is used to avoid that situation. While the existing ways to create navigations will still be a problem, we're wondering if it makes sense to make this new API asynchronous / returns something like a Promise<NavigationHandle>? That will make it not depend on the creation being synchronous / avoid re-entrancy entirely, and if in the far future we somehow deprecate the old APIs, then maybe we can avoid the fake PostTask (very wishful thinking). (creis@, CMIIW!)

Also, in general, I wonder if having more asynchronous / less coupling on the timing of things for navigation-related APIs make sense (e.g. maybe make onNavigationStarted/Redirected/Completed not synchronously called from the //content counterpart as well)? The downside is they're not going to be reliably timely, so maybe we can make exception for things that are important to be fired exactly on the time things change, e.g. onNavigationCompleted (which is fired when we change the document / url / etc). (This is pretty far away from the original topic of this thread though, so I'm happy to start another thread)

[Peter Conn]

>  Can you clarify what kind of involvement you think should happen?

For any navigation change that could affect existing WebView apps, loop the WebView team in at the design phase to assess the impact and see if there are ways to mitigate it. And, in the unlikely case we can't reach a good conclusion, give us an opportunity to escalate.

I'm not too familiar with the WebView navigation re-entrancy problem you mention (maybe Torne is?). If this is a case of "we kept this around because that's how WebView#loadUrl used to work", then maybe with WebViewCompat#navigate we have the opportunity to get rid of that behaviour.

I'd like to avoid exposing Promises to the developer (how do we explain when they're fulfilled and the current API is more callback based). How about this (this is from an Android developer's PoV):

// Introduce a new PendingNavigation class.

PendingNavigation nav = WebViewCompat.navigate(URL, ...);

// Modification to the existing WebNavigationClient.
WebNavigationClient navClient = new WebNavigationClient() {

  // This is where a PendingNavigation turns into a Navigation.
  void onNavigationStarted(PendingNavigation pendingNavigation, Navigation navigation) {
    if (nav == pendingNavigation) {
      mMyNavigation = navigation;
    }
  }
};

That way we can still keep track of which Navigation results from which navigate call, we're not mixing up Promises with the existing callback architecture and we don't have to add a new concept for when a Navigation object starts being usable. Just using a straight Navigation object would still be simpler though.

[Torne (Richard Coles)]

On Wed, 21 May 2025 at 10:13, Peter Conn <pec...@google.com> wrote:

>  Can you clarify what kind of involvement you think should happen?

For any navigation change that could affect existing WebView apps, loop the WebView team in at the design phase to assess the impact and see if there are ways to mitigate it. And, in the unlikely case we can't reach a good conclusion, give us an opportunity to escalate.

Yeah, ideally we'd learn about these things before the CL implementing it actually comes along so we can discuss it, but if content is going to use an enum then that will at least mean that there's code on the WebView side to be updated to match against the new value as a final place to catch the change.

 

I'm not too familiar with the WebView navigation re-entrancy problem you mention (maybe Torne is?). If this is a case of "we kept this around because that's how WebView#loadUrl used to work", then maybe with WebViewCompat#navigate we have the opportunity to get rid of that behaviour.

IIRC the problem is that shouldOverrideUrlLoading is called from a throttle while the navigation is still in progress and lots of apps call loadUrl inside their callback implementation, resulting in reentrancy into the navigation stack.

Apps do this for lots of reasons and while I would *like* if they didn't, I'm not sure that's something we can reasonably enforce without a lot of movement from the ecosystem, and I'm not sure we would want to disallow it just for navigate() while still allowing loadUrl() as that seems unhelpful. Some of the reasons:

1. To reload the same URL but with extra headers specified via the overload of loadUrl that takes extra headers. This is a kind of crappy workaround for us not having a way to actually modify headers on outgoing requests in a more useful way, and apps that want to do this may well want to use navigate() for it in future, as the extra header semantics are going to be clearer.

2. To load some other URL instead - often a local file or data: URI with some kind of error page when the app is trying to block/allowlist navigations to communicate what happened to the user. This is also a case where using navigate() may well be desirable as they probably want to *replace* the current history entry so that going back takes you to the previous successful page instead of just trying to load the disallowed URL again (though maybe this is already what happens with loadUrl, I am not sure how this is actually handled in the reentrant case right now?).

3. To load the exact same URL with no changes whatsoever. Somehow a lot of app developers and a lot of example code on the internet got the idea that the "usual" implementation of shouldOverrideUrlLoading should be "loadUrl(url); return true;" which results in *every* navigation being cancelled and restarted, and also turns them into browser-initiated navigations with the accompanying security-relevant effects. I don't think anyone is actually doing this *because* they want to convert things into browser-initiated navigations, this is just an extremely widely copied antipattern. This is the case that I've considered *specifically* trying to block before, but if we aren't going to be able to completely get rid of the reentrancy then it probably isn't worth the churn it will cause for the ecosystem :(

 

I'd like to avoid exposing Promises to the developer (how do we explain when they're fulfilled and the current API is more callback based). How about this (this is from an Android developer's PoV):

// Introduce a new PendingNavigation class.

PendingNavigation nav = WebViewCompat.navigate(URL, ...);

// Modification to the existing WebNavigationClient.
WebNavigationClient navClient = new WebNavigationClient() {

  // This is where a PendingNavigation turns into a Navigation.
  void onNavigationStarted(PendingNavigation pendingNavigation, Navigation navigation) {
    if (nav == pendingNavigation) {
      mMyNavigation = navigation;
    }
  }
};

That way we can still keep track of which Navigation results from which navigate call, we're not mixing up Promises with the existing callback architecture and we don't have to add a new concept for when a Navigation object starts being usable. Just using a straight Navigation object would still be simpler though.

It seems like PendingNavigation here is just a less standard Future<Navigation>? Not sure this is really any easier. The latest AndroidX API guidelines discourage returning futures in favor of just having callback parameters though IIRC (for some kotlin coroutine interop reason?)

If we actually wanted to get rid of the reentrancy here I suspect the way that handles the developer use cases we know about best would be to just have a new version of shouldOverrideUrlLoading that has a richer return value - instead of just true/false to cancel the navigation or not, let them also return the navigation they want to *replace* it with? Would that be easier for //content?

Though.. we would still have to support the current behavior for a very long time for compatibility, so it may not actually help in practice.

 

On Wed, 21 May 2025 at 13:30, Rakina Zata Amni <rak...@chromium.org> wrote:

On Tue, May 20, 2025 at 11:40 PM Peter Conn <pec...@google.com> wrote:

IIUC, the API shape we're aligning towards is:

• NavigationController::LoadUrl returning something like Result<NavigationHandle, ErrorString>.
  
• WebViewCompat#navigate returning a @NonNull Navigation (based on that NavigationHandle) or throwing some Exception.

Throwing the exception is going to be more informative to the developer than just returning null, and practically they're both going to be as intrusive.

Yeah, I think that's reasonable, provided that the WebView team is involved in the discussion before shipping.

Some of the content-owners (the ones cc-ed in the thread) discussed this internally and the shape seems reasonable. Can you clarify what kind of involvement you think should happen? What we imagine this to look like is //content expose base::expected<NavigationHandle, ErrorReason>  as the return value. The ErrorReason will be an enum that WebView handles all the cases of, and if we add a new reason that will automatically require an update on the WebView handling code, at which point WebView reviewers can decide what to do (piggyback an existing exception, create a new one, etc). (With this I think WebView can also avoid having to maintain URL checks on the WebView side, and just depend on the enum reason to tell you that the URL is invalid, etc)

Also, a somewhat orthogonal question (but related to this new API) came from creis@, who recently looked into re-entrancy problems encountered by WebView apps when removing the beforeunload legacy PostTask (here, which was linked a bit earlier in this thread as well). The main problem there is we currently need to do a "fake" asynchronous step between creating a NavigationHandle and triggering some throttles/callbacks firing in the WebView code, because the WebView code can trigger navigations synchronously, deleting the NavigationHandle that triggered those callbacks. This navigation re-entrancy is not allowed to happen within //content code, thus a fake PostTask is used to avoid that situation. While the existing ways to create navigations will still be a problem, we're wondering if it makes sense to make this new API asynchronous / returns something like a Promise<NavigationHandle>? That will make it not depend on the creation being synchronous / avoid re-entrancy entirely, and if in the far future we somehow deprecate the old APIs, then maybe we can avoid the fake PostTask (very wishful thinking). (creis@, CMIIW!)

Also, in general, I wonder if having more asynchronous / less coupling on the timing of things for navigation-related APIs make sense (e.g. maybe make onNavigationStarted/Redirected/Completed not synchronously called from the //content counterpart as well)? The downside is they're not going to be reliably timely, so maybe we can make exception for things that are important to be fired exactly on the time things change, e.g. onNavigationCompleted (which is fired when we change the document / url / etc). (This is pretty far away from the original topic of this thread though, so I'm happy to start another thread)

The original WebView callbacks like onPageStarted aren't *really* timely either - in almost all cases I'm aware of they are posted to the UI thread as new tasks for historical reasons and because it makes for better error handling/reporting behavior when the app's callback implementation throws an exception. We haven't consistently done this for newer APIs unfortunately, and I'm not sure what we did here for onNavigationStarted etc either, but, yeah, apps really shouldn't be relying on these things being timely - only shouldOverrideUrlLoading is "really" blocking by design, because we have to get the return value to know whether to cancel the navigation.

There isn't, as far as I know, a guaranteed way to update a displayed URL in sync with the page rendering either with the existing callbacks *or* the new ones; this is a really hard problem to perfectly solve and we haven't really tried very hard. Currently people tend to do it in a way that either misses updates for same-page navigations completely which is in theory just a usability problem and not a security issue, or in ways that update things too early/often and can likely be used for URL spoofing in sufficiently contrived cases.

[Peter Conn]

Hello,

I'm going to be OOO from EoD today until 28th of June, so I won't be responding for a while :-).

A summary (of the whole topic, not just this thread, as much for me as everyone else):

- In Q3, androidx.webkit plans to offer an experimental WebViewCompat#navigate API - as an experimental API it will offer no compatibility/stability guarantees.

- The decisions in this thread won't block the experimental API, but will block progressing it to stable.

- WebView's preferred approach would be that WebViewCompat#navigate either returns a non-null object or throws an exception - //content/ owners seem open to providing the failure reason from NavigationController::LoadURLWithParams.

Topics still to be resolved:

- We'd prefer to return a Navigation object, but some Future/PendingNavigation could be considered instead.

- A commitment that "For any navigation change that could affect existing WebView apps, [//content/ owners will] loop the WebView team in at the design phase to assess the impact and see if there are ways to mitigate it. And, in the unlikely case we can't reach a good conclusion, give us an opportunity to escalate."

More tangential points:

- Unfortunately, this API isn't an opportunity to fix the navigation re-entrancy problem. There's potential for a better shouldOverrideUrlLoading that could do so, but the old version would need to be supported.

- There's also a wider discussion about the timing requirements on Navigation APIs.

Thanks,

Peter C

[Rakina Zata Amni]

Thanks Peter for summarizing and have a good OOO,

I'll let Charlie comment on the re-entrancy problems since he's been working more closely on that. I'll look at the async step addition for the navigation APIs separately.

Looping in WebView people will definitely happen at least when we get the CL, but I want to set the right expectations on potentially looping in earlier than that: the new null case can be caused by any web platform API or policy, which are likely not designed by the //content owners themselves. The first time //content owners hear about those things will likely be when we get added as reviewers for the CL, so that's the earliest we can commit to (but the actual implementors themselves might reach out earlier when they're prototyping). But we can try to avoid having a new null case as much as possible, e.g. doing the cancellations on later stages.

(Maybe with the Future/PendingNavigation approach this will be easier since there's no exceptions to be thrown/no expectations that a PendingNavigation will have a corresponding onNavigationStarted.. but we can discuss that after you're back)