Fix UserAgent ANR - Take 2

[hichamboushaba]

Closes WOOMOB-968

Description

This PR is a second attempt to try to fix the UserAgent ANR, as a reminder, this is an ANR that occurs when calling WebSettings.getDefaultUserAgent, we call this on the UserAgent#init, which happens on app launch, and sometimes it results in a background ANR. This is a known issue for Google, as the call is heavy, and they suggest making it on a background thread to avoid blocking the main thread, something that we tried on the first attempt, but it resulted in another WebView crash (peaMlT-Tk-p2), and we had to revert our fix.

My unproved theory for the crash is that the usage of the background thread in the UserAgent#init increased the chances for the stuck process situation that's explained here, and thus leading the AwDataDirLock crashes.

Now, we need to take a different approach for the fix, and I'll list the options we have to discuss and pick the better one:

Option 1: Use two UserAgent variants, one for API requests and one for WebView

My understanding is that for the API requests, the most important part of the UserAgent is just the app name and version, as the other parts of the UserAgent are more important when viewing HTML content where the web server might need to adapt the content depending on the WebView capabilities.
So based on the above, the idea here is to use two UserAgent variants:

• One for the API requests, it will use the VM property http.agent, this is the default UserAgent of the device before adding the WebView parts, and it's the default value used by HttpUrlConnection
• For the WebView, we'll keep using WebSettings.getDefaultUserAgent as then it will be called on foreground when the WebView is being initialized, and it will generally be fine.

For comparison, with this change, and with an emulator running Android 15, we'll use the following values:

• apiUserAgent='Dalvik/2.1.0 (Linux; U; Android 15; sdk_gphone64_arm64 Build/AE3A.240806.043) wc-android/22.9-rc-2'
• webViewUserAgent='Mozilla/5.0 (Linux; Android 15; sdk_gphone64_arm64 Build/AE3A.240806.043; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/124.0.6367.219 Mobile Safari/537.36 wc-android/22.9-rc-2'

Option 2: Use SharedPreferences for caching the UserAgent.

(This was the initial approach I used in this PR; check it in commit c74f359. After further research on AwDataDirLock and considering the shared theory above, I believe this could cause the same crashes. Sharing for discussion only)
In this approach, SharedPreferences will serve as a cache for the value. The plan is to load the initial value from SharedPreferences and then, after a set delay, update the cache (in case WebView has been updated).
The key factor in this fix is the delay before calling WebSettings.getDefaultUserAgent. When crashes related to AwDataDirLock occurred, we believe they mainly happened during app startup, as there were no encrypted logs available (edit: still confused about the lack of logs, but I'm not convinced it means necessarily app startup).

Option 3: Use SharedPreferences for caching the UserAgent 2

(This is a third option that's similar to Option 2, but which could be more robust, I didn't implement it just because Option 1 seemed simpler, I can implement it if we believe keeping the same UserAgent value for both API requests and the WebView is beneficial.)
In this option, we'll use the SharedPreferences as cache, but we'll make sure to call WebSettings.getDefaultUserAgent only when the app is going to foreground, when the app is going to foreground, there are less chances of keeping the process stuck, as the app will be given higher priority by the system. To achieve this, we can use ProcessLifecycleOwner and invoke the loading of the UserAgent when the app reaches the Started state.

@JorgeMucientes @malinajirka @wzieba pinging you as you have more context on this issue given the discussions on Linear, please share your thoughts on the suggested approaches.

Testing information

API requests

1. Use a tool to inspect network requests (App Inspection from Android Studio or Flipper)
2. Launch the app.
3. Check some requests and confirm they have the expected UserAgent, in the format <http.agent> wc-android/<version>

WebView

1. Open Blaze campaign creation screen.
2. Enter all details and tap on confirm.
3. Tap on the payment method button.
4. Tap on Add a new payment method.
5. Confirm the WebView loads as expected and that no nav bar is shown (I mean the Calypso nav bar)

The tests that have been performed

The above.

• I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

[dangermattic]

	2 Warnings
⚠️	View files have been modified, but no screenshot or video is included in the pull request. Consider adding some for clarity.
⚠️	This PR is assigned to the milestone 23.1. This milestone is due in less than 2 days. Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by 🚫 Danger

[hichamboushaba]

The unit tests are broken now, I updated them when I implemented Option 1, but now they will fail, I will fix then when we agree on the approach.

[wpmobilebot]

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce-Wear Android
	Platform	⌚️ Wear OS
	Flavor	Jalapeno
	Build Type	Debug
	Commit	4ad7b33
	Direct Download	woocommerce-wear-prototype-build-pr14431-4ad7b33.apk

[wpmobilebot]

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce Android
	Platform	📱 Mobile
	Flavor	Jalapeno
	Build Type	Debug
	Commit	4ad7b33
	Direct Download	woocommerce-prototype-build-pr14431-4ad7b33.apk

[wzieba]

the idea here is to use two UserAgent variants:

This sounds good to me 👍

I can't say I fully understood the AwDataDirLock issue (I read the attached comment from the tracker, but still) or how moving to the background thread could increase this, but having two user agents sound to me like a completely valid approach to test.

[malinajirka]

I forgot to reply yesterday 🤦‍♂️.

Thanks for clearly summarizing your findings @hichamboushaba! I also think testing the two-agents approach is worth a shot.

[JorgeMucientes]

Same as shared by Wojtek. I didn't really get the reasons why the last time moving the UserAgent initialization to the background led to the crashes. In any case, the 2 userAgents approach sounds like a good approach to test.

[hichamboushaba]

I can't say I fully understood the AwDataDirLock issue (I read the attached comment from the tracker, but still) or how moving to the background thread could increase this

Thank you all for the input, just regarding this, I'll try to explain further my theory here.
In our app, we use WorkManager to handle some background tasks, these background tasks use the NetworkType.CONNECTED constraint, so according to my theory, what could happen is that when the network is unstable, then this could happen:

1. WorkManager starts the execution of a task, which will trigger the background thread for getting the UserAgent.
2. Network disconnects quickly after, and WorkManager stops the Worker, then reschedule it for when Network connects again.
3. For some reason, the process gets stuck (as discussed in the above issue)
4. Network gets connected again, and the Worker is launched.
5. Android starts a new process, and we launch a new background thread for getting the UserAgent.
6. AwDataDirLock exception is thrown as we have now two processes accessing the same data dir.

This is just a theory, and I can't prove it, but it seems to match what we had, as all the crashes happened after a NETWORK_AVAILABLE event (as mentioned here peaMlT-Tk-p2#comment-2286).

---

The PR is now ready for review.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 0% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 37.92%. Comparing base (895a038) to head (4ad7b33).
⚠️ Report is 43 commits behind head on trunk.

Files with missing lines	Patch %	Lines
...a/org/wordpress/android/fluxc/network/UserAgent.kt	0.00%	14 Missing ⚠️
...erce/android/ui/compose/component/web/WCWebView.kt	0.00%	1 Missing ⚠️
...pplicationpasswords/ApplicationPasswordsNetwork.kt	0.00%	1 Missing ⚠️
...onpasswords/WPApiApplicationPasswordsRestClient.kt	0.00%	1 Missing ⚠️
...pcom/jetpackai/JetpackAITranscriptionRestClient.kt	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk   #14431      +/-   ##
============================================
- Coverage     37.92%   37.92%   -0.01%     
  Complexity     9329     9329              
============================================
  Files          2015     2015              
  Lines        113218   113222       +4     
  Branches      14984    14985       +1     
============================================
  Hits          42942    42942              
- Misses        66360    66364       +4     
  Partials       3916     3916              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[JorgeMucientes]

Is there any reason to keep this? Its unused.

[hichamboushaba]

Are you sure it's really unused? I left it because I'm not entirely sure it's not used somewhere, AS find usages doesn't work well here, because it's an overriden function.

If we can confirm it's unused, I also prefer to have a better toString implementation here, or to get rid of the implementation completely.

[JorgeMucientes]

I can't 100% confirm but back when I look at this what I did was "Fin in Files" and did the following searches:

• agent.toS
• Agent.to
• gent.to

And none of those searches showed any matches. So I presumed there were no usages of this implementation of toString. However this method doesn't confirm 100% there aren't any uses.

[hichamboushaba]

I found out that Intellij Idea "Find Usages" have an option dialog that allows disabling finding Base functions:

And this confirmed there is no usages at all, and with this confirmation I removed it in 4ad7b33

[JorgeMucientes]

Great work @hichamboushaba, everything works as expected and code looks good. I just left a minor suggestion but nothing blocking.