Fire Mode: Launch NTP in different modes#8742
Conversation
3d52f23 to
96a48e5
Compare
e607eec to
4ead19a
Compare
4ead19a to
605d6de
Compare
96a48e5 to
04a7fae
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 04a7fae. Configure here.
605d6de to
09cf304
Compare
04a7fae to
0bf23a7
Compare
malmstein
left a comment
There was a problem hiding this comment.
LGTM, approving. did not install or smoke-test the APK — static-only. nice refactor: generalising the old pendingFireToRegularIntent into a typed PendingModeSwitch (ProcessIntent / OpenNewTab) carried across the recreate is clean, and the switchModeThen design that re-enters itself on the recreated instance is genuinely well thought through — the doc comments and the explicit intent-redirection security invariant on runAction are exactly what I'd want to see on this path. the rejection fallback only reaches OpenNewTab(FIRE) when fire is unavailable, which is the right behaviour.
one non-blocking ask before merge: please add the round-trip test for toBundle/toPendingModeSwitch — it's the piece most likely to break quietly and the easiest thing here to cover. and since I didn't run it, worth a quick device check of the chat-menu "new tab" / "new fire tab" entries and an external LAUNCH_REQUIRES_REGULAR_MODE intent arriving while in fire mode, to confirm the deferred action survives the recreate.
| return bundle | ||
| } | ||
|
|
||
| internal fun Bundle.toPendingModeSwitch(): PendingModeSwitch? { |
There was a problem hiding this comment.
toBundle / toPendingModeSwitch is the load-bearing part here — if it doesn't round-trip cleanly the deferred action is silently lost across the recreate. it's pure and Android-testable (same AndroidJUnit4 setup as RealTabTitleResolverTest from #8714) — can we add a round-trip test covering both PendingAction variants plus the null/garbage-bundle paths (unknown action, bad mode name, missing intent) before this merges?
There was a problem hiding this comment.
Added PendingModeSwitchTest (AndroidJUnit4, same setup as RealTabTitleResolverTest) — round-trips both PendingAction variants plus the null paths: empty bundle, unknown mode name, missing action, unknown action, and ProcessIntent with a missing intent. The malformed-bundle cases start from a valid bundle and corrupt one field, so a stale wire-format key fails the test rather than silently passing. 12f228066c.
09cf304 to
e8ca846
Compare
0bf23a7 to
082a64a
Compare
70eaff9 to
9465d5b
Compare
082a64a to
428c983
Compare
toPendingModeSwitch() is a fail-soft parser: every other malformed-bundle
branch returns null. The bare BrowserMode.valueOf() was the one branch that
could throw on restore (e.g. a persisted name no longer in the enum), so
parse it with runCatching{}.getOrNull() to degrade to "no pending action"
like the rest of the function.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Covers both PendingAction variants (ProcessIntent, OpenNewTab) plus the null-returning paths that silently drop a deferred action across recreation: empty bundle, unknown mode name, missing/unknown action, and ProcessIntent with a missing intent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
12f2280 to
407bc24
Compare


Task/Issue URL: https://app.asana.com/1/137249556945/project/1207418217763355/task/1215275434974447?focus=true
Description
This PR implements the functionality that allows launching NTPs specifically in Regular or Browser mode.
Steps to test this PR
Note
Medium Risk
Touches core BrowserActivity lifecycle, mode switching, and intent handling; incorrect deferral or replay could drop tabs or mishandle launch intents, though ProcessIntent consumption is explicitly guarded.
Overview
Adds mode-aware new tab launches from Duck.ai’s “+” menu: New tab switches to Regular mode first; New Fire tab switches to Fire mode first.
Replaces the old FIRE→REGULAR-only deferred
Intentstash with a generalswitchModeThenflow: pending work is stored asPendingModeSwitch(ProcessIntentorOpenNewTab), saved in instance state across activity recreate, then replayed inshowWebContent.launchNewTabgains an optionalbrowserModeparameter (default: current mode) and always goes through that deferral path when a switch is needed.New
PendingModeSwitch.ktdefines bundle round-tripping;PendingModeSwitchTestcovers encode/decode edge cases.runActiondocuments that carried intents are consumed viaprocessIntent, not re-launched.Reviewed by Cursor Bugbot for commit 407bc24. Bugbot is set up for automated code reviews on this repo. Configure here.