Commit 735f0792 authored by mcasas@chromium.org's avatar mcasas@chromium.org

Mac AVFoundation: check -CrAVCaptureDevice::observationInfo before removing observers.

During Chrome shutdown, DeviceMonitorMac deallocs
CrAVFoundationDeviceObserver, which is registered as an
observer of suspended and connected events on the
CrAVCaptureDevice. Every so seldom (see bugs), the
removal of these observers causes a crash. Observers
removal seems to be a long standing problem with
Cocoa and Objective-C++.

This CL speculates with a solution for this trouble
via testing if the protocol method -observationInfo [1]
is not |nil| before removing the observers. This is
based on the observation that:
a) There might be an AVFoundation-internal race
between observer removal and AVCaptureDevice destruction.
b) A previous, unrelated Chrome crash might have
left the AVFoundation into some unstable meta state.
(This is based on xians@ hitting this bug during
browser_tests).

Tested locally and seems to work fine.

BUG=288562, 371271


[1] https://developer.apple.com/library/ios/documentation/Cocoa/Reference/Foundation/Protocols/NSKeyValueObserving_Protocol/Reference/Reference.html#jumpTo_10

Review URL: https://codereview.chromium.org/274073002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@269603 0039d316-1c4b-4281-b951-d872f2087c98
parent 7ed0b916
......@@ -378,10 +378,15 @@ void AVFoundationMonitorImpl::StartObserverOnDeviceThread() {
std::set<CrAVCaptureDevice*>::iterator found =
std::find(monitoredDevices_.begin(), monitoredDevices_.end(), device);
DCHECK(found != monitoredDevices_.end());
[device removeObserver:self
forKeyPath:@"suspended"];
[device removeObserver:self
forKeyPath:@"connected"];
// Every so seldom, |device| might be gone when getting here, in that case
// removing the observer causes a crash. Try to avoid it by checking sanity of
// the |device| via its -observationInfo. http://crbug.com/371271.
if ([device observationInfo]) {
[device removeObserver:self
forKeyPath:@"suspended"];
[device removeObserver:self
forKeyPath:@"connected"];
}
monitoredDevices_.erase(found);
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment