[connect] Improve WebView state handling #10268

lng-stripe · 2025-02-26T03:35:04Z

Summary

Sorry for the big PR. It's a big refactoring that took quite a bit of design-by-doing to get right.

tl;dr:

Issue: WebViews are reloading on configuration changes.
Fix: Cache the WebView in a ViewModel for re-use across configuration changes. Allow integrators to provide a cache key to support multiple instances of a component in the same activity/screen.

Quoting my kdocs for StripeConnectWebViewContainerViewModel:

/**
 * ViewModel for [StripeConnectWebViewContainer]. Importantly, it also provides a cached WebView instance.
 * Creating and caching a View in a ViewModel is generally bad practice, but it's necessary in our case to provide
 * a good experience for both SDK integrators and end users.
 *
 * The issue is that the typical View state restoration mechanism does essentially nothing for WebViews. Any time
 * a WebView is recreated, it loses its state and reloads the page. This would be a bad UX especially for components
 * with forms.
 *
 * As of this writing, the official guidance is to "avoid activity recreation by specifying the configuration changes
 * handled by your app (rather than by the system)". However, that would likely be infeasible for users wanting to
 * integrate this SDK into their apps.
 *
 * So, the approach here is to:
 *  1. Create the WebView using the application context in the ViewModel.
 *  2. Cache the WebView in the ViewModel, which is retained across configuration changes.
 *  3. Directly handle WebView interactions in the ViewModel itself (as opposed to the WebView's containing view),
 *     otherwise WebView events may be dropped during View recreation.
 *  4. Whenever an activity [Context] is needed, find it within the view tree hierarchy.
 *
 * @see https://developer.android.com/develop/ui/compose/quick-guides/content/manage-webview-state
 */

What	Before	After
UX	WebView is always reloaded on configuration changes. User may lose their work during Onboarding, for example.	WebView state is preserved perfectly across any number of configuration changes.
WebView creation	Plain `WebView` created and configured by the container view via `StripeConnectWebViewContainerImpl`	New self-configured `StripeConnectWebView` is created by `StripeConnectWebViewContainerViewModel`.
WebView caching	None	Cached in `StripeConnectWebViewContainerViewModel`, leveraging platform persistence mechanisms
WebView interaction logic	`StripeConnectWebViewContainerController`, which did not survive configuration changes	`StripeConnectWebViewContainerViewModel`, which stays alive during configuration change to handle WebView events, e.g. loading state changes
Public API	N/A	Minimal addition of an optional cache key parameter to component creation methods

See inline comments for more detailed implementation changes.

Motivation

Part of https://jira.corp.stripe.com/browse/CAX-3737

Testing

Added tests
Modified tests
Manually verified

Demos

Before

webview-before.mov

After

webview-after.mov

After (smoke testing)

webview-after-regression-test.mov

After (XML layout support)

webview-after-xml.mov

github-actions · 2025-02-26T03:40:51Z

Diffuse output:

OLD: identity-example-release-base.apk (signature: V1, V2)
NEW: identity-example-release-pr.apk (signature: V1, V2)

          │          compressed          │         uncompressed         
          ├───────────┬───────────┬──────┼───────────┬───────────┬──────
 APK      │ old       │ new       │ diff │ old       │ new       │ diff 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
      dex │     2 MiB │     2 MiB │  0 B │   4.1 MiB │   4.1 MiB │  0 B 
     arsc │     1 MiB │     1 MiB │  0 B │     1 MiB │     1 MiB │  0 B 
 manifest │   2.3 KiB │   2.3 KiB │  0 B │     8 KiB │     8 KiB │  0 B 
      res │ 302.6 KiB │ 302.6 KiB │  0 B │ 456.7 KiB │ 456.7 KiB │  0 B 
   native │   6.2 MiB │   6.2 MiB │  0 B │  15.8 MiB │  15.8 MiB │  0 B 
    asset │   7.3 KiB │   7.3 KiB │  0 B │   7.1 KiB │   7.1 KiB │  0 B 
    other │  95.1 KiB │  95.1 KiB │ -1 B │ 182.2 KiB │ 182.2 KiB │  0 B 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
    total │   9.6 MiB │   9.6 MiB │ -1 B │  21.6 MiB │  21.6 MiB │  0 B 

 DEX     │ old   │ new   │ diff      
