feat: Add Popup Bridge++ app switch for PayPal/Venmo detection and launchApp()#109
feat: Add Popup Bridge++ app switch for PayPal/Venmo detection and launchApp()#109pedrofsn wants to merge 34 commits into
Conversation
…n, launchApp(), and deep-link return handling
…BridgeClient that were discovered during end-to-end testing of the Popup Bridge++ app switch flow
…2/update These changes fix bugs and harden the return-to-app handling in Popup BridgeClient that were discovered during end-to-end testing of the Popup Bridge++ app switch flow
…AppSwitch is true Previously, cancel URLs (containing '/cancel' path) were being routed to the app-switch handler even when enablePopupBridgeAppSwitch was false (default). This caused the browser-switch flow to silently fail without any JavaScript callback. The fix ensures that the app-switch return URI check (isAppSwitchReturnUri) is only performed when enablePopupBridgeAppSwitch is true, preserving the original browser-switch behavior for all other cases.
|
Please fix Lint, and instrumentation tests for this PR. |
|
Once you've resolved the instrumentation test failure please have your tech lead review this PR. Once it's approved by them you can remove the |
The dismiss test was asserting immediately after clicking "I don't like any of these colors" without waiting for the full popup bridge return cycle (deep link onNewIntent > handleReturnToApp > window.popupBridge.onComplete > DOM update). This caused a race condition that failed consistently on API 35 in CI and locally on arm64, while passing on API 31 where the emulator is fast enough. Added waitForExists(BROWSER_TIMEOUT) before the final assertion, consistent with how the color selection tests already handle the same async return flow.
eb38364 to
ad73947
Compare
|
Race Condition Fix The dismiss test ( Fix: Added before the final assertion, which is consistent with how the color selection tests already handle the same async return flow. This ensures we wait for the bridge to complete before asserting. |
…ation Move app-switch logic (launchApp, handleAppSwitchReturn, URI helpers, clearReturnIntentIfPresent) into a dedicated AppSwitchHandler class, reducing PopupBridgeClient from 17 functions to 9 (threshold: 11). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add isVenmoAppSwitchUri() to detect account.venmo.com/braintree/checkout
- Add rewriteToVenmoHost() to rewrite the URL host to venmo.com before
firing the intent — venmo.com has a broad catch-all intent filter that
handles any path including /braintree/checkout
- Make VENMO_APP_PACKAGE internal so AppSwitchHandler can access it
- Set setPackage("com.venmo") for Venmo URLs so the intent targets the
Venmo app directly
- Add 5 unit tests covering isVenmoAppSwitchUri and rewriteToVenmoHost
When the Braintree JS SDK calls popupBridge.open(url) with a Venmo URL (account.venmo.com/braintree/checkout), intercept it and call appSwitchHandler.launchApp() instead of BrowserSwitchClient.start(). If the app switch fails, onOpenUrl fallback delegates back to the browser. Wire onVenmoUrl callback so shouldOverrideUrlLoading also routes to AppSwitchHandler when the WebView navigates directly to the Venmo URL.
The Braintree JS SDK navigates the WebView directly to account.venmo.com/braintree/checkout in addition to calling popupBridge.open(). Override shouldOverrideUrlLoading to detect this main-frame navigation, block the WebView from rendering it, and trigger the native app switch via the onVenmoUrl callback.
…ibility - Gate onVenmoUrl and onOpen Venmo intercept behind enablePopupBridgeAppSwitch. Without this, merchants with the flag disabled still got Venmo app-switched but received a canceled result on return (handleReturnToApp was gated but the launch paths were not). - Fix setVenmoInstalled indentation in PopupBridgeWebViewClient. - Make isPayPalInstalled and isVenmoInstalled internal (were implicitly public).
- Add "Launch gse-appstestbed" button routing to https://gse-appstestbed.com - Enable DOM storage, third-party cookies, and enablePopupBridgeAppSwitch when loading gse-appstestbed so the Venmo native app switch flow works end-to-end in the demo Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix indentation in PopupBridgeJavascriptInterface returnUrlPrefix getter - Consolidate isPayPalInstalled analytics check inside enablePopupBridgeAppSwitch block
|
/ready |
Resolves conflicts in CHANGELOG.md and PopupBridgeTest.java. Keeps 5.1.1 app-switch entry and unreleased browser-switch/compileSdk bumps.
…tests The onLaunchApp callback is only wired in PopupBridgeClient when enablePopupBridgeAppSwitch is true. The 7 tests that exercise this callback were calling initializeClient() with the default (false), so the mockk slot was never captured and threw IllegalStateException. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces an optional “Popup Bridge++” app-switch flow that can detect/launch native PayPal/Venmo apps from a WebView-based checkout and handle deep-link returns back into the WebView, while keeping the feature disabled by default for backwards compatibility.
Changes:
- Add
enablePopupBridgeAppSwitchplumbing,launchApp()JS interface support, and anAppSwitchHandlerto manage app launches + return handling. - Add PayPal/Venmo install detection (manifest
<queries>,Contextextensions, and JS-exposed properties) plus new analytics event constants. - Extend the Demo app and add/adjust unit tests for the new behavior.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| PopupBridge/src/main/java/com/braintreepayments/api/PopupBridgeClient.kt | Wires the optional app-switch feature into PopupBridgeClient init and return handling. |
| PopupBridge/src/main/java/com/braintreepayments/api/PopupBridgeWebViewClient.kt | Adds Venmo URL interception hook for launching the native app from WebView navigation. |
| PopupBridge/src/main/java/com/braintreepayments/api/internal/AppSwitchHandler.kt | Implements app launch, package pinning, and app-switch deep-link return classification/handling. |
| PopupBridge/src/main/java/com/braintreepayments/api/internal/PopupBridgeJavascriptInterface.kt | Adds JS-accessible install checks and launchApp() callback support. |
| PopupBridge/src/main/java/com/braintreepayments/api/internal/AppInstalledChecks.kt | Adds PayPal install detection and exposes package constants internally. |
| PopupBridge/src/main/java/com/braintreepayments/api/PopupBridgeAnalytics.kt | Adds analytics constants for app-switch lifecycle events. |
| PopupBridge/src/main/AndroidManifest.xml | Adds PayPal package to <queries> for install detection visibility. |
| PopupBridge/src/test/java/com/braintreepayments/api/PopupBridgeClientUnitTest.kt | Adds app-switch/unit coverage for launch and return paths. |
| PopupBridge/src/test/java/com/braintreepayments/api/PopupBridgeWebViewClientTest.kt | Updates assertions for Venmo-installed JS injection. |
| PopupBridge/src/test/java/com/braintreepayments/api/PopupBridgeAnalyticsTest.kt | Verifies analytics constant values (new + existing). |
| PopupBridge/src/test/java/com/braintreepayments/api/internal/AppSwitchHandlerTest.kt | Tests Venmo app-switch URL detection and rewrite behavior. |
| PopupBridge/src/test/java/com/braintreepayments/api/internal/AppInstalledChecksTest.kt | Tests PayPal/Venmo install-check extensions. |
| PopupBridge/src/test/java/com/braintreepayments/api/internal/PopupBridgeJavascriptInterfaceUnitTest.kt | Adds tests for new JS interface surface. |
| Demo/src/main/java/com/braintreepayments/popupbridge/demo/MainActivity.java | Adds a “custom URL” launcher entry point. |
| Demo/src/main/java/com/braintreepayments/popupbridge/demo/PopupActivity.java | Enables app-switch in the demo based on the chosen URL and updates intent handling. |
| Demo/src/main/res/layout/activity_main.xml | Adds a button to launch the custom URL flow. |
| CHANGELOG.md | Adds a 5.1.1 entry describing the new app-switch feature. |
Comments suppressed due to low confidence (1)
PopupBridge/src/main/java/com/braintreepayments/api/PopupBridgeWebViewClient.kt:35
- API 23 devices (minSdk is 23) will use the deprecated
shouldOverrideUrlLoading(WebView, String)overload. Without Venmo app-switch interception there, the app-switch flow won’t trigger on Android 6. Add the same Venmo URL handling to the string overload, and gate it ononVenmoUrlbeing set so non-app-switch flows are unaffected.
@Deprecated("Deprecated in [android.webkit.WebViewClient]")
override fun shouldOverrideUrlLoading(view: WebView?, url: String?): Boolean {
return delegate?.shouldOverrideUrlLoading(view, url) ?: super.shouldOverrideUrlLoading(view, url)
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
When testing the flows, even with |
- Gate shouldOverrideUrlLoading Venmo intercept on onVenmoUrl != null to avoid swallowing normal WebView navigation when app-switch is disabled - Gate AppSwitchHandler.shouldHandleReturn on expectingAppSwitchReturn to prevent browser-switch returns from being routed into the app-switch handler - Fix KDoc indentation on isPayPalInstalled in PopupBridgeJavascriptInterface - Fix test indentation in PopupBridgeWebViewClientTest (two runTest blocks) - Replace tautological assertions with meaningful assertFalse checks; add proper PackageManager mock setup for isVenmoInstalled test - Update PopupBridgeClientUnitTest to set expectingAppSwitchReturn=true before testing app-switch return handling - Rename isGSE variable to enableAppSwitch in PopupActivity demo - Add comment on CUSTOM_URL in MainActivity about GSE test URL
I'm experiencing the same behavior |
…to align with iOS
…succeeded/available) Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- MainActivity passes enableAppSwitch=true via intent extra so all URLs trigger PayPal app switch during testing - PopupActivity reads the flag from intent extras instead of comparing against a hardcoded CUSTOM_URL, making the mechanism more flexible
saralvasquez
left a comment
There was a problem hiding this comment.
This is a great first pass! I have a few comments and questions. Additionally I ran a quick search with Claude and it seems like there might be some logic paths that aren't covered by unit tests. Could you just take a quick pass at these flows and double check they're covered. Thanks!
1. PopupBridgeWebViewClient Venmo URL interception — The new shouldOverrideUrlLoading block that intercepts Venmo URLs and invokes onVenmoUrl has no test. No test sets onVenmoUrl on the client, passes a https://account.venmo.com/braintree/checkout request, and verifies the callback fires and true is returned.
2. onOpen routing Venmo URLs to app switch — When enablePayPalAppSwitch=true, the onOpen lambda in PopupBridgeClient checks uri.isVenmoAppSwitchUri() and routes to appSwitchHandler.launchApp() instead of openUrl(). This branch is untested.
3. isPayPalInstalled JS property when app IS installed — The existing test only covers enablePayPalAppSwitch=false (asserts false). There is no test for enablePayPalAppSwitch=true with PayPal actually installed (asserting true).
4. Fragment-based app-switch return URI — isAppSwitchReturnUri returns true when host == POPUP_BRIDGE_URL_HOST && !fragment.isNullOrBlank(). No test exercises a return URI that uses a fragment instead of a recognized path.
5. Sandbox PayPal URL — isPayPalAppSwitchUri explicitly handles sandbox.paypal.com/app-switch-checkout, but there's no test for it — only paypal.com is exercised indirectly.
6. Error path return URIs — hasAppSwitchPath matches /onerror and /error paths and routes them to onComplete. No test verifies this; only /onApprove and cancel paths are covered.
|
Op also it looks like there's some lint issues here. Could you take a look at that? |
CUSTOM_URL and its associated button are no longer needed — app switch is now enabled via intent extra, making the custom URL workaround obsolete. Also removes unused @nonnull import.
Covers 6 untested logic paths flagged by code review: - shouldOverrideUrlLoading invokes onVenmoUrl and returns true for Venmo URL - shouldOverrideUrlLoading returns false for Venmo URL when onVenmoUrl is null - onOpen with Venmo URL routes to launchApp when enablePayPalAppSwitch=true - onOpen with Venmo URL uses browser switch when enablePayPalAppSwitch=false - isPayPalInstalled returns true when PayPal is installed and flag is enabled - Fragment-based app-switch return URI invokes JS completion - Sandbox PayPal URL (sandbox.paypal.com) launches intent correctly - /onerror and /error paths route to onComplete (not onCanceled)
jaxdesmarais
left a comment
There was a problem hiding this comment.
Thanks for this, the extraction into AppSwitchHandler is the right instinct, and I want to call out that Android avoids the biggest issue we hit on iOS (#95): the return comes back through the existing, explicit handleReturnToApp(intent) entry point rather than a global NotificationCenter post, so there's no public transport constant and no merchant-side wiring. 👍
That said, a few things carry over from the iOS review and a few are Android-specific. My top concern is the functional report below let's get that sorted before deeper review
Blocker: Two reviewers report PayPal app switch doesn't actually work with the flag enabled (Venmo does). Since native PayPal switch is the headline of this PR, we need a repro/fix before sign-off. Likely suspects: the JS SDK never seeing window.popupBridge.isPayPalInstalled === true, the com.paypal.android.p2pmobile package not matching the test build, or the PayPal app lacking an intent filter for the /app-switch-checkout path the intent pins to. Can you confirm which, with the analytics (app-switch:available / :started / :failed) from a failing run?
| val intent = Intent(Intent.ACTION_VIEW, targetUri).apply { | ||
| addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) | ||
| if (targetUri.isPayPalAppSwitchUri()) { | ||
| setPackage(PAYPAL_APP_PACKAGE) | ||
| } else if (uri.isVenmoAppSwitchUri()) { | ||
| setPackage(VENMO_APP_PACKAGE) | ||
| } | ||
| } |
There was a problem hiding this comment.
When the URL is neither a PayPal nor a Venmo app-switch URI, this still fires Intent(ACTION_VIEW, uri) with FLAG_ACTIVITY_NEW_TASK and no package set, an implicit intent for an arbitrary URI that originates from web content (reachable via window.popupBridge.launchApp(anyUrl) once the flag is on). This is the Android version of the validation gap Copilot flagged on iOS #95. We should validate the URI is an expected PayPal/Venmo app-switch URL (https + known host/path) before launching, and otherwise route to onError/onOpenUrl rather than launching a bare implicit intent. Note the happy-path test (...invoke("https://www.paypal.com/checkout")) actually exercises this no-package branch, so it's not hypothetical.
There was a problem hiding this comment.
Good catch. Will add explicit URI validation before startActivity: only https:// URIs matching paypal.com/sandbox.paypal.com (path /app-switch-checkout) or venmo.com/account.venmo.com (path /braintree/checkout) are allowed. Anything else routes to onError. Will fix the test to also cover the rejection path.
| } | ||
|
|
||
| clearReturnIntentIfPresent() | ||
| expectingAppSwitchReturn = true |
There was a problem hiding this comment.
expectingAppSwitchReturn is set true before the switch, but there's no path that resets it / signals cancel if the user returns to the merchant app without a deep link (back button, recents, or backgrounding the PayPal app). Browser-switch handles this via BrowserSwitchFinalResult.NoResult -> runCanceledJavaScript(); the app-switch path has no equivalent, so the JS flow hangs. This is the same "missing return hangs the flow" failure mode we worried about on iOS, just via a different mechanism. Can we add an app-switch cancel/no-result path (e.g. on onResume with no matching deep link while expectingAppSwitchReturn is true)?
There was a problem hiding this comment.
Agreed. I will add a cancel path in onResume: when expectingAppSwitchReturn is true and shouldHandleReturn(intent.data) is false, we'll call onCanceled() and reset the flag. This mirrors BrowserSwitchFinalResult.NoResult and ensures the JS flow is never left hanging.
| ) | ||
| is BrowserSwitchFinalResult.NoResult -> runCanceledJavaScript() | ||
| } | ||
| appSwitchHandler.clearReturnIntentIfPresent() |
There was a problem hiding this comment.
appSwitchHandler.clearReturnIntentIfPresent() runs in the browser-switch coroutine branch unconditionally i.e. even when enablePayPalAppSwitch is false. It mutates activity.intent (nulls data) whenever the host is popupbridgev1, which is true for normal browser-switch returns. That's an un-gated behavior change on the default/disabled path. Please move it inside the enablePayPalAppSwitch guard so the feature-off flow is byte-for-byte unchanged.
There was a problem hiding this comment.
Fixed locally. clearReturnIntentIfPresent() is now wrapped inside if (enablePayPalAppSwitch) (lines 183-185 of PopupBridgeClient.kt). (Fix applied, pending commit/push.) Gap noted: no unit test verifies clearReturnIntentIfPresent() is NOT called when feature is off — the feature-off test in PopupBridgeClientUnitTest.kt does not mock or assert on this method call
| return hasAppSwitchPath() | ||
| } | ||
|
|
||
| private fun Uri.isCancelUri(): Boolean { |
There was a problem hiding this comment.
The cancel/approve/error classification uses substring matching (contains("oncancel") || contains("/cancel"), and the same for approve/error in hasAppSwitchPath). Legitimate path segments like /cancellation or /error-details would be misrouted, and it's hard to reason about (also the oncancel vs /cancel asymmetry @anibalb2500 already asked about). Can we switch to exact path-segment matching against the known return paths instead of contains?
|
|
||
| private fun Uri.isAppSwitchReturnUri(): Boolean { | ||
| if (host != POPUP_BRIDGE_URL_HOST) return false | ||
| if (!fragment.isNullOrBlank()) return true |
There was a problem hiding this comment.
isAppSwitchReturnUri treats any non-blank fragment as an app-switch return. That's very broad it's only saved by the expectingAppSwitchReturn gate upstream. What return shapes actually carry a fragment here? If it's a specific known case we should match it explicitly rather than "any fragment wins."
There was a problem hiding this comment.
fragment is the fallback for PayPal return URIs that carry no path (e.g. popupbridgev1://popupbridgev1#someFragment, confirmed in AppSwitchHandlerTest).
Logic: host == popupbridgev1 AND (fragment present OR hasAppSwitchPath()).
The broad fragment check is safe because expectingAppSwitchReturn upstream gate only allows this branch after we explicitly launched an app switch. The check is intentional, not a loose guard.
| webView.settings.javaScriptEnabled = true | ||
| webView.addJavascriptInterface(popupBridgeJavascriptInterface, POPUP_BRIDGE_NAME) | ||
| webView.webViewClient = popupBridgeWebViewClient | ||
| if (enablePayPalAppSwitch) { |
There was a problem hiding this comment.
Naming: enablePayPalAppSwitch gates both PayPal and Venmo here (the onVenmoUrl wiring on the next line is inside this block). That's the same concern I raised on iOS, the flag name under-describes its scope. Let's either rename it to reflect that it enables app switch for both, or split the two. (Also worth aligning the final name 1:1 with whatever lands on iOS.)
There was a problem hiding this comment.
Confirmed from source: iOS Braintree SDK uses enablePayPalAppSwitch in BTPayPalVaultRequest.swift and BTPayPalCheckoutRequest.swift.
Android was previously enablePopupBridgeAppSwitch and was renamed in commit cc404e7 specifically to match iOS.
|
|
||
| with(popupBridgeJavascriptInterface) { | ||
| onOpen = { url -> openUrl(url) } | ||
| onOpen = { url -> |
There was a problem hiding this comment.
Venmo app-switch URLs are now intercepted in two places here in the onOpen JS-bridge wrapper, and in PopupBridgeWebViewClient.shouldOverrideUrlLoading. Is both paths genuinely needed, or can Venmo interception live in one place? This is the "app-switch logic sprinkled through the coordinator" smell @buzzamus flagged on iOS.
There was a problem hiding this comment.
We verified this against the Venmo Android SDK (checkoutJSIntegration.js). The PPCP JS SDK calls window.popupBridge.open(url), that is the real trigger. The two paths are not redundant but complementary.
Path B: onOpen (PopupBridgeClient.kt:123): fires first when PPCP JS calls window.popupBridge.open(venmoUrl). popup-bridge intercepts at the JS bridge layer, routes to appSwitchHandler.launchApp(), and the WebView navigation never happens. This is the primary path.
Path A: shouldOverrideUrlLoading (PopupBridgeWebViewClient.kt:40): fires if the WebView navigates window.location directly, i.e. a code path that bypasses popupBridge.open() (different SDK version, or direct navigation before popup bridge initializes). Defensive fallback.
The two paths are mutually exclusive in practice: if Path B fires, window.location never changes so Path A never triggers. If the JS navigates directly (Path B missed), Path A catches it. The duplication is intentional defense-in-depth, not an oversight
There was a problem hiding this comment.
Confirmed in checkoutJSIntegration.js line 14 does window.location = openUrl, the JS SDK itself performs a window.location assignment. Path A (shouldOverrideUrlLoading) is the direct safety net for that exact assignment if Path B misses. Both paths are necessary
| const val POPUP_BRIDGE_APP_DETECTED = "popup-bridge:app-switch:available" | ||
| const val POPUP_BRIDGE_APP_LAUNCHED = "popup-bridge:app-switch:started" | ||
| const val POPUP_BRIDGE_APP_LAUNCH_FAILED = "popup-bridge:app-switch:failed" | ||
| const val POPUP_BRIDGE_APP_SWITCH_RETURNED = "popup-bridge:app-switch:succeeded" |
There was a problem hiding this comment.
Same note I left on iOS: app detection (app-switch:available) should be a tag on the existing events, not its own event event names are for conversion, and the other author removed the standalone detection event on iOS for this reason. Let's keep the analytics shape consistent across platforms.
…aintree#2 braintree#3 braintree#4 braintree#8 braintree#1 — URI validation: reject non-PayPal/Venmo URIs before startActivity in launchAppOnMainThread; routes to onError instead of firing an implicit intent for arbitrary web-content URLs. braintree#2 — Cancel path: add handleNoResult() to AppSwitchHandler; called in handleReturnToApp when enablePayPalAppSwitch is true but no matching deep link arrives (user pressed Back). Prevents JS flow from hanging. braintree#3 — Feature guard: wrap clearReturnIntentIfPresent() inside if (enablePayPalAppSwitch) in the browser-switch coroutine branch so the feature-off path is byte-for-byte unchanged. braintree#4 — Exact path-segment matching: replace contains() in isCancelUri() and hasAppSwitchPath() with split/filter/any exact-segment logic; eliminates false positives like /cancellation matching /cancel. braintree#8 — Remove POPUP_BRIDGE_APP_DETECTED: drop the app-switch:available analytics event entirely to match iOS behaviour (iOS does not track app detection at all — only the initiation event). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
334ae13 to
0bf342f
Compare
Summary of changes
launchApp()method to open native PayPal/Venmo apps instead of browserisPayPalInstalledandisVenmoInstalledJavaScript interface propertiesenablePopupBridgeAppSwitchconstructor parameter (defaults to false for backwards compatibility)CI Note
The
dependency-reviewCI step is expected to fail for inner-sourced PRs and is not required for mergeability. Confirmed by Sara Vasquez: this step cannot run on forks due to GitHub permission restrictions.AI Usage
Which AI Agent Was Used?
How was AI used?
Estimated AI Code Contribution
Checklist
Authors
Inner Source Process
/inner sourceon this PR — addsinner sourceandtech lead review requiredlabels. Open PR in draft state./ready— removestech lead review requiredlabel. Move to ready to review.Inner Source Checklist