Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

moves autofill service (and activities) to its own process #5720

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

cmonfortep
Copy link
Contributor

@cmonfortep cmonfortep commented Mar 2, 2025

Task/Issue URL: https://app.asana.com/0/1200156640058969/1209537052601451

Description

Moves Autofill Service (and activities) to its own process

Steps to test this PR

Feature 1

  • When using autofill in other apps
  • ensure we don't refresh atb (nor emit exti)
  • ensure no ml pixel is sent when selecting a suggestion

then smoke test the service

UI changes

Before After
!(Upload before screenshot) (Upload after screenshot)

@cmonfortep
Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@cmonfortep cmonfortep force-pushed the feature/cristian/autofill/move_autofill_service_to_own_process branch from 2ecd9a6 to 1f53db5 Compare March 2, 2025 21:55
@cmonfortep cmonfortep marked this pull request as ready for review March 2, 2025 23:54
@cmonfortep cmonfortep requested a review from CDRussell March 2, 2025 23:57
@@ -100,7 +100,9 @@ class RealAutofillService : AutofillService() {

callback.onSuccess(response)
}.onFailure {
pixel.fire(AutofillPixelNames.AUTOFILL_SERVICE_CRASH, mapOf("message" to it.extractExceptionCause()))
if (it !is kotlinx.coroutines.CancellationException) {
Copy link
Contributor Author

@cmonfortep cmonfortep Mar 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While testing I've noticed some crash pixels related when the coroutine is cancelled (before completion). That doesn't represent a crash, so skipping if that's the case.

@cmonfortep cmonfortep requested a review from karlenDimla March 5, 2025 08:49
@@ -110,6 +111,14 @@ open class DuckDuckGoApplication : HasDaggerInjector, MultiProcessApplication()
}
}
}

runInSecondaryProcessNamed(AUTOFILL_SERVICE_PROCESS_NAME) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once you have rebased with develop, you wouldn’t need any changes in this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just pushed the actual changes, so this should be solved.

@cmonfortep cmonfortep force-pushed the feature/cristian/autofill/move_autofill_service_to_own_process branch from 1f53db5 to 9bba237 Compare March 5, 2025 11:35
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.

2 participants