─────────┼───────┼───────┼───────────
   files │     1 │     1 │ 0         
 strings │ 20114 │ 20114 │ 0 (+0 -0) 
   types │  6221 │  6221 │ 0 (+0 -0) 
 classes │  5015 │  5015 │ 0 (+0 -0) 
 methods │ 29998 │ 29998 │ 0 (+0 -0) 
  fields │ 17347 │ 17347 │ 0 (+0 -0) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  164 │  164 │  0   
 entries │ 3643 │ 3643 │  0

APK

   compressed    │   uncompressed   │                                           
──────────┬──────┼───────────┬──────┤                                           
 size     │ diff │ size      │ diff │ path                                      
──────────┼──────┼───────────┼──────┼───────────────────────────────────────────
  1.2 KiB │ -2 B │   1.2 KiB │  0 B │ ∆ META-INF/CERT.RSA                       
    272 B │ +1 B │     120 B │  0 B │ ∆ META-INF/version-control-info.textproto 
 28.9 KiB │ -1 B │    64 KiB │  0 B │ ∆ META-INF/CERT.SF                        
 25.6 KiB │ +1 B │  63.9 KiB │  0 B │ ∆ META-INF/MANIFEST.MF                    
──────────┼──────┼───────────┼──────┼───────────────────────────────────────────
   56 KiB │ -1 B │ 129.2 KiB │  0 B │ (total)

lng-stripe · 2025-02-26T16:12:14Z

connect/api/connect.api

@@ -284,14 +284,6 @@ public final class com/stripe/android/connect/appearance/fonts/CustomFontSource$
 	public final fun serializer ()Lkotlinx/serialization/KSerializer;
 }

