Remove strong reference cycle in PhotoCaptureViewController

[hakloev]

Why

Due to a strong reference cycle in PhotoCaptureViewController it's never deallocated. This makes the AVCaptureSession in CaptureManager persist. The consequence of this, is that the camera indicator introduced in iOS 14 will indicate that the application is using the camera, even after the view controller is closed.

What

Identified that the strong reference cycle were in the observer for orientation changes and replaced it with a selector pattern instead. Could have captured the reference cycle in the block, but felt the selector pattern were cleaner and safer.

Debugging

The first screenshot shows the reference cycle in PhotoCaptureViewController in the memory graph debugger. The second shows that there's no reference to Finjinon after the view controller is closed.

Before	After

[fespinoza]

I haven't use much the memory debugger... sounds like a nice demo for the iOS meetup :D

[hakloev]

This shows the importance of remembering to remove observeres when dismissing view controllers 🙂

[osanoj]

This shows the importance of remembering to remove observeres when dismissing view controllers 🙂

I don't think you actually need to remove observers anymore (but in general it feels better to do it). In this case the observer used self (without adding weak to it) so self would then never be deallocated... Right?

Actually when I think about it I guess it is wrong to setup the notification observer in viewDidLoad and remove it in viewDidDisappear if someone would reuse the view object you would only get the notifications observed the first time...

[hakloev]

Yes, you're right about the observer. That's why I said we could've captured the reference cycle in the block.

I looked at it once more, and your second point is also correct. Actually the documentation from Apple recommends removing observers in deinit. I'll move it 👍

[osanoj]

Yes, you're right about the observer. That's why I said we could've captured the reference cycle in the block.

I looked at it once more, and your second point is also correct. Actually the documentation from Apple recommends removing observers in deinit. I'll move it 👍

Remember to also weakify the reference to self then, so deinit actually will be called 😂