Skip to content

Rename screenName to deeplinkScreenName and widen deeplink coverage - #9577

Open
catalinradoiu wants to merge 1 commit into
developfrom
feature/cradoiu/improve-deeplinking
Open

Rename screenName to deeplinkScreenName and widen deeplink coverage#9577
catalinradoiu wants to merge 1 commit into
developfrom
feature/cradoiu/improve-deeplinking

Conversation

@catalinradoiu

@catalinradoiu catalinradoiu commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Task/Issue URL: https://app.asana.com/1/137249556945/project/1211724162604201/task/1213980692214675
Tech Design URL (if applicable):
API Proposals URL(s) (if applicable): None

Description

Two things, per the task.

1. screenName -> deeplinkScreenName on @ContributeToActivityStarter. The old name read like "the name of the screen" rather than "the name this screen is deeplinked by". Only the KSP processor reads the argument, so this is the annotation, the processor, the golden files, and the 27 existing call sites. The KDoc now also states the naming convention, the uniqueness requirement, and which screens should not declare a name.

DeeplinkActivityParams.screenName is deliberately left alone: it already sits inside a Deeplink* type, so deeplinkActivityParams.deeplinkScreenName stutters, and renaming it would be an -api change for no clarity gain.

2. Declared a name for 25 screens that are sensible deeplink entry points, so they can be targeted from a What's New message. Names follow the existing <feature>.<subScreen> convention, each segment camelCase:

Area Names
Settings about, privateSearch, webTrackingProtection, threatProtection, dataClearing, permissions, sitePermissions, downloads, duckplayer.settings
Other platforms & sync macos, windows, sync
VPN / AppTP vpn.geoswitching, vpn.exclusions, vpn.customDns, apptp.onboarding
Passwords & autofill passwords, passwords.import, autofill.settings
Privacy Pro subscriptions.restore, subscriptions.changePlan, ppro.feedback, pir, pir.dashboard, duckai.paidSettings

Screens deliberately left without a name:

  • WebView hosts that load a caller-provided URLWebViewActivity, SettingsWebViewActivity, SubscriptionsWebViewActivityWithParams, PdfViewerActivity. A deeplink name there would let an RMF payload choose the URL loaded inside the browser's own trusted chrome. (The fixed-destination subscriptions.purchase / subscriptions.upgrade variants are unaffected.)
  • Mid-flow screens / screens needing caller context — email protection in-context signup, the Google password/bookmark import web flows, AutofillPasswordsManagementViewCredential, PrivacyDashboardHybridActivity (needs a tab), the breakage reporting flow, ModalSurfaceActivity, and others.
  • Everything in *-internal modules and internal build variants (15 activities).

Two things fell out of the audit and are fixed here:

  • Sync had no deeplink at all. SyncActivityParamMapper is handwritten (it switches between SyncActivity and SyncActivityV2 behind useSimplifiedSync) and returned null for every deeplink. It now resolves "sync", with the flag switching intact.
  • Params types whose values all have defaults could not resolve. The generated mapper built params either from an objectInstance or from a non-empty payload, so subscriptions.restore (a data class with all-default values) silently returned null on an empty payload. Mappers now fall back to parsing "{}".

Note for whoever writes the RMF message: passwords and autofill.settings need {"source":"Unknown"} as the payload — their source has no default. Everything else works with no payload.

Steps to test this PR

Deeplinks resolve

  • Install an internal build.
  • Internal settings -> "Remote messaging Settings" -> paste a config URL containing a cards_list message on the modal surface with navigation actions for the names in the table above -> "Download config now".
  • Background the app for the modal threshold (or bump the message id), reopen, and confirm the What's New modal appears with every entry.
  • Tap each entry and confirm it opens the matching screen.
  • Note: one unresolvable name drops the whole message silently, so if no modal appears at all, a name or payload is wrong rather than the config failing to download.

Payload-carrying screens

  • Confirm passwords and autofill.settings open with {"source":"Unknown"}.
  • Confirm subscriptions.restore opens with an empty payload (this is the codegen fallback above; it returned null before this PR).

Sync

  • Confirm a navigation action with value sync opens the sync screen, with useSimplifiedSync both on and off.

Nothing regressed

  • Existing deeplinks still work, e.g. settings, tabSwitcherWithParams with {"browserMode":"FIRE"}.

UI changes

Before After
No UI changes No UI changes

Note

Medium Risk
Widens remote-message navigation into more in-app screens and changes generated deeplink param construction. Risk is mainly mis-routed RMF actions or silent message drops from name/payload mismatches, not auth or data handling.

Overview
Renames @ContributeToActivityStarter's screenName to deeplinkScreenName so the annotation reads as a deeplink identifier, not a screen title. Docs and KSP now spell out uniqueness, <feature>.<subScreen> naming, and which screens must not opt in (mid-flow, caller context, caller-provided URLs, internal modules). DeeplinkActivityParams.screenName is unchanged.

Many landing/settings screens now declare a name so RMF can open them. Existing names are migrated in place. WebView hosts that take a URL, mid-flow screens, and internal variants stay unnamed.

Generated mappers fall back to Moshi "{}" when there is no payload, so data classes with all-default fields (e.g. restore subscription) resolve instead of returning null. Handwritten SyncActivityParamMapper now maps "sync" to SyncActivityWithEmptyParams while still switching V1/V2 via useSimplifiedSync.

Reviewed by Cursor Bugbot for commit 3f04b7a. Bugbot is set up for automated code reviews on this repo. Configure here.

Rename the @ContributeToActivityStarter parameter to deeplinkScreenName so
it reads as what it is, and declare a name for the screens that make sense
as a deeplink entry point.

Screens deliberately left without a name: mid-flow screens, screens needing
caller context, WebView hosts that load a caller-provided URL, and everything
in internal/dev modules.

Sync had no deeplink at all because its mapper is handwritten to switch
between SyncActivity and SyncActivityV2; it now resolves "sync".

The generated mapper only built params from an object instance or a non-empty
payload, so a params type whose values all default (subscriptions.restore)
could not resolve. It now falls back to parsing an empty payload.

Task/Issue URL: https://app.asana.com/1/137249556945/project/1211724162604201/task/1213980692214675

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant