Fix ANR getting default user agent

[nbradbury]

Fixes #21679
Fixes #20147

We have a long-standing ANR caused by reading WebSettings.getDefaultUserAgent on the main thread. As noted in this comment on the Google issue:

WebSettings.getDefaultUserAgent() is intentionally designed to load WebView code...because this is how we determine what the full user agent string actually is.

this may seem very slow for your use case, but this is not something which we can optimize much further. WebView has a large amount of code because it contains a full copy of the chromium browsing engine, so this requires a lot of time to load and initialize.

To work around this problem, this PR first sets the global user agent to the app name & version, then sets the global user agent by calling getDefaultUserAgent in a separate thread. Note that this occurs when UserAgent.initialize is called, which happens during app start.

Because of this, there is a non-zero chance that a network call may be made with the app name & version as the user agent (ie: before the background thread to getDefaultUserAgent completes), but this is unlikely, and regardless it's better than having hundreds of users affected by the ANR.

I'm unsure of what testing steps to recommend, other than setting a breakpoint here to ensure we're setting the user agent correctly. It may also help to add the line below above this line.

delay(5000)

This will cause the app to delay 5000ms before retrieving the user agent, which will verify that it is actually being retrieved on a background thread instead of on the main thread because the app doesn't hang.

[dangermattic]

	1 Message
📖	This PR is still a Draft: some checks will be skipped.

Generated by 🚫 Danger

[wpmobilebot]

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

	App Name	Jetpack
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr21681-3ac50ab
	Commit	3ac50ab
	Direct Download	jetpack-prototype-build-pr21681-3ac50ab.apk

Note: Google Login is not supported on these builds.

[wpmobilebot]

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

	App Name	WordPress
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr21681-3ac50ab
	Commit	3ac50ab
	Direct Download	wordpress-prototype-build-pr21681-3ac50ab.apk

Note: Google Login is not supported on these builds.

[codecov]

Codecov Report

Attention: Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 39.30%. Comparing base (93ff765) to head (3ac50ab).

Files with missing lines	Patch %	Lines
...a/org/wordpress/android/fluxc/network/UserAgent.kt	90.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##            trunk   #21681      +/-   ##
==========================================
- Coverage   39.41%   39.30%   -0.11%     
==========================================
  Files        2122     2116       -6     
  Lines       99570    99391     -179     
  Branches    15324    15316       -8     
==========================================
- Hits        39247    39068     -179     
  Misses      56844    56844              
  Partials     3479     3479              

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

[hichamboushaba]

@nbradbury just a FYI that we had a similar fix done in FluxC few months ago (wordpress-mobile/WordPress-FluxC-Android#3067), then we had to revert it (wordpress-mobile/WordPress-FluxC-Android#3101) as it led to some worse crashes.

Please review the above before proceeding with the merge.

[nbradbury]

@hichamboushaba Thanks for the background, that's very helpful! I'm going to revert this to a draft until I've had time to fully look into this.

[ParaskP7]

I'm going to revert this to a draft until I've had time to fully look into this.

FYI: @nbradbury I started looking into that, but since you reverted it I'll now stop until until further notice. ⏱️

[ParaskP7]

FYI: I just removed myself from this draft PR, just so to not have this on my review-requested GH view. Feel free to add me back to it when and if need-be.

[nbradbury]

@hichamboushaba My understanding is that the issue with the FluxC fix was that moving WebSettings.getDefaultUserAgent to a background thread caused multiple WebView processes to be created. I would think making UserAgent a singleton would resolve that, but it appears that was done for WCAndroid and then reverted?

[hichamboushaba]

@hichamboushaba My understanding is that the issue with wordpress-mobile/WordPress-FluxC-Android#3067 was that moving WebSettings.getDefaultUserAgent to a background thread caused multiple WebView processes to be created. I would think making UserAgent a singleton would resolve that, but it appears that was done for WCAndroid and then reverted?

We still use use Singleton for the UserAgent in WCAndroid, the revert was just for the example app (just because I wanted to keep the revert PR as simple as possible).
Even with being singleton, we got the crashes, we didn't spend much time investigating this after reverting it, but it seems to me the race condition could be happening between WebSettings.getDefaultUserAgent and creating the WebView instances for the app, so if we launch the app directly to a screen that has a WebView 🤔.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud