Bug 1734923 Comment 22 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

I'm handing off my WIP to the GeckoView team. The [proposal document](https://docs.google.com/document/d/1YFgXsUaRE487VpX-AkTs-7yf1VF40nrJwsDmOHOSh78/edit#) was reviewed so my approach seems okay from a high level.

I pushed my WIP code. The first commit adds the LoadRequestFilter class and can probably be reused. The second commit adds an API in GeckoSessionSettings for the app to set its LoadRequestFilters: it can probably be reused. The third commit integrates the declarative API with page load requests: this commit might be worth rewriting because it's sloppy. The fourth commit is a demonstration of this API in geckoview_example and probably shouldn't be landed. I wrote tests though they're not totally thorough so they should be revisited.

The LoadRequestFilterAppLink code was adapted from [ac's AppLinksInterceptor](https://searchfox.org/mozilla-mobile/rev/9632fc5c1e5c99c4bb9d189f33a2c136866b4298/android-components/components/feature/app-links/src/main/java/mozilla/components/feature/app/links/AppLinksInterceptor.kt#72-112) logic, which I found to be really challenging to do. To minimize possibly added bugs, I used the ac tests verbatim with a shim layer: however, this runs on robolectric (to allow us to run verbatim) so I'm not sure we want to keep it. We should also see if there are more tests to write.

In addition to the "Unsolved challenges" in the proposal doc and the TODOs in the code, here is one other TODO item. In the current implementation, the app link code, i.e. the slow queryIntentActivities, is run for every LoadRequestFilter that declares MATCH_APP_LINK to true: this doesn't affect fenix because it only declares one but we may want to cache the results.

I can continue to do the performance measurements and I'm around if you have questions. Thank you!
I'm handing off my WIP to the GeckoView team. The [proposal document](https://docs.google.com/document/d/1YFgXsUaRE487VpX-AkTs-7yf1VF40nrJwsDmOHOSh78/edit#) was reviewed so my approach seems okay from a high level.

I pushed my WIP code. The first commit adds the LoadRequestFilter class and can probably be reused. The second commit adds an API in GeckoSessionSettings for the app to set its LoadRequestFilters: it can probably be reused. The third commit integrates the declarative API with page load requests: this commit might be worth rewriting because it's sloppy. The fourth commit is a demonstration of this API in geckoview_example and probably shouldn't be landed. I wrote tests though they're not totally thorough so they should be revisited.

The LoadRequestFilterAppLink code was adapted from [ac's AppLinksInterceptor](https://searchfox.org/mozilla-mobile/rev/9632fc5c1e5c99c4bb9d189f33a2c136866b4298/android-components/components/feature/app-links/src/main/java/mozilla/components/feature/app/links/AppLinksInterceptor.kt#72-112) logic, which I found to be really challenging to do. To minimize possibly added bugs, I used the ac tests verbatim with a shim layer: however, this runs on robolectric (to allow us to run verbatim) so I'm not sure we want to keep it. We should also see if there are more tests to write.

In addition to the "Unsolved challenges" in the proposal doc and the TODOs in the code, here is one other TODO item. In the current implementation, the app link code, i.e. the slow queryIntentActivities, is run for every LoadRequestFilter that declares MATCH_APP_LINK to true: this doesn't affect fenix because it only declares one but we may want to cache the results.

Keep in mind, **to get the full benefits for this patch, we also need to [make changes in fenix/ac](https://github.com/mozilla-mobile/fenix/issues/21530#issuecomment-938954112)** that prevent us from running into the same root cause: posting to the UI thread.

I can continue to do the performance measurements and I'm around if you have questions. Thank you!

Back to Bug 1734923 Comment 22