New issue
Advanced search Search tips

Issue 893739 link

Starred by 2 users
Status: Available
Owner: ----
Cc:
chcunningham@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

Use GUARDED_BY to annotate data protected by base::Lock in media/

Project Member Reported by jrumm...@chromium.org, Oct 9 
Recent changes have added GUARDED_BY() as an annotation that a protected member is protected by a base::Lock, and enables compile-time analysis to verify this [1]. This should be enabled in all media/ code.

Some code has comments to reflect this, e.g.
  KeyIdToSessionKeysMap key_map_;    // Protected by |key_map_lock_|.
However, most code in media/ doesn't indicate what field(s) are protected.


[1] https://groups.google.com/a/chromium.org/forum/?utm_medium=email&utm_source=footer#!msg/chromium-dev/mG5sfyU1GGk/f08tf3LrAgAJ
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 11 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/08d41ac558fd84035f4fea691370edf2fe7373d9

commit 08d41ac558fd84035f4fea691370edf2fe7373d9
Author: John Rummell <jrummell@chromium.org>
Date: Thu Oct 11 22:42:50 2018

Add GUARDED_BY annotations to //media/cdm.

Adding the annotation makes it clear which fields are protected by
base::Lock, and enables compile-time analysis to verify this.

BUG=893739,887645
TEST=compiles

Change-Id: I1b8cdd720fe4c30ed800d73bc117a33780593214
Reviewed-on: https://chromium-review.googlesource.com/c/1274832
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Commit-Queue: John Rummell <jrummell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598987}
[modify] https://crrev.com/08d41ac558fd84035f4fea691370edf2fe7373d9/media/cdm/aes_decryptor.h
[modify] https://crrev.com/08d41ac558fd84035f4fea691370edf2fe7373d9/media/cdm/cdm_manager.h
[modify] https://crrev.com/08d41ac558fd84035f4fea691370edf2fe7373d9/media/cdm/player_tracker_impl.h
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 13 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/03013c95df1418bf3c5595734cdbe756a4625c24

commit 03013c95df1418bf3c5595734cdbe756a4625c24
Author: John Rummell <jrummell@chromium.org>
Date: Sat Oct 13 01:20:08 2018

Add GUARDED_BY annotations to //media/base.

Adding the annotation makes it clear which fields are protected by
base::Lock, and enables compile-time analysis to verify this.

BUG=893739,887645
TEST=compiles

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I8dacf5b6a8ee64356344b31cc45cf4c5516e95bb
Reviewed-on: https://chromium-review.googlesource.com/c/1275306
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: John Rummell <jrummell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599460}
[modify] https://crrev.com/03013c95df1418bf3c5595734cdbe756a4625c24/media/base/audio_buffer.cc
[modify] https://crrev.com/03013c95df1418bf3c5595734cdbe756a4625c24/media/base/audio_buffer.h
[modify] https://crrev.com/03013c95df1418bf3c5595734cdbe756a4625c24/media/base/audio_buffer_unittest.cc
[modify] https://crrev.com/03013c95df1418bf3c5595734cdbe756a4625c24/media/base/audio_renderer_mixer.cc
[modify] https://crrev.com/03013c95df1418bf3c5595734cdbe756a4625c24/media/base/audio_renderer_mixer.h
[modify] https://crrev.com/03013c95df1418bf3c5595734cdbe756a4625c24/media/base/audio_renderer_mixer_input.h
[modify] https://crrev.com/03013c95df1418bf3c5595734cdbe756a4625c24/media/base/audio_renderer_mixer_unittest.cc
[modify] https://crrev.com/03013c95df1418bf3c5595734cdbe756a4625c24/media/base/callback_registry.h
[modify] https://crrev.com/03013c95df1418bf3c5595734cdbe756a4625c24/media/base/fake_audio_worker.cc
[modify] https://crrev.com/03013c95df1418bf3c5595734cdbe756a4625c24/media/base/media_log.cc
[modify] https://crrev.com/03013c95df1418bf3c5595734cdbe756a4625c24/media/base/media_log.h
[modify] https://crrev.com/03013c95df1418bf3c5595734cdbe756a4625c24/media/base/pipeline_impl.cc
[modify] https://crrev.com/03013c95df1418bf3c5595734cdbe756a4625c24/media/base/silent_sink_suspender.cc
[modify] https://crrev.com/03013c95df1418bf3c5595734cdbe756a4625c24/media/base/silent_sink_suspender.h
[modify] https://crrev.com/03013c95df1418bf3c5595734cdbe756a4625c24/media/base/silent_sink_suspender_unittest.cc
[modify] https://crrev.com/03013c95df1418bf3c5595734cdbe756a4625c24/media/base/video_frame.h
[modify] https://crrev.com/03013c95df1418bf3c5595734cdbe756a4625c24/media/base/video_frame_pool.cc
[modify] https://crrev.com/03013c95df1418bf3c5595734cdbe756a4625c24/media/base/wall_clock_time_source.cc
[modify] https://crrev.com/03013c95df1418bf3c5595734cdbe756a4625c24/media/base/wall_clock_time_source.h
[modify] https://crrev.com/03013c95df1418bf3c5595734cdbe756a4625c24/media/base/wall_clock_time_source_unittest.cc
[modify] https://crrev.com/03013c95df1418bf3c5595734cdbe756a4625c24/media/filters/video_renderer_algorithm_unittest.cc
[modify] https://crrev.com/03013c95df1418bf3c5595734cdbe756a4625c24/media/renderers/video_renderer_impl_unittest.cc
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 16 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6fab02da5f40356ca35ea7a4134d4995738de765

