Skip to content

Conversation

@mokagio
Copy link
Contributor

@mokagio mokagio commented Nov 22, 2022

While working on #19635, Xcode reported:

Thread Performance Checker: -[AVCaptureSession startRunning] should be called from background thread. Calling it on the main thread can lead to UI unresponsiveness

image

Testing

I applied the code and run through the Scan QR code flow (visit this page on the desktop browser) and verified the TSAN warning was gone.

As an aside, I've been getting the following error, but I'm 99.9% sure it's unrelated to the code change: Update The reason was that I was using an A8c account, for which QR code login doesn't work.

Error screenshot IMG_54F528C80EF2-1

Regression Notes

  1. Potential unintended areas of impact – N.A.
  2. What I did to test those areas of impact (or what existing automated tests I relied on) – N.A.
  3. What automated tests I added (or what prevented me from doing so) – N.A.

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

Xcode said:

> Thread Performance Checker: -[AVCaptureSession startRunning] should be
> called from background thread. Calling it on the main thread can lead to
> UI unresponsiveness

func start() {
session?.startRunning()
DispatchQueue.global(qos: .background).async { [weak self] in
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should use a serial queue to start and stop the camera session. See this Apple sample project.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Nov 23, 2022

You can test the changes in Jetpack from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr19636-09f44d8 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Nov 23, 2022

You can test the changes in WordPress from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr19636-09f44d8 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@mokagio mokagio force-pushed the mokagio/AVCaptureSession-startRunning-on-background branch from b99fbd6 to 09f44d8 Compare November 23, 2022 12:12
@mokagio mokagio enabled auto-merge November 23, 2022 12:27
@mokagio mokagio disabled auto-merge November 23, 2022 18:32
@mokagio mokagio enabled auto-merge November 23, 2022 18:32
@mokagio mokagio requested a review from crazytonyli November 23, 2022 18:32
@mokagio mokagio merged commit 5a6c3e7 into trunk Nov 24, 2022
@mokagio mokagio deleted the mokagio/AVCaptureSession-startRunning-on-background branch November 24, 2022 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants