Fix for Crash in VideoCaptureController::DoNewBufferOnIOThread

Fix for Crash in VideoCaptureController::DoNewBufferOnIOThread.

Crash reports[1] indicate that it can happen that VideoCaptureController 
receives frames while its |frame_buffer_pool_| is null.

The cause appears to be due to a race condition that I missed in both the 
original CL[2] and the reland[3], where VideoCaptureController receives a frame 
before a FrameBufferPool has been set (during device startup) or after 
FrameBufferPool has been reset to nullptr (during deivce stop). 
The race condition during startup can only happen if the device delivers frames 
on a private internal thread different from the "device thread" that calls 
AllocateAndStart()). Below are example sequences (as indicated by the numbers) 
where VideoCaptureController would receive frames while FrameBufferPool is null:

Device Startup Case

IO Thread:
1. Issue CreateAndStartDevice to Device Thread
8. Receive DeviceStarted callback
9. SetFrameBufferPool
6. Receive OnFrame message

Device Thread:
2. CreateDevice
3. StartDevice
7. Issue DeviceStarted callback to IO Thread

Capture and Delivery Thread:
4. Capture frame
5. Issue OnFrame message to IO Thread


Device Stop Case

IO Thread:
1. DoStopDevice()
2. SetFrameBufferPool(nullptr)
3. Issue StopDevice request to Device Thread
6. Receive OnFrame message

Device Thread:
4. Capture frame
5. Issue OnFrame message to IO Thread
7. StopDevice


The fix is to ignore incoming frames when FrameBufferPool is null.


BUG=681836
TEST=
  content_unittests --gtest_filter="*Video*"
  Apprtc loopback on Debug

[2] https://codereview.chromium.org/2551193002/
[3] https://codereview.chromium.org/2613793007/
[1] https://crash.corp.google.com/browse?q=custom_data.ChromeCrashProto.ptype%3D%27browser%27%20AND%20custom_data.ChromeCrashProto.magic_signature_1.name%3D%27content%3A%3AVideoCaptureController%3A%3ADoNewBufferOnIOThread%27%20AND%20product.name%3D%27Chrome%27&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D#samplereports:5,productversion:1000

Review-Url: https://codereview.chromium.org/2643463002
Cr-Commit-Position: refs/heads/master@{#444168}
Committed: https://chromium.googlesource.com/chromium/src/+/b00a589e9a0e0fee86866d21e8fcf4991e82b468

[chfremer, 2017-01-17 20:37:11]

Description was changed from

==========
Fix for Crash in VideoCaptureController::DoNewBufferOnIOThread
https://bugs.chromium.org/p/chromium/issues/detail?id=681836

BUG=
==========

to

==========
Fix for Crash in VideoCaptureController::DoNewBufferOnIOThread.

Crash reports[1] indicate that it can happen that VideoCaptureController 
receives frames while its |frame_buffer_pool_| is null.

The cause appears to be due to a race condition that I missed in both the 
original CL[2] and the reland[3], where VideoCaptureController receives a frame 
before a FrameBufferPool has been set (during device startup) or after 
FrameBufferPool has been reset to nullptr (during deivce stop). 
The race condition during startup can only happen if the device delivers frames 
on a private internal thread different from the "device thread" that calls 
AllocateAndStart()). Below are example sequences (as indicated by the numbers) 
where VideoCaptureController would receive frames while FrameBufferPool is null:

Device Startup Case

IO Thread:
1. Issue CreateAndStartDevice to Device Thread
8. Receive DeviceStarted callback
9. SetFrameBufferPool
6. Receive OnFrame message

Device Thread:
2. CreateDevice
3. StartDevice
7. Issue DeviceStarted callback to IO Thread

Capture and Delivery Thread:
4. Capture frame
5. Issue OnFrame message to IO Thread


Device Stop Case

IO Thread:
1. DoStopDevice()
2. SetFrameBufferPool(nullptr)
3. Issue StopDevice request to Device Thread
6. Receive OnFrame message

Device Thread:
4. Capture frame
5. Issue OnFrame message to IO Thread
7. StopDevice


The fix is to ignore incoming frames when FrameBufferPool is null.


BUG=681836
TEST=

[2] https://codereview.chromium.org/2551193002/
[3] https://codereview.chromium.org/2613793007/
[1]
https://crash.corp.google.com/browse?q=custom_data.ChromeCrashProto.ptype%3D%...
==========

[chfremer, 2017-01-17 20:37:21]

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

[commit-bot: I haz the power, 2017-01-17 20:37:50]

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

[chfremer, 2017-01-17 20:39:05]

Description was changed from

==========
Fix for Crash in VideoCaptureController::DoNewBufferOnIOThread.

Crash reports[1] indicate that it can happen that VideoCaptureController 
receives frames while its |frame_buffer_pool_| is null.

The cause appears to be due to a race condition that I missed in both the 
original CL[2] and the reland[3], where VideoCaptureController receives a frame 
before a FrameBufferPool has been set (during device startup) or after 
FrameBufferPool has been reset to nullptr (during deivce stop). 
The race condition during startup can only happen if the device delivers frames 
on a private internal thread different from the "device thread" that calls 
AllocateAndStart()). Below are example sequences (as indicated by the numbers) 
where VideoCaptureController would receive frames while FrameBufferPool is null:

Device Startup Case

IO Thread:
1. Issue CreateAndStartDevice to Device Thread
8. Receive DeviceStarted callback
9. SetFrameBufferPool
6. Receive OnFrame message

Device Thread:
2. CreateDevice
3. StartDevice
7. Issue DeviceStarted callback to IO Thread

Capture and Delivery Thread:
4. Capture frame
5. Issue OnFrame message to IO Thread


Device Stop Case

IO Thread:
1. DoStopDevice()
2. SetFrameBufferPool(nullptr)
3. Issue StopDevice request to Device Thread
6. Receive OnFrame message

Device Thread:
4. Capture frame
5. Issue OnFrame message to IO Thread
7. StopDevice


The fix is to ignore incoming frames when FrameBufferPool is null.


BUG=681836
TEST=

[2] https://codereview.chromium.org/2551193002/
[3] https://codereview.chromium.org/2613793007/
[1]
https://crash.corp.google.com/browse?q=custom_data.ChromeCrashProto.ptype%3D%...
==========

to

==========
Fix for Crash in VideoCaptureController::DoNewBufferOnIOThread.

Crash reports[1] indicate that it can happen that VideoCaptureController 
receives frames while its |frame_buffer_pool_| is null.

The cause appears to be due to a race condition that I missed in both the 
original CL[2] and the reland[3], where VideoCaptureController receives a frame 
before a FrameBufferPool has been set (during device startup) or after 
FrameBufferPool has been reset to nullptr (during deivce stop). 
The race condition during startup can only happen if the device delivers frames 
on a private internal thread different from the "device thread" that calls 
AllocateAndStart()). Below are example sequences (as indicated by the numbers) 
where VideoCaptureController would receive frames while FrameBufferPool is null:

Device Startup Case

IO Thread:
1. Issue CreateAndStartDevice to Device Thread
8. Receive DeviceStarted callback
9. SetFrameBufferPool
6. Receive OnFrame message

Device Thread:
2. CreateDevice
3. StartDevice
7. Issue DeviceStarted callback to IO Thread

Capture and Delivery Thread:
4. Capture frame
5. Issue OnFrame message to IO Thread


Device Stop Case

IO Thread:
1. DoStopDevice()
2. SetFrameBufferPool(nullptr)
3. Issue StopDevice request to Device Thread
6. Receive OnFrame message

Device Thread:
4. Capture frame
5. Issue OnFrame message to IO Thread
7. StopDevice


The fix is to ignore incoming frames when FrameBufferPool is null.


BUG=681836
TEST=
  content_unittests --gtest_filter="*Video*"
  Apprtc loopback on Debug

[2] https://codereview.chromium.org/2551193002/
[3] https://codereview.chromium.org/2613793007/
[1]
https://crash.corp.google.com/browse?q=custom_data.ChromeCrashProto.ptype%3D%...
==========

[chfremer, 2017-01-17 20:42:26]

chfremer@chromium.org changed reviewers:
+ emircan@chromium.org, mcasas@chromium.org, miu@chromium.org

[chfremer, 2017-01-17 20:42:27]

miu@: PTAL
mcasas@, emircan@: FYI

[miu, 2017-01-17 22:00:20]

lgtm

[commit-bot: I haz the power, 2017-01-17 22:25:31]

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

[commit-bot: I haz the power, 2017-01-17 22:25:32]

Dry run: This issue passed the CQ dry run.

[chfremer, 2017-01-17 22:29:52]

The CQ bit was checked by chfremer@chromium.org