-public final class com/stripe/android/connect/databinding/StripeConnectWebviewBinding : androidx/viewbinding/ViewBinding {


👀 This XML layout was used by StripeConnectWebViewContainer, but now it constructs the view programmatically now that it's getting the WebView from the ViewModel.

Do you know if this was part included in the API review? If so, I think you need to amend the API review and have someone re-approve it

This was only used internally and was not meant to be exposed in the public API.

lng-stripe · 2025-02-26T16:14:01Z

connect/src/main/java/com/stripe/android/connect/AccountOnboardingView.kt

-internal object AccountOnboardingListenerDelegate : ComponentListenerDelegate<AccountOnboardingListener> {
-    override fun AccountOnboardingListener.delegate(message: SetterFunctionCalledMessage) {
+internal object AccountOnboardingListenerDelegate : ComponentListenerDelegate<AccountOnboardingListener>() {
+    override fun delegate(listener: AccountOnboardingListener, message: SetterFunctionCalledMessage) {


👀 Minor refactoring to be less fancy, more approachable.

lng-stripe · 2025-02-26T16:14:27Z

connect/src/main/java/com/stripe/android/connect/EmbeddedComponentManager.kt

-            "You must call EmbeddedComponentManager.onActivityCreate in your Activity.onCreate function"
-        }
-
+        checkContextDuringCreate(context)


👀 Extracted helper function.

lng-stripe · 2025-02-26T16:16:04Z

connect/src/main/java/com/stripe/android/connect/EmbeddedComponentManager.kt

+    internal suspend fun requestCameraPermission(activity: Activity): Boolean? {
+        if (checkSelfPermission(activity, Manifest.permission.CAMERA) == PackageManager.PERMISSION_GRANTED) {


👀 Note the change from Context to Activity here and in other places, since using application context would be a runtime error in many cases.

lng-stripe · 2025-02-26T16:22:32Z

connect/src/main/java/com/stripe/android/connect/StripeEmbeddedComponentListener.kt

 @OptIn(PrivateBetaConnectSDK::class)
-internal fun interface ComponentListenerDelegate<Listener : StripeEmbeddedComponentListener> {
-    fun Listener.delegate(message: SetterFunctionCalledMessage)
+internal open class ComponentListenerDelegate<Listener : StripeEmbeddedComponentListener> {


👀 Refactored to be an open class in order to move handling of common component events here. Before, the delegation to the listener was done in the Controller, which is no longer feasible. The Controller has been replaced by a ViewModel, which may not keep a reference to the listener otherwise we risk leaking the Activity during a config change.

lng-stripe · 2025-02-26T16:49:51Z

...t/src/main/java/com/stripe/android/connect/webview/StripeConnectWebViewContainerViewModel.kt

+    val eventFlow: Flow<ComponentEvent> get() = _eventFlow.asSharedFlow()
+
+    @VisibleForTesting
+    internal val delegate = StripeConnectWebViewDelegate()


👀 I extracted WebView -> VM interactions to this inner class for better code organization and encapsulation, e.g. the Container view should not be able to call onPageStarted on the VM.

lng-stripe · 2025-02-26T16:50:28Z

...t/src/main/java/com/stripe/android/connect/webview/StripeConnectWebViewContainerViewModel.kt

+    @SuppressLint("StaticFieldLeak") // Should be safe because we're using application.
+    val webView: StripeConnectWebView =
+        createWebView(
+            application = application,
+            delegate = delegate,


lng-stripe · 2025-02-26T16:51:25Z

...t/src/main/java/com/stripe/android/connect/webview/StripeConnectWebViewContainerViewModel.kt

+            // don't send errors for requests that aren't for the main page load
+            if (isMainPageLoad) {
+                // TODO - wrap error better
+                _eventFlow.tryEmit(ComponentEvent.LoadError(RuntimeException(errorString)))


👀 Emitting an event object instead of directly invoking the Listener before.

lng-stripe · 2025-02-26T16:52:30Z

...t/src/main/java/com/stripe/android/connect/webview/StripeConnectWebViewContainerViewModel.kt

+
+        val BASE_DEPENDENCIES_KEY = object : CreationExtras.Key<BaseDependencies> {}
+
+        val Factory: ViewModelProvider.Factory = viewModelFactory {


👀 Extra scrutiny here, please.

what are you concerned about here? Anything in particular I should be looking out for?

Relates to my other comment:

👀 I'd appreciate extra scrutiny here since these ViewModel APIs are new to me. Relevant docs: https://developer.android.com/topic/libraries/architecture/viewmodel/viewmodel-factories

lng-stripe · 2025-02-26T16:54:23Z

connect/src/test/java/com/stripe/android/connect/webview/StripeConnectWebViewTest.kt

+import org.robolectric.RuntimeEnvironment
+
+@RunWith(RobolectricTestRunner::class)
+class StripeConnectWebViewTest {


👀 Tests in here aren't comprehensive, but they weren't before either. I added more to largely sanity check testability.

amk-stripe · 2025-02-26T18:06:11Z

connect/api/connect.api

@@ -284,14 +284,6 @@ public final class com/stripe/android/connect/appearance/fonts/CustomFontSource$
 	public final fun serializer ()Lkotlinx/serialization/KSerializer;
 }

-public final class com/stripe/android/connect/databinding/StripeConnectWebviewBinding : androidx/viewbinding/ViewBinding {


Do you know if this was part included in the API review? If so, I think you need to amend the API review and have someone re-approve it

amk-stripe · 2025-02-26T18:08:03Z

connect/src/main/java/com/stripe/android/connect/AccountOnboardingView.kt

@@ -31,11 +32,14 @@ class AccountOnboardingView private constructor(
        embeddedComponentManager: EmbeddedComponentManager? = null,
        listener: AccountOnboardingListener? = null,
        props: AccountOnboardingProps? = null,
+        cacheKey: String? = null,


Does this need to be included in an updated API review?

Yes. This change will be bundled in an upcoming API review that will also include presenting Account Onboarding full screen as opposed to returning a View.

amk-stripe · 2025-02-26T18:16:38Z

...t/src/main/java/com/stripe/android/connect/webview/StripeConnectWebViewContainerViewModel.kt

+
+        val BASE_DEPENDENCIES_KEY = object : CreationExtras.Key<BaseDependencies> {}
+
+        val Factory: ViewModelProvider.Factory = viewModelFactory {


what are you concerned about here? Anything in particular I should be looking out for?

lng-stripe added 19 commits February 24, 2025 14:57

refactor without xml layout

84ee5ff

fix StripeConnectWebViewContainer detachment

6de1e3f

support webview caching to handle config changes

6ae0df4

support webview caching in XML layouts

fedc3e0

add debug logging

382ee92

update api

608b03a

tag log messages

1cdcb91

fix log message

d986a90

refactor using ViewModel

754b8fe

controller -> viewModel

4ab7159

nit

61750eb

refactor out StripeConnectWebView

fa67f24

use Activity instead of Application context as needed

0f39f03

fix log tag

362ec30

fix StripeConnectWebViewContainerViewModel tests

deb676c

update StripeConnectWebViewTest

7188e59

api and cleanup

43744b5

cleanup context passing

4b0d671

comments and cleanup

bb4728e

lng-stripe added 5 commits February 25, 2025 22:59

docs, cleanup, add manager hash to VM key

63c0447

update API

eaa5f80

cleanup and docs

44476da

add more StripeConnectWebView tests

c286918

more tests, more cleanup

563468d

lng-stripe force-pushed the lng/connect-fix-webview-state branch from 3faf73b to 563468d Compare February 26, 2025 04:34

fix launching secure external web

dacfee5

lng-stripe commented Feb 26, 2025

View reviewed changes

lng-stripe marked this pull request as ready for review February 26, 2025 16:54

lng-stripe requested a review from a team as a code owner February 26, 2025 16:54

lng-stripe requested review from a team as code owners February 26, 2025 16:54

lng-stripe requested review from nschris-stripe and amk-stripe February 26, 2025 16:54

lng-stripe assigned amk-stripe Feb 26, 2025

amk-stripe reviewed Feb 26, 2025

View reviewed changes

amk-stripe approved these changes Feb 26, 2025

View reviewed changes

lng-stripe merged commit 3cc46f7 into master Feb 26, 2025
13 checks passed

lng-stripe deleted the lng/connect-fix-webview-state branch February 26, 2025 21:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[connect] Improve WebView state handling #10268

[connect] Improve WebView state handling #10268

lng-stripe commented Feb 26, 2025 •

edited

Loading

github-actions bot commented Feb 26, 2025 •

edited

Loading

lng-stripe Feb 26, 2025

amk-stripe Feb 26, 2025

lng-stripe Feb 26, 2025

lng-stripe Feb 26, 2025

lng-stripe Feb 26, 2025

lng-stripe Feb 26, 2025

lng-stripe Feb 26, 2025

lng-stripe Feb 26, 2025

lng-stripe Feb 26, 2025

lng-stripe Feb 26, 2025

lng-stripe Feb 26, 2025

amk-stripe Feb 26, 2025

lng-stripe Feb 26, 2025

lng-stripe Feb 26, 2025

amk-stripe Feb 26, 2025

amk-stripe Feb 26, 2025

lng-stripe Feb 26, 2025

amk-stripe Feb 26, 2025

		internal suspend fun requestCameraPermission(activity: Activity): Boolean? {
		if (checkSelfPermission(activity, Manifest.permission.CAMERA) == PackageManager.PERMISSION_GRANTED) {


		val BASE_DEPENDENCIES_KEY = object : CreationExtras.Key<BaseDependencies> {}

		val Factory: ViewModelProvider.Factory = viewModelFactory {

[connect] Improve WebView state handling #10268

[connect] Improve WebView state handling #10268

Conversation

lng-stripe commented Feb 26, 2025 • edited Loading

Summary

Motivation

Testing

Demos

github-actions bot commented Feb 26, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lng-stripe commented Feb 26, 2025 •

edited

Loading

github-actions bot commented Feb 26, 2025 •

edited

Loading