New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 887645 link

Starred by 3 users

Issue metadata

Status: Untriaged
Owner: ----
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Let's actually use GUARDED_BY annotations.

Project Member Reported by lukasza@chromium.org, Sep 20

Issue description

r591776 landed a while ago and seems to be sticking (maybe ignoring  issue 887610  :-P) so let's try and actually add GUARDED_BY annotations where appropriate.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 21

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

commit 40066f589562a2eafc38ff014509f47af1acbd42
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Fri Sep 21 21:14:41 2018

Adding GUARDED_BY annotations to //content/browser.

This CL also fixes a missing lock that was found by the compile-time
analysis in ChildProcessSecurityPolicyImpl::HasPermissionsForFileSystemFile.

Bug: 887645
Change-Id: I940ade66bb2d754669c2b5bd408b63c91a1a8d5a
Reviewed-on: https://chromium-review.googlesource.com/1237145
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593334}
[modify] https://crrev.com/40066f589562a2eafc38ff014509f47af1acbd42/content/browser/android/java/gin_java_bridge_dispatcher_host.h
[modify] https://crrev.com/40066f589562a2eafc38ff014509f47af1acbd42/content/browser/android/java/gin_java_bridge_message_filter.h
[modify] https://crrev.com/40066f589562a2eafc38ff014509f47af1acbd42/content/browser/android/synchronous_compositor_sync_call_bridge.h
[modify] https://crrev.com/40066f589562a2eafc38ff014509f47af1acbd42/content/browser/child_process_security_policy_browsertest.cc
[modify] https://crrev.com/40066f589562a2eafc38ff014509f47af1acbd42/content/browser/child_process_security_policy_impl.cc
[modify] https://crrev.com/40066f589562a2eafc38ff014509f47af1acbd42/content/browser/child_process_security_policy_impl.h
[modify] https://crrev.com/40066f589562a2eafc38ff014509f47af1acbd42/content/browser/dom_storage/dom_storage_context_wrapper.h
[modify] https://crrev.com/40066f589562a2eafc38ff014509f47af1acbd42/content/browser/dom_storage/session_storage_database.h
[modify] https://crrev.com/40066f589562a2eafc38ff014509f47af1acbd42/content/browser/gpu/gpu_data_manager_impl.h
[modify] https://crrev.com/40066f589562a2eafc38ff014509f47af1acbd42/content/browser/gpu/gpu_data_manager_impl_private_unittest.cc
[modify] https://crrev.com/40066f589562a2eafc38ff014509f47af1acbd42/content/browser/histogram_synchronizer.h
[modify] https://crrev.com/40066f589562a2eafc38ff014509f47af1acbd42/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/40066f589562a2eafc38ff014509f47af1acbd42/content/browser/webui/url_data_manager.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Sep 25

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

commit 36b749b426edbe1e9a6cb31a0e43a1aaa8852930
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Tue Sep 25 17:22:27 2018

Adding GUARDED_BY annotations to //content/common.

Bug: 887645
Change-Id: I0f05856b0473bb6c197d53184322faa1042b2f23
Reviewed-on: https://chromium-review.googlesource.com/1237253
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593981}
[modify] https://crrev.com/36b749b426edbe1e9a6cb31a0e43a1aaa8852930/content/common/font_cache_dispatcher_win.cc
[modify] https://crrev.com/36b749b426edbe1e9a6cb31a0e43a1aaa8852930/content/common/plugin_list.h
[modify] https://crrev.com/36b749b426edbe1e9a6cb31a0e43a1aaa8852930/content/common/service_manager/service_manager_connection_impl.cc

Cc: thakis@chromium.org scottmg@chromium.org brucedaw...@chromium.org yukawa@chromium.org
 Issue 562998  has been merged into this issue.
Project Member

Comment 4 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 5 by bugdroid1@chromium.org, Oct 13

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

commit 7fabf3b19b146f53676abc8bf3b88728a917a73e
Author: Dan Sanders <sandersd@chromium.org>
Date: Sat Oct 13 00:37:41 2018

[media] Add GUARDED_BY to PictureBufferManager.

Bug: 887645
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: Id55f111d7463d71db02e26fe154a0029a10cc53a
Reviewed-on: https://chromium-review.googlesource.com/c/1279237
Reviewed-by: John Rummell <jrummell@chromium.org>
Commit-Queue: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599443}
[modify] https://crrev.com/7fabf3b19b146f53676abc8bf3b88728a917a73e/media/gpu/ipc/service/picture_buffer_manager.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 13

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

commit a0810e4e9e8ffa5475acd49373367423d64e1740
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Sat Oct 13 00:55:37 2018

MSE: Annotate thread-safetiness related to ChunkDemuxerStream::lock_

Adds GUARDED_BY and related annotations to ChunkDemuxerStream.
Adding the annotations makes it clear which fields are protected by
ChunkDemuxerStream::lock_ and enables compile-time verification.

These additions confirmed previous lack of appropriate locking in four
ancillary ChunkDemuxerStream methods; this CL also fixes those cases.

Credit to jrummell@ for identifying the missing locking, and to
chcunningham@ for promoting the new static annotations' usage.

BUG=895017,887645
R=jrummell@chromium.org

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: I0c14ac2fedf60df84e616cf3c268b876d8f8c514
Reviewed-on: https://chromium-review.googlesource.com/c/1279242
Reviewed-by: John Rummell <jrummell@chromium.org>
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599452}
[modify] https://crrev.com/a0810e4e9e8ffa5475acd49373367423d64e1740/media/filters/chunk_demuxer.cc
[modify] https://crrev.com/a0810e4e9e8ffa5475acd49373367423d64e1740/media/filters/chunk_demuxer.h

Project Member

Comment 7 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 8 by bugdroid1@chromium.org, Oct 16

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

commit deba7c112e2d948a4b24e0bdaf1a763a9fd380bd
Author: Mike Wittman <wittman@chromium.org>
Date: Tue Oct 16 20:00:24 2018

Add lock annotations for sampling profiler

Adds GUARDED_BY/EXCLUSIVE_LOCKS_REQUIRED annotations for the parts of
the sampling profiler implementation using locking.

Bug: 887645
Change-Id: Ibe96c541a50254af3822d6811fe03250543a98eb
Reviewed-on: https://chromium-review.googlesource.com/c/1283483
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Commit-Queue: Mike Wittman <wittman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600092}
[modify] https://crrev.com/deba7c112e2d948a4b24e0bdaf1a763a9fd380bd/base/profiler/stack_sampling_profiler.cc
[modify] https://crrev.com/deba7c112e2d948a4b24e0bdaf1a763a9fd380bd/components/metrics/call_stack_profile_metrics_provider.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 26

Project Member

Comment 10 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