VideoCaptureHost: Remove |controllers_| entry before calling VideoCaptureManager::StopCaptureForCli…

VideoCaptureHost: Remove |controllers_| entry before calling VideoCaptureManager::StopCaptureForClient() to prevent a race

The destruction sequence in case of error implies
several walks around VideoCaptureManager and
MediaStreamManager. I believe the changes introduced
by mojo have made the UAF in the bug more evident.

In any case, this CL prevents that cycle by removing
the entry from |controllers_| before requesting
something from the VideoCaptureManager.

BUG=654199
Committed: https://crrev.com/92d59c5c7195bc11f72c6d2a965cbe8ea571e455
Cr-Commit-Position: refs/heads/master@{#424210}

[mcasas, 2016-10-10 18:14:27]

mcasas@chromium.org changed reviewers:
+ xianglu@chromium.org

[mcasas, 2016-10-10 18:14:27]

xianglu@ PTAL

[mcasas, 2016-10-10 18:14:32]

The CQ bit was checked by mcasas@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-10 18:14:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-10 19:04:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-10 19:04:04]

Dry run: This issue passed the CQ dry run.

[xianglu, 2016-10-10 19:45:14]

lgtm.

[mcasas, 2016-10-10 19:47:43]

The CQ bit was checked by mcasas@chromium.org

[commit-bot: I haz the power, 2016-10-10 19:48:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-10 19:53:43]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-10-10 19:55:53]

Description was changed from

==========
VideoCaptureHost: Remove |controllers_| entry before calling
VideoCaptureManager::StopCaptureForClient() to prevent a race

The destruction sequence in case of error implies
several walks around VideoCaptureManager and
MediaStreamManager. I believe the changes introduced
by mojo have made the UAF in the bug more evident.

In any case, this CL prevents that cycle by removing
the entry from |controllers_| before requesting
something from the VideoCaptureManager.

BUG=654199
==========

to

==========
VideoCaptureHost: Remove |controllers_| entry before calling
VideoCaptureManager::StopCaptureForClient() to prevent a race

The destruction sequence in case of error implies
several walks around VideoCaptureManager and
MediaStreamManager. I believe the changes introduced
by mojo have made the UAF in the bug more evident.

In any case, this CL prevents that cycle by removing
the entry from |controllers_| before requesting
something from the VideoCaptureManager.

BUG=654199

Committed: https://crrev.com/92d59c5c7195bc11f72c6d2a965cbe8ea571e455
Cr-Commit-Position: refs/heads/master@{#424210}
==========

[commit-bot: I haz the power, 2016-10-10 19:55:54]

Patchset 1 (id:??) landed as
https://crrev.com/92d59c5c7195bc11f72c6d2a965cbe8ea571e455
Cr-Commit-Position: refs/heads/master@{#424210}