Remove D3D KeyedMutex Path in Media Capture patch [chromium/src : main]
Shaobo Yan (Gerrit)
Shaobo Yan added 1 comment![]( "Open in Gerrit")
Related details
- Sunny Sachanandani
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sunny Sachanandani (Gerrit)
Sunny Sachanandani voted and added 1 comment![]( "Open in Gerrit")
Votes added by Sunny Sachanandani
| Code-Review | +1 |
1 comment
lgtm from me, but it requires OWNERS approvals from media folks
Related details
- Shaobo Yan
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Shaobo Yan (Gerrit)
Shaobo Yan added 1 comment![]( "Open in Gerrit")
- Ilya Nikolaevskiy
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jianlin Qiu (Gerrit)
Jianlin Qiu added 1 comment![]( "Open in Gerrit")
At https://source.chromium.org/chromium/chromium/src/+/main:media/capture/video/fake_video_capture_device.cc;l=956;drc=6099daedae9edf5b9ccbfb8267e108aaaf87371b we copy from mapped buffer to GMB GPU texture. If you remove the keyed_mutex without proper sync for fake GMB capture, VEA will get artifacts.
Related details
- Ilya Nikolaevskiy
- Shaobo Yan
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Shaobo Yan (Gerrit)
Shaobo Yan added 1 comment![]( "Open in Gerrit")
At https://source.chromium.org/chromium/chromium/src/+/main:media/capture/video/fake_video_capture_device.cc;l=956;drc=6099daedae9edf5b9ccbfb8267e108aaaf87371b we copy from mapped buffer to GMB GPU texture. If you remove the keyed_mutex without proper sync for fake GMB capture, VEA will get artifacts.
Thanks Johnny@ for this info. I'll see what I can do for fake capture.
Related details
- Ilya Nikolaevskiy
- Jianlin Qiu
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ilya Nikolaevskiy (Gerrit)
Ilya Nikolaevskiy added 1 comment![]( "Open in Gerrit")
You need to update some more places.
1) FakeVideoCaptureDevice [1] relies on this function [2] to copy the data (the call goes through [3]).
It uses keyed mutex. The real VideoCaptureDeviceMfWin waits for the copy to be completed, but the fake one doesn't. You need to add some synchronisation there.
2) Search for places where keyed mutex is used, like [4]. Maybe it should be removed there too?
Related details
- Jianlin Qiu
- Shaobo Yan
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Shaobo Yan (Gerrit)
Shaobo Yan voted and added 1 comment![]( "Open in Gerrit")
Votes added by Shaobo Yan
| Commit-Queue | +1 |
1 comment
sunnyps@,ilnik@ and jianlin@, I've updated the CL to clean more keyed_mutex, including fake_camera. Pls help to take a look, thanks!
Related details
- Qiu, Jianlin
- Sunny Sachanandani
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Qiu, Jianlin (Gerrit)
Qiu, Jianlin added 1 comment![]( "Open in Gerrit")
sunnyps@,ilnik@ and jianlin@, I've updated the CL to clean more keyed_mutex, including fake_camera. Pls help to take a look, thanks!
Please refer to my comment for PS5. The fake camera device is not using the Mock device.
Related details
- Shaobo Yan
- Sunny Sachanandani
Shaobo Yan (Gerrit)
Shaobo Yan added 1 comment![]( "Open in Gerrit")
Qiu, Jianlinsunnyps@,ilnik@ and jianlin@, I've updated the CL to clean more keyed_mutex, including fake_camera. Pls help to take a look, thanks!
Please refer to my comment for PS5. The fake camera device is not using the Mock device.
Thanks. I think the sync is handled by:
1.buffer_access = GetHandleForInProcessAccess() — returns a DXGIGMBTrackerHandle wrapping shared memory.
2.PaintFrame() — writes frame data into shared memory (CPU-side, no GPU involved yet).
3. buffer_access.reset() — destroys DXGIGMBTrackerHandle, whose destructor calls:
CopyShMemToDXGIBuffer() → CopyMemToD3D11Tex()
UpdateSubresource() — queues GPU copy from shared memory → GPU texture.
EnqueueSetEvent() + event.Wait() — blocks the CPU thread until the GPU finishes the copy.
Only then does reset() return.
4.The frame is delivered to VEA via OnIncomingCapturedBuffer() after reset() has returned.
Am I missing something?
Related details
- Qiu, Jianlin
- Sunny Sachanandani
Ilya Nikolaevskiy (Gerrit)
Ilya Nikolaevskiy added 1 comment![]( "Open in Gerrit")
There's a new setback here: WebGPU and ImportExternalTexture.
When the call happens, it issues GPU commands and doesn't hold the video frame while the commands are executing. This leads to renderer process returning the video frame to the video captuerer, while the GPU process might still work with the texture. So now the keyed mutex is needed to protext the texture. It still might happen that the capturer will repopulate the texture with the next frame, but it will result only in a small jitter instead of tearing or glitches in the texture.
I think HW encoders also might do similar thing: hold onto texture while the video frame is already returned to the capturer, but currently HW encoder wrappers do a local copy of the texture first, exactly to combat this: https://source.chromium.org/chromium/chromium/src/+/main:media/gpu/windows/media_foundation_video_encode_accelerator_win.cc;l=2274;drc=542eedeaf4606aec2003451a8365ac5a0269e33e
There might be more similar bugs which are mitigated by using the keyed mutex.
Related details
- Qiu, Jianlin
- Shaobo Yan
- Sunny Sachanandani
Shaobo Yan (Gerrit)
Shaobo Yan added 1 comment![]( "Open in Gerrit")
There's a new setback here: WebGPU and ImportExternalTexture.
When the call happens, it issues GPU commands and doesn't hold the video frame while the commands are executing. This leads to renderer process returning the video frame to the video captuerer, while the GPU process might still work with the texture. So now the keyed mutex is needed to protext the texture. It still might happen that the capturer will repopulate the texture with the next frame, but it will result only in a small jitter instead of tearing or glitches in the texture.
I think HW encoders also might do similar thing: hold onto texture while the video frame is already returned to the capturer, but currently HW encoder wrappers do a local copy of the texture first, exactly to combat this: https://source.chromium.org/chromium/chromium/src/+/main:media/gpu/windows/media_foundation_video_encode_accelerator_win.cc;l=2274;drc=542eedeaf4606aec2003451a8365ac5a0269e33e
There might be more similar bugs which are mitigated by using the keyed mutex.
I think we already support the mechanism which called read_lock_fences_enabled to ensure gpu process returned the frame until all ops finished. https://chromium-review.googlesource.com/c/chromium/src/+/5194586
Related details
- Ilya Nikolaevskiy
- Qiu, Jianlin
- Sunny Sachanandani
Ilya Nikolaevskiy (Gerrit)
Ilya Nikolaevskiy added 1 comment![]( "Open in Gerrit")
Shaobo YanThere's a new setback here: WebGPU and ImportExternalTexture.
When the call happens, it issues GPU commands and doesn't hold the video frame while the commands are executing. This leads to renderer process returning the video frame to the video captuerer, while the GPU process might still work with the texture. So now the keyed mutex is needed to protext the texture. It still might happen that the capturer will repopulate the texture with the next frame, but it will result only in a small jitter instead of tearing or glitches in the texture.
I think HW encoders also might do similar thing: hold onto texture while the video frame is already returned to the capturer, but currently HW encoder wrappers do a local copy of the texture first, exactly to combat this: https://source.chromium.org/chromium/chromium/src/+/main:media/gpu/windows/media_foundation_video_encode_accelerator_win.cc;l=2274;drc=542eedeaf4606aec2003451a8365ac5a0269e33e
There might be more similar bugs which are mitigated by using the keyed mutex.
I think we already support the mechanism which called read_lock_fences_enabled to ensure gpu process returned the frame until all ops finished. https://chromium-review.googlesource.com/c/chromium/src/+/5194586
Are you sure this is happening in copyExternalImageToTexture[1] path too?
I can see that it wraps shared image[2] to WebGPUMailboxTexture, but i don't see how it will prolong the associated AcceleratedStaticBitmapImage lifetime.
Related details
- Qiu, Jianlin
- Shaobo Yan
- Sunny Sachanandani
Qiu, Jianlin (Gerrit)
Qiu, Jianlin added 1 comment![]( "Open in Gerrit")
Qiu, Jianlinsunnyps@,ilnik@ and jianlin@, I've updated the CL to clean more keyed_mutex, including fake_camera. Pls help to take a look, thanks!
Shaobo YanPlease refer to my comment for PS5. The fake camera device is not using the Mock device.
Thanks. I think the sync is handled by:
1.buffer_access = GetHandleForInProcessAccess() — returns a DXGIGMBTrackerHandle wrapping shared memory.
2.PaintFrame() — writes frame data into shared memory (CPU-side, no GPU involved yet).
3. buffer_access.reset() — destroys DXGIGMBTrackerHandle, whose destructor calls:
CopyShMemToDXGIBuffer() → CopyMemToD3D11Tex()
UpdateSubresource() — queues GPU copy from shared memory → GPU texture.
EnqueueSetEvent() + event.Wait() — blocks the CPU thread until the GPU finishes the copy.
Only then does reset() return.4.The frame is delivered to VEA via OnIncomingCapturedBuffer() after reset() has returned.
Am I missing something?
- Shaobo Yan
- Sunny Sachanandani
Shaobo Yan (Gerrit)
Shaobo Yan added 1 comment![]( "Open in Gerrit")
Shaobo YanThere's a new setback here: WebGPU and ImportExternalTexture.
When the call happens, it issues GPU commands and doesn't hold the video frame while the commands are executing. This leads to renderer process returning the video frame to the video captuerer, while the GPU process might still work with the texture. So now the keyed mutex is needed to protext the texture. It still might happen that the capturer will repopulate the texture with the next frame, but it will result only in a small jitter instead of tearing or glitches in the texture.
I think HW encoders also might do similar thing: hold onto texture while the video frame is already returned to the capturer, but currently HW encoder wrappers do a local copy of the texture first, exactly to combat this: https://source.chromium.org/chromium/chromium/src/+/main:media/gpu/windows/media_foundation_video_encode_accelerator_win.cc;l=2274;drc=542eedeaf4606aec2003451a8365ac5a0269e33e
There might be more similar bugs which are mitigated by using the keyed mutex.
Ilya NikolaevskiyI think we already support the mechanism which called read_lock_fences_enabled to ensure gpu process returned the frame until all ops finished. https://chromium-review.googlesource.com/c/chromium/src/+/5194586
Are you sure this is happening in copyExternalImageToTexture[1] path too?
I can see that it wraps shared image[2] to WebGPUMailboxTexture, but i don't see how it will prolong the associated AcceleratedStaticBitmapImage lifetime.
Correct. I think CopyEI2T path doesn't count in this. We need to integrate the similar mechanism for it. I'll use another CL to address this.
Related details
- Ilya Nikolaevskiy
- Sunny Sachanandani
Ilya Nikolaevskiy (Gerrit)
Ilya Nikolaevskiy added 1 comment![]( "Open in Gerrit")
Shaobo YanThere's a new setback here: WebGPU and ImportExternalTexture.
When the call happens, it issues GPU commands and doesn't hold the video frame while the commands are executing. This leads to renderer process returning the video frame to the video captuerer, while the GPU process might still work with the texture. So now the keyed mutex is needed to protext the texture. It still might happen that the capturer will repopulate the texture with the next frame, but it will result only in a small jitter instead of tearing or glitches in the texture.
I think HW encoders also might do similar thing: hold onto texture while the video frame is already returned to the capturer, but currently HW encoder wrappers do a local copy of the texture first, exactly to combat this: https://source.chromium.org/chromium/chromium/src/+/main:media/gpu/windows/media_foundation_video_encode_accelerator_win.cc;l=2274;drc=542eedeaf4606aec2003451a8365ac5a0269e33e
There might be more similar bugs which are mitigated by using the keyed mutex.
Ilya NikolaevskiyI think we already support the mechanism which called read_lock_fences_enabled to ensure gpu process returned the frame until all ops finished. https://chromium-review.googlesource.com/c/chromium/src/+/5194586
Shaobo YanAre you sure this is happening in copyExternalImageToTexture[1] path too?
I can see that it wraps shared image[2] to WebGPUMailboxTexture, but i don't see how it will prolong the associated AcceleratedStaticBitmapImage lifetime.
Correct. I think CopyEI2T path doesn't count in this. We need to integrate the similar mechanism for it. I'll use another CL to address this.
Thanks! I'll LGTM this CL once you rebase it on that CL, or land it. I think we mustn't land this before the issue with CopyExternalTexture is fixed.
Related details
- Shaobo Yan
- Sunny Sachanandani
Shaobo Yan (Gerrit)
Shaobo Yan added 1 comment![]( "Open in Gerrit")
Shaobo YanThere's a new setback here: WebGPU and ImportExternalTexture.
When the call happens, it issues GPU commands and doesn't hold the video frame while the commands are executing. This leads to renderer process returning the video frame to the video captuerer, while the GPU process might still work with the texture. So now the keyed mutex is needed to protext the texture. It still might happen that the capturer will repopulate the texture with the next frame, but it will result only in a small jitter instead of tearing or glitches in the texture.
I think HW encoders also might do similar thing: hold onto texture while the video frame is already returned to the capturer, but currently HW encoder wrappers do a local copy of the texture first, exactly to combat this: https://source.chromium.org/chromium/chromium/src/+/main:media/gpu/windows/media_foundation_video_encode_accelerator_win.cc;l=2274;drc=542eedeaf4606aec2003451a8365ac5a0269e33e
There might be more similar bugs which are mitigated by using the keyed mutex.
Ilya NikolaevskiyI think we already support the mechanism which called read_lock_fences_enabled to ensure gpu process returned the frame until all ops finished. https://chromium-review.googlesource.com/c/chromium/src/+/5194586
Shaobo YanAre you sure this is happening in copyExternalImageToTexture[1] path too?
I can see that it wraps shared image[2] to WebGPUMailboxTexture, but i don't see how it will prolong the associated AcceleratedStaticBitmapImage lifetime.
Ilya NikolaevskiyCorrect. I think CopyEI2T path doesn't count in this. We need to integrate the similar mechanism for it. I'll use another CL to address this.
Thanks! I'll LGTM this CL once you rebase it on that CL, or land it. I think we mustn't land this before the issue with CopyExternalTexture is fixed.
- Ilya Nikolaevskiy
- Sunny Sachanandani
Sunny Sachanandani (Gerrit)
Sunny Sachanandani added 4 comments![]( "Open in Gerrit")
base::WaitableEvent event(
base::WaitableEvent::ResetPolicy::AUTOMATIC,
base::WaitableEvent::InitialState::NOT_SIGNALED);Just use the default params here - we don't really care about the event being reset which is also a bit dangerous since it can lead to an infinite wait
device_context->Flush();I believe EnqueueSetEvent guarantees a flush and if it fails, it probably means the device is lost and flushing won't really do anything.
base::WaitableEvent event(base::WaitableEvent::ResetPolicy::AUTOMATIC,
base::WaitableEvent::InitialState::NOT_SIGNALED);Same here - the default params are what you want.
device_context->Flush();And here - we don't want to flush if EnqueueSetEvent fails - that means something catastrophic has happened to the D3D11 device.
Related details
- Ilya Nikolaevskiy
- Shaobo Yan
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ilya Nikolaevskiy (Gerrit)
Ilya Nikolaevskiy added 1 comment![]( "Open in Gerrit")
Shaobo YanThere's a new setback here: WebGPU and ImportExternalTexture.
When the call happens, it issues GPU commands and doesn't hold the video frame while the commands are executing. This leads to renderer process returning the video frame to the video captuerer, while the GPU process might still work with the texture. So now the keyed mutex is needed to protext the texture. It still might happen that the capturer will repopulate the texture with the next frame, but it will result only in a small jitter instead of tearing or glitches in the texture.
I think HW encoders also might do similar thing: hold onto texture while the video frame is already returned to the capturer, but currently HW encoder wrappers do a local copy of the texture first, exactly to combat this: https://source.chromium.org/chromium/chromium/src/+/main:media/gpu/windows/media_foundation_video_encode_accelerator_win.cc;l=2274;drc=542eedeaf4606aec2003451a8365ac5a0269e33e
There might be more similar bugs which are mitigated by using the keyed mutex.
Ilya NikolaevskiyI think we already support the mechanism which called read_lock_fences_enabled to ensure gpu process returned the frame until all ops finished. https://chromium-review.googlesource.com/c/chromium/src/+/5194586
Shaobo YanAre you sure this is happening in copyExternalImageToTexture[1] path too?
I can see that it wraps shared image[2] to WebGPUMailboxTexture, but i don't see how it will prolong the associated AcceleratedStaticBitmapImage lifetime.
Ilya NikolaevskiyCorrect. I think CopyEI2T path doesn't count in this. We need to integrate the similar mechanism for it. I'll use another CL to address this.
Shaobo YanThanks! I'll LGTM this CL once you rebase it on that CL, or land it. I think we mustn't land this before the issue with CopyExternalTexture is fixed.
Agree. I'll keep this CL on.
I've seen crrev.com/7614912 to fix it, thank you for CCing me there. Could you please explain to me how it ensures VideoFrame isn't released and returned to the capturer? Maybe I've missed something in my explanation below.
So, in [1] a ClientSharedImage is wrapped as WebGPU texture, and that webgpu texture will be kept alive after your change untill the webgpu copy or import completes, I agree. But that code only holds ClientSharedImage, which is basically a mailbox. It doesn't ensure the accelerated static bitmap image or media::VideoFrame is kept alive.
To my knowledge, there's no code right now which keeps VideoFrame alive while the corresponding SharedImage is alive.
So it still can happen that the Accelerated Static Bitmap Image is destroyed, the VideoFrame is destroyed, it's returned to the capturer, capturer reuses the video frame, fills the underlying D3D texture with new data (creating a data race). Then this texture will reach the renderer again.
In the first place, SharedImage lifetime isn't an issue here. The SharedImage will be alive until the capture is stopped, it's reused [2] for new frames, with the same underlying D3D texture. It will be only destroyed then the capturer is terminated [3].
However, the capturer is allowed to reuse the texture once the VideoFrame is destroyed [4]. So there needs to be some mechanism to ensure the VideoFrame isn't destoyed until the ClientSharedImage is not used anymore.
Maybe the ClientSharedImage is decoupled from VideoFrame somewhere along the path so it's impossible to hold the reference to the VideoFrame itself. Maybe it's not held by the AcceleratedStaticImageBitmap.
Then the solution is to add the destruction observer to the ClientSharedImage itself, not the VideoFrame itself in [3]. Then as long as someone holds the ClientSharedImage or the VideoFrame (which holds ClientSharedImage), the capturer will not be allowed to reuse the texture.
Now I think this also applies to the ImportExternalTexture path.
Related details
- Shaobo Yan
Shaobo Yan (Gerrit)
Shaobo Yan added 1 comment![]( "Open in Gerrit")
Thanks for take a deep look into this path! I think in the code path we do:
1. https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/graphics/gpu/webgpu_mailbox_texture.cc;drc=56c66e417c83e2096a4e4e8a5c4ab7bbd525c9f3;l=175 We use `base::RetainedRef` for the imported media::VideoFrame. Until webgpu_mailbox_texture is destroyed (https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/graphics/gpu/webgpu_mailbox_texture.cc;drc=56c66e417c83e2096a4e4e8a5c4ab7bbd525c9f3;l=242). So I think this approach ensures both shared image and media::VideoFrame are not reach refcounted zero until webgpu_mailbox_texture is destroyed. Pls correct me if this mechanism not work or not cover all cases.
2. For GPUExternalTexture(a.k.a importExternalTexture), we have a cache mechanism and hold a strong reference for media::VideoFrame. And before we release the ref object, we need to call that function to ensure all gpu commands finished. So I think importExternalTexture is safe even the mechanism in 1 is not work.
Related details
- Ilya Nikolaevskiy
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ilya Nikolaevskiy (Gerrit)
Ilya Nikolaevskiy voted and added 2 comments![]( "Open in Gerrit")
Votes added by Ilya Nikolaevskiy
| Code-Review | +1 |
You need to update some more places.
1) FakeVideoCaptureDevice [1] relies on this function [2] to copy the data (the call goes through [3]).
It uses keyed mutex. The real VideoCaptureDeviceMfWin waits for the copy to be completed, but the fake one doesn't. You need to add some synchronisation there.2) Search for places where keyed mutex is used, like [4]. Maybe it should be removed there too?
I think now that you do not remove keyed mutex acquiring comletely, but ensure that it's gated behind the check on the texture flags, this is fine.
Ah, I see, I was looking at the wrong path (CopyFromCanvas instead of CopyFromVideoElement). It seems that everything is covered here. Thanks for explanation!
Just make sure that this is landed after https://crrev.com/c/7614912
Related details
- Shaobo Yan
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Shaobo Yan (Gerrit)
Shaobo Yan added 4 comments![]( "Open in Gerrit")
base::WaitableEvent event(
base::WaitableEvent::ResetPolicy::AUTOMATIC,
base::WaitableEvent::InitialState::NOT_SIGNALED);Just use the default params here - we don't really care about the event being reset which is also a bit dangerous since it can lead to an infinite wait
Done
I believe EnqueueSetEvent guarantees a flush and if it fails, it probably means the device is lost and flushing won't really do anything.
Done
base::WaitableEvent event(base::WaitableEvent::ResetPolicy::AUTOMATIC,
base::WaitableEvent::InitialState::NOT_SIGNALED);Same here - the default params are what you want.
Done
And here - we don't want to flush if EnqueueSetEvent fails - that means something catastrophic has happened to the D3D11 device.
- Sunny Sachanandani
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Shaobo Yan (Gerrit)
Shaobo Yan added 1 comment![]( "Open in Gerrit")
- Chromium IPC Reviews
- Sunny Sachanandani
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
gwsq (Gerrit)
Message from gwsq![]( "Open in Gerrit")
From googleclient/chrome/chromium_gwsq/ipc/config.gwsq:
IPC: w...@chromium.org
📎 It looks like you’re making a possibly security-sensitive change! 📎 IPC security review isn’t a rubberstamp, so your friendly security reviewer will need a fair amount of context to review your CL effectively. Please review your CL description and code comments to make sure they provide context for someone unfamiliar with your project/area. Pay special attention to where data comes from and which processes it flows between (and their privilege levels). Feel free to point your security reviewer at design docs, bugs, or other links if you can’t reasonably make a self-contained CL description. (Also see https://cbea.ms/git-commit/).
IPC reviewer(s): w...@chromium.org
Reviewer source(s):
w...@chromium.org is from context(googleclient/chrome/chromium_gwsq/ipc/config.gwsq)
Related details
- Sunny Sachanandani
- Will Harris
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Will Harris (Gerrit)
Will Harris voted Code-Review+1![]( "Open in Gerrit")
- Shaobo Yan
- Sunny Sachanandani
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |