video_capture: use incorrect read_only flag when wrapping shared memory handle for mojo |
||||
Issue descriptionOffending code: https://cs.corp.google.com/eureka_internal/chromium/src/media/capture/video/shared_memory_handle_provider.cc?l=71 The call to mojo::WrapSharedMemoryHandle should use the caller's requested |read_only| flag for the mojo handle, instead of the |read_only_flag_|, which is the flag for the shared memory itself.
,
Nov 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bd5be6bff429e99c7335daee53601ee0ae4a4c91 commit bd5be6bff429e99c7335daee53601ee0ae4a4c91 Author: Peter Qiu <zqiu@chromium.org> Date: Thu Nov 30 17:48:36 2017 Set mojo handle with requested |read_only| flag Bug: 789688 Test: manual Change-Id: I5821af5fd79931197dad45b13b365e909c83a0a4 Reviewed-on: https://chromium-review.googlesource.com/798016 Reviewed-by: Christian Fremerey <chfremer@chromium.org> Commit-Queue: Peter Qiu <zqiu@chromium.org> Cr-Commit-Position: refs/heads/master@{#520593} [modify] https://crrev.com/bd5be6bff429e99c7335daee53601ee0ae4a4c91/media/capture/video/shared_memory_handle_provider.cc
,
Nov 30 2017
,
Dec 4 2017
Would it have been possible to catch this class of bugs with automated tests?
,
Dec 4 2017
,
Dec 4 2017
Should be able to catch it with automated test. Will add one.
,
Dec 7 2017
Hi :) Have you added the test yet?
,
Dec 7 2017
Sorry, not yet. Been busy with other stuff. Will try to do so in the next couple days.
,
Dec 7 2017
,
Dec 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1f2953f9934c34fc58792ac75a21bae95436ddde commit 1f2953f9934c34fc58792ac75a21bae95436ddde Author: Peter Qiu <zqiu@chromium.org> Date: Fri Dec 08 00:34:18 2017 media: add unittest for SharedMemoryHandleProvider The following test cases are added: - Verify read_only flag of the wrapped mojo handle Bug: 789688 Test: Run capture_unittests Change-Id: I14a98322d448e49d526641adea19d30fcc40bc11 Reviewed-on: https://chromium-review.googlesource.com/814796 Commit-Queue: Peter Qiu <zqiu@chromium.org> Reviewed-by: Christian Fremerey <chfremer@chromium.org> Cr-Commit-Position: refs/heads/master@{#522643} [modify] https://crrev.com/1f2953f9934c34fc58792ac75a21bae95436ddde/media/capture/BUILD.gn [modify] https://crrev.com/1f2953f9934c34fc58792ac75a21bae95436ddde/media/capture/video/DEPS [add] https://crrev.com/1f2953f9934c34fc58792ac75a21bae95436ddde/media/capture/video/shared_memory_handle_provider_unittest.cc
,
Dec 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8a8a6df450d4ef68627e86b923123ba47e7f3caa commit 8a8a6df450d4ef68627e86b923123ba47e7f3caa Author: Maxim Kolosovskiy <kolos@chromium.org> Date: Fri Dec 08 10:42:41 2017 Revert "media: add unittest for SharedMemoryHandleProvider" This reverts commit 1f2953f9934c34fc58792ac75a21bae95436ddde. Reason for revert: Broken SharedMemoryHandleProviderTest.VerifyInterProcessTransitHandleForReadWrite SharedMemoryHandleProviderTest.VerifyInterProcessTransitHandleForReadOnly Starting from https://uberchromegw.corp.google.com/i/chromium.memory/builders/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/builds/25107 Original change's description: > media: add unittest for SharedMemoryHandleProvider > > The following test cases are added: > - Verify read_only flag of the wrapped mojo handle > > Bug: 789688 > Test: Run capture_unittests > Change-Id: I14a98322d448e49d526641adea19d30fcc40bc11 > Reviewed-on: https://chromium-review.googlesource.com/814796 > Commit-Queue: Peter Qiu <zqiu@chromium.org> > Reviewed-by: Christian Fremerey <chfremer@chromium.org> > Cr-Commit-Position: refs/heads/master@{#522643} TBR=rockot@chromium.org,zqiu@chromium.org,ehmaldonado@chromium.org,chfremer@chromium.org Change-Id: Ia0bf949a8256cd4d3bdca37f737170ef23dc2d17 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 789688 Reviewed-on: https://chromium-review.googlesource.com/816974 Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org> Commit-Queue: Maxim Kolosovskiy <kolos@chromium.org> Cr-Commit-Position: refs/heads/master@{#522754} [modify] https://crrev.com/8a8a6df450d4ef68627e86b923123ba47e7f3caa/media/capture/BUILD.gn [modify] https://crrev.com/8a8a6df450d4ef68627e86b923123ba47e7f3caa/media/capture/video/DEPS [delete] https://crrev.com/836829eb1399f6f977d0df3fd2789767ff1adc9b/media/capture/video/shared_memory_handle_provider_unittest.cc
,
Dec 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5b673995239b62bf5c986aa6f43059523aee119a commit 5b673995239b62bf5c986aa6f43059523aee119a Author: Peter Qiu <zqiu@chromium.org> Date: Thu Dec 21 19:47:11 2017 Reland: "media: add unittest for SharedMemoryHandleProvider" The following test cases are added: - Verify read_only flag of the wrapped mojo handle Bug: 789688 Test: Run capture_unittests with lsan Change-Id: I0fd57c905d3c7830a8553818464c20c0070a9132 Reviewed-on: https://chromium-review.googlesource.com/830729 Reviewed-by: Ken Rockot <rockot@chromium.org> Reviewed-by: Christian Fremerey <chfremer@chromium.org> Commit-Queue: Peter Qiu <zqiu@chromium.org> Cr-Commit-Position: refs/heads/master@{#525775} [modify] https://crrev.com/5b673995239b62bf5c986aa6f43059523aee119a/media/capture/BUILD.gn [modify] https://crrev.com/5b673995239b62bf5c986aa6f43059523aee119a/media/capture/video/DEPS [add] https://crrev.com/5b673995239b62bf5c986aa6f43059523aee119a/media/capture/video/shared_memory_handle_provider_unittest.cc
,
Dec 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d65c30aa63f2e6708d12c7c6286b73cef3b8f0ba commit d65c30aa63f2e6708d12c7c6286b73cef3b8f0ba Author: Keishi Hattori <keishi@chromium.org> Date: Fri Dec 22 05:35:02 2017 Revert "Reland: "media: add unittest for SharedMemoryHandleProvider"" This reverts commit 5b673995239b62bf5c986aa6f43059523aee119a. Reason for revert: Linux Chromium OS ASan LSan Tests (1) [4 out of the last 4 builds have failed] https://uberchromegw.corp.google.com/i/chromium.memory/builders/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/builds/25337 Original change's description: > Reland: "media: add unittest for SharedMemoryHandleProvider" > > The following test cases are added: > - Verify read_only flag of the wrapped mojo handle > > Bug: 789688 > Test: Run capture_unittests with lsan > > Change-Id: I0fd57c905d3c7830a8553818464c20c0070a9132 > Reviewed-on: https://chromium-review.googlesource.com/830729 > Reviewed-by: Ken Rockot <rockot@chromium.org> > Reviewed-by: Christian Fremerey <chfremer@chromium.org> > Commit-Queue: Peter Qiu <zqiu@chromium.org> > Cr-Commit-Position: refs/heads/master@{#525775} TBR=rockot@chromium.org,zqiu@chromium.org,kolos@chromium.org,ehmaldonado@chromium.org,chfremer@chromium.org Change-Id: Ic5334722e9a0e473461252a85767265c247a71bf No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 789688 Reviewed-on: https://chromium-review.googlesource.com/842323 Reviewed-by: Keishi Hattori <keishi@chromium.org> Commit-Queue: Keishi Hattori <keishi@chromium.org> Cr-Commit-Position: refs/heads/master@{#525941} [modify] https://crrev.com/d65c30aa63f2e6708d12c7c6286b73cef3b8f0ba/media/capture/BUILD.gn [modify] https://crrev.com/d65c30aa63f2e6708d12c7c6286b73cef3b8f0ba/media/capture/video/DEPS [delete] https://crrev.com/0dcffd50781be10d1b1c2fc4c2f8bcd850a611b8/media/capture/video/shared_memory_handle_provider_unittest.cc
,
Jan 4 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3f6772c100cc54efbf339a1153f5df0d53688d42 commit 3f6772c100cc54efbf339a1153f5df0d53688d42 Author: Peter Qiu <zqiu@chromium.org> Date: Thu Jan 04 16:48:25 2018 Reland: "Reland: "media: add unittest for SharedMemoryHandleProvider"" The following test cases are added: - Verify read_only flag of the wrapped mojo handle Bug: 789688 Test: Run capture_unittests with lsan for Linux and Chrome OS Change-Id: Ie3ed385b4f5df2b3ddbecc36bccb6425a7c1fcc2 Reviewed-on: https://chromium-review.googlesource.com/849332 Reviewed-by: Ken Rockot <rockot@chromium.org> Reviewed-by: Christian Fremerey <chfremer@chromium.org> Commit-Queue: Peter Qiu <zqiu@chromium.org> Cr-Commit-Position: refs/heads/master@{#527011} [modify] https://crrev.com/3f6772c100cc54efbf339a1153f5df0d53688d42/media/capture/BUILD.gn [add] https://crrev.com/3f6772c100cc54efbf339a1153f5df0d53688d42/media/capture/DEPS [add] https://crrev.com/3f6772c100cc54efbf339a1153f5df0d53688d42/media/capture/run_all_unittests.cc [modify] https://crrev.com/3f6772c100cc54efbf339a1153f5df0d53688d42/media/capture/video/DEPS [add] https://crrev.com/3f6772c100cc54efbf339a1153f5df0d53688d42/media/capture/video/shared_memory_handle_provider_unittest.cc [modify] https://crrev.com/3f6772c100cc54efbf339a1153f5df0d53688d42/media/capture/video/video_capture_device_unittest.cc
,
Jan 5 2018
|
||||
►
Sign in to add a comment |
||||
Comment 1 by z...@chromium.org
, Nov 29 2017Status: Started (was: Untriaged)