commit 6fab02da5f40356ca35ea7a4134d4995738de765
Author: Dan Sanders <sandersd@chromium.org>
Date: Tue Oct 16 06:05:05 2018

[media] Clarify PipelineImpl locking comments.

Bug: 893739
Change-Id: Icb19a36549d9f877f5b32a5a10b05f1420f812a8
Reviewed-on: https://chromium-review.googlesource.com/c/1279292
Commit-Queue: Dan Sanders <sandersd@chromium.org>
Reviewed-by: John Rummell <jrummell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599882}
[modify] https://crrev.com/6fab02da5f40356ca35ea7a4134d4995738de765/media/base/pipeline_impl.cc
[modify] https://crrev.com/6fab02da5f40356ca35ea7a4134d4995738de765/media/base/renderer.h
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 26 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2b715be1c5616b5d771cc12e5be1073e93cba932

commit 2b715be1c5616b5d771cc12e5be1073e93cba932
Author: John Rummell <jrummell@chromium.org>
Date: Fri Oct 26 23:55:15 2018

Add GUARDED_BY annotations to //media/audio.

Adding the annotation makes it clear which fields are protected by
base::Lock, and enables compile-time analysis to verify this.

BUG=893739,887645
TEST=compiles

Change-Id: Ideddb22c0687168d25ceab9ad30f32797862fcfd
Reviewed-on: https://chromium-review.googlesource.com/c/1302323
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: John Rummell <jrummell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603260}
[modify] https://crrev.com/2b715be1c5616b5d771cc12e5be1073e93cba932/media/audio/audio_manager.cc
[modify] https://crrev.com/2b715be1c5616b5d771cc12e5be1073e93cba932/media/audio/audio_output_device.h
[modify] https://crrev.com/2b715be1c5616b5d771cc12e5be1073e93cba932/media/audio/audio_output_stream_sink.h
[modify] https://crrev.com/2b715be1c5616b5d771cc12e5be1073e93cba932/media/audio/simple_sources.cc
[modify] https://crrev.com/2b715be1c5616b5d771cc12e5be1073e93cba932/media/audio/simple_sources.h
[modify] https://crrev.com/2b715be1c5616b5d771cc12e5be1073e93cba932/media/audio/virtual_audio_input_stream.h
[modify] https://crrev.com/2b715be1c5616b5d771cc12e5be1073e93cba932/media/audio/virtual_audio_sink.h
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 30 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cfbf91180c29c6aeb58c6b752b4af8c65ad18269

commit cfbf91180c29c6aeb58c6b752b4af8c65ad18269
Author: Max Morin <maxmorin@chromium.org>
Date: Tue Oct 30 10:58:56 2018

Add a missing GUARDED_BY in AudioOutputDevice.

|stopping_hack_| is also protected by the lock.

Bug: 893739
Change-Id: I9bb82a4f75ee238e79bf53d43bc342cb9341082c
Reviewed-on: https://chromium-review.googlesource.com/c/1304473
Reviewed-by: John Rummell <jrummell@chromium.org>
Commit-Queue: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603861}
[modify] https://crrev.com/cfbf91180c29c6aeb58c6b752b4af8c65ad18269/media/audio/audio_output_device.h
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 2 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/884edb78ccb6a88d716f246eb827a55b21670b81

commit 884edb78ccb6a88d716f246eb827a55b21670b81
Author: John Rummell <jrummell@chromium.org>
Date: Fri Nov 02 01:26:13 2018

Add GUARDED_BY annotations to //media/blink.

Adding the annotation makes it clear which fields are protected by
base::Lock, and enables compile-time analysis to verify this.

BUG=893739,887645
TEST=compiles

Change-Id: Ibf8ff9b73f218fcd3cfeb4387e7ad406418ef522
Reviewed-on: https://chromium-review.googlesource.com/c/1303405
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: John Rummell <jrummell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604802}
[modify] https://crrev.com/884edb78ccb6a88d716f246eb827a55b21670b81/media/blink/video_frame_compositor.h
[modify] https://crrev.com/884edb78ccb6a88d716f246eb827a55b21670b81/media/blink/webaudiosourceprovider_impl.cc
[modify] https://crrev.com/884edb78ccb6a88d716f246eb827a55b21670b81/media/blink/webaudiosourceprovider_impl.h
►

Sign in to add a comment