[Bug] Remove flow and use callbacks instead. I know, pain.

[notandyvee]

Fixes Sentry issue - Github Issue

There is a bug where scanning for a QR code causes an "Image is already closed" IllegalStateException. As it turns out this occurs anytime you are mid scan and you go onPause. Coming back causes the crash 100% of the time.

During my investigation I partially learned why the crash occurred. During the ImageAnalysis.Analyzer we are grabbing the ImageProxy and scanning it. Before doing the actual scanning though, we create a callback flow. When the user puts the app in the background and comes back, the old frames from the flow are still present. The reason this is a problem is because the ImageProxy is closed. When we come back the flow is restarted, and BAM, crash.

I attempted multiple different things to make the flow work as if it was on the main thread. I couldn't find a way to throw away flow "events" currently awaiting processing. Why would we throw any away? According to the documentation for Image Analysis, our app should be analyzing frames in under 32ms, and then closing the ImageProxy. The expectation is that this be done quickly and efficiently in the same thread.

I should note that a try/catch does work in solving this problem while changing minimal code. But it's not the right approach. I couldn't figure out a solution while still using flows since there exists a requirement to analyze images quickly without the need to process them in the background. So I did the unthinkable... I went to good ol' callbacks 😅 . I removed the flows and kept everything in the same thread. And surprisingly it works. Using a Pixel 4 XL it ran fairly smooth. I tested logging in with QR code and it still works fine.

I should note I ran a quick test to figure out how long the processing takes. With the exception of the initial process, which takes the longest, all other images take under 15ms to process. Meaning we shouldn't notice any performance issues.

If you have a better solution that avoids the crash while still using flows, I'd be interested in hearing about it. Good learning opportunity.

---

To Test:

Pre-req: You need to be logged in without a admin account or any 2FA. So use a test account.

• On web while not logged into wordpress.com, select the option to login via app.
• On your phone install this build and go to the Me tab.
• Select Scan Login Code.
• Before scanning the barcode background the app or lock the screen and go back while the camera is running.
• Assuming no crash occurs, then scan the barcode.
• If you successfully logged in on web with the barcode this was successful.

---

Regression Notes

1. Potential unintended areas of impact

   Scanning barcode could be slow.

2. What I did to test those areas of impact (or what existing automated tests I relied on)

   Manual testing.

3. What automated tests I added (or what prevented me from doing so)

   N/A

---

PR Submission Checklist:

• I have completed the Regression Notes.
• I have considered adding accessibility improvements for my changes.
• I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

[sonarcloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[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	pr20870-3329d74
	Commit	3329d74
	Direct Download	wordpress-prototype-build-pr20870-3329d74.apk

Note: Google Login is not supported on these builds.

[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	pr20870-3329d74
	Commit	3329d74
	Direct Download	jetpack-prototype-build-pr20870-3329d74.apk

Note: Google Login is not supported on these builds.

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 33 lines in your changes are missing coverage. Please review.

Project coverage is 40.70%. Comparing base (c3decbc) to head (3329d74).
Report is 340 commits behind head on trunk.

Files	Patch %	Lines
...ndroid/ui/barcodescanner/GoogleMLKitCodeScanner.kt	0.00%	18 Missing ⚠️
...dpress/android/ui/barcodescanner/BarcodeScanner.kt	0.00%	15 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##            trunk   #20870   +/-   ##
=======================================
  Coverage   40.70%   40.70%           
=======================================
  Files        1499     1499           
  Lines       68763    68756    -7     
  Branches    11366    11367    +1     
=======================================
  Hits        27990    27990           
+ Misses      38240    38233    -7     
  Partials     2533     2533           

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

[notandyvee]

Hey @zwarm , I added you since you worked on this feature and migrated it. Your thoughts would be great.

[notandyvee]

@ravishanker mind reviewing? 🙏🏻

[ravishanker]

@notandyvee Great job on investigating, and effort in converting it to Callbacks. Let's get @zwarm input on this change if she might have a better idea, and whether she's OK with the change.

[zwarm]

@notandyvee

Thanks for the great detective work. 👏

Like you, I was able to keep the flow logic by making a few changes to GoogleMLKitCodeScanner and BarcodeScanningFragment, but it's not ideal because the user would have to relaunch the scanner upon on return to the app (guess it's better than the app crashing).

However your solution works like a charm, so +1 on moving forward with it. I'm approving, but leave the merge for you after @ravishanker gives the 👍 .

[ravishanker]

Great job. LGTM 👍