fix: separate local storage between WebView processes

[ItsChaceD]

Description

This PR implements localStorage isolation on Android API 28+ via setDataDirectorySuffix, bringing it into alignment with iOS behavior.

Due to Android system limitations, isolation can only be achieved by running the WebView in a separate :OSInAppBrowser process with a dedicated data directory suffix. This requires events to be propagated between the processes via broadcast receivers. To avoid a breaking change, we keep the class signatures the same and add a custom @RequiresEventBridgeRegistration annotation to proactively warn native library users of the new registration requirement.

Context

https://outsystemsrd.atlassian.net/browse/RMET-4918?atlOrigin=eyJpIjoiOWI4NWVmMDIwZDhhNDRmMGJhMTljMWFlNTQxYmFkZjIiLCJwIjoiaiJ9

Type of changes

• Fix (non-breaking change which fixes an issue)
• Feature (non-breaking change which adds functionality)
• Refactor (cosmetic changes)
• Breaking change (change that would cause existing functionality to not work as expected)

Platforms affected

• Android
• iOS
• JavaScript

[OS-pedrogustavobilro]

Feels confusing for this class to be Serializable, subclasses have Context and Activity which are not serializable. The serialization is only relevant for the broadcast receiver, which is only called for webview, but from what I see in the PR you don't set OSIABWebViewActivity anymore for OSIABWebViewEvent.

Can you explain why you implemented it that way?

[ItsChaceD]

You're right that Activity and Context cannot be serialized, but it is a hard requirement for it to be Parcelable / Serializable since we are sending the object between processes via an Intent.

fun broadcastEvent(context: Context, event: OSIABEvents) {
  val intent = Intent(ACTION_IAB_EVENT).apply {
      setPackage(context.packageName)
      putExtra(EXTRA_EVENT_DATA, event)
  }
  context.sendBroadcast(intent)
}

I addressed this by marking those specific fields as @Transient.

[OS-pedrogustavobilro]

We should make sure plugins (OutSystems especially) document this change, wouldn't be surprised if it ends up breaking existing Android apps in some unexpected way, due to the many different ways people use InAppBrowser.

[OS-pedrogustavobilro]

Actually, doesn't this mean that existing apps that have cookies or local storage data for a certain webpage in InAppBrowser, will no longer have that data when they update the app to this version where the data directory changes?

[ItsChaceD]

You're right, it may break existing apps that use InAppBrowser.. However, if we leave the current behavior then it may be a security flaw because we shouldn't give the browser access to the main app's data. So we have 2 options:

1. "Always Isolated". This fixes the security bug and also currently the plugin behaves differently on both platforms, which can lead to bugs in the app's logic.. so it makes Android behavior match iOS. Storage will persist as normal after the first run.

Document a warning saying something like (Thanks AI for the docs help 😅):

Warning

Breaking Change (Android): Apps upgrading to this version will lose any existing localStorage or cookies previously stored by the InAppBrowser. This is because the WebView now runs in a separate process with its own data directory. Users may need to re-authenticate with websites that relied on persisted session data.

2. "Opt-in" Isolation. Keep the current shared behavior as default, but add an isolateStorage config option. Best for backwards compatibility and existing data, but not the best security practice.

If you think there will be a bigger impact on customers with the first approach, then we could maybe do "opt-in" for this version and then have it always isolated for the next major version. Thoughts?

[OS-pedrogustavobilro]

If this raises security concerns then I don't think we want an opt-in.

We should just be careful on releasing this and informing customers, to make sure impacts are minimized. Hard to say on how many customers could be impacted, due to the myriad of ways and webpages and use cases you can have for InAppBrowser.

[OS-pedrogustavobilro]

From what I saw here, we never set activity anymore when instantiating this event. If you can confirm on your end - if so we can remove this property from the class mayhaps?

[OS-pedrogustavobilro]

Should we call finalizeBrowser here? Maybe it's harmless, but I'm thinking of a scenario where there's an exception and the IAB doesn't open to begin with, but finalizeBrowser will call onBrowserFinished under the hood, of which I'm not 100% certain if it's what we want.

Your thoughts?

[OS-pedrogustavobilro]

Shouldn't we change the order, i.e. first listen to events, and only after send the broadcast?

Sending the broadcast isn't instantaneous I guess, but because the events flow is a SharedFlow (i.e., a hot flow), it will not wait to have producers to start emitting, meaning that if theory the flow gets updated before we start collecting, the item emission will get lost. Reversing the order would make sure that does not happen.

Unless you had issues with the reverse order.