[commit-bot: I haz the power, 2017-01-17 22:30:30]

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, 2017-01-17 22:37:16]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1484692192893310, "parent_rev":
"22745f53a9c75a605242393019cb1b9a87d8e688", "commit_rev":
"b00a589e9a0e0fee86866d21e8fcf4991e82b468"}

[commit-bot: I haz the power, 2017-01-17 22:37:50]

Description was changed from

==========
Fix for Crash in VideoCaptureController::DoNewBufferOnIOThread.

Crash reports[1] indicate that it can happen that VideoCaptureController 
receives frames while its |frame_buffer_pool_| is null.

The cause appears to be due to a race condition that I missed in both the 
original CL[2] and the reland[3], where VideoCaptureController receives a frame 
before a FrameBufferPool has been set (during device startup) or after 
FrameBufferPool has been reset to nullptr (during deivce stop). 
The race condition during startup can only happen if the device delivers frames 
on a private internal thread different from the "device thread" that calls 
AllocateAndStart()). Below are example sequences (as indicated by the numbers) 
where VideoCaptureController would receive frames while FrameBufferPool is null:

Device Startup Case

IO Thread:
1. Issue CreateAndStartDevice to Device Thread
8. Receive DeviceStarted callback
9. SetFrameBufferPool
6. Receive OnFrame message

Device Thread:
2. CreateDevice
3. StartDevice
7. Issue DeviceStarted callback to IO Thread

Capture and Delivery Thread:
4. Capture frame
5. Issue OnFrame message to IO Thread


Device Stop Case

IO Thread:
1. DoStopDevice()
2. SetFrameBufferPool(nullptr)
3. Issue StopDevice request to Device Thread
6. Receive OnFrame message

Device Thread:
4. Capture frame
5. Issue OnFrame message to IO Thread
7. StopDevice


The fix is to ignore incoming frames when FrameBufferPool is null.


BUG=681836
TEST=
  content_unittests --gtest_filter="*Video*"
  Apprtc loopback on Debug

[2] https://codereview.chromium.org/2551193002/
[3] https://codereview.chromium.org/2613793007/
[1]
https://crash.corp.google.com/browse?q=custom_data.ChromeCrashProto.ptype%3D%...
==========

to

==========
Fix for Crash in VideoCaptureController::DoNewBufferOnIOThread.

Crash reports[1] indicate that it can happen that VideoCaptureController 
receives frames while its |frame_buffer_pool_| is null.

The cause appears to be due to a race condition that I missed in both the 
original CL[2] and the reland[3], where VideoCaptureController receives a frame 
before a FrameBufferPool has been set (during device startup) or after 
FrameBufferPool has been reset to nullptr (during deivce stop). 
The race condition during startup can only happen if the device delivers frames 
on a private internal thread different from the "device thread" that calls 
AllocateAndStart()). Below are example sequences (as indicated by the numbers) 
where VideoCaptureController would receive frames while FrameBufferPool is null:

Device Startup Case

IO Thread:
1. Issue CreateAndStartDevice to Device Thread
8. Receive DeviceStarted callback
9. SetFrameBufferPool
6. Receive OnFrame message

Device Thread:
2. CreateDevice
3. StartDevice
7. Issue DeviceStarted callback to IO Thread

Capture and Delivery Thread:
4. Capture frame
5. Issue OnFrame message to IO Thread


Device Stop Case

IO Thread:
1. DoStopDevice()
2. SetFrameBufferPool(nullptr)
3. Issue StopDevice request to Device Thread
6. Receive OnFrame message

Device Thread:
4. Capture frame
5. Issue OnFrame message to IO Thread
7. StopDevice


The fix is to ignore incoming frames when FrameBufferPool is null.


BUG=681836
TEST=
  content_unittests --gtest_filter="*Video*"
  Apprtc loopback on Debug

[2] https://codereview.chromium.org/2551193002/
[3] https://codereview.chromium.org/2613793007/
[1]
https://crash.corp.google.com/browse?q=custom_data.ChromeCrashProto.ptype%3D%...

Review-Url: https://codereview.chromium.org/2643463002
Cr-Commit-Position: refs/heads/master@{#444168}
Committed:
https://chromium.googlesource.com/chromium/src/+/b00a589e9a0e0fee86866d21e8fc...
==========

[commit-bot: I haz the power, 2017-01-17 22:37:51]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/b00a589e9a0e0fee86866d21e8fc...