New issue
Advanced search Search tips

Issue 727645 link

Starred by 2 users
Status: Started
Owner:
emaxx@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

Add missing thread/sequence checks into the policy code

Project Member Reported by emaxx@chromium.org, May 30 2017 
The current migration from inheriting from base::NonThreadSafe to composition of SEQUENCE_CHECKER revealed that there some classes in the policy code that didn't actually do the necessary DCHECK's.

We should identify and fix those places, so that the non-thread-safe classes will contain necessary checks for catching unsafe usages.

Comment 1 by emaxx@chromium.org, May 30 2017 

The first example is the SchemaRegistry class.
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 3 2017 

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

commit aef0ea84b8dacfd9b642002f7ff11956d298cd29
Author: Maksim Ivanov <emaxx@chromium.org>
Date: Mon Jul 03 16:34:21 2017

Added some more sequence checks in /policy/

* Added missed sequence checks to classes that were already partially
  covered with them:
  CombinedSchemaRegistry,
  ComponentCloudPolicyService,
  ComponentCloudPolicyStore,
  ConfigDirPolicyLoader,
  ForwardingSchemaRegistry,
  SchemaRegistry;
* Added new sequence checks to classes that didn't have them at all:
  CloudExternalDataManagerBase::Backend,
  ComponentCloudPolicyService::Backend;
* Some C++11 clean-up.

Bug: chromium:727645
Change-Id: I34498fbab575587757aecd7f81da6f847ed8eb08
Reviewed-on: https://chromium-review.googlesource.com/558408
Commit-Queue: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Ivan Šandrk <isandrk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484014}
[modify] https://crrev.com/aef0ea84b8dacfd9b642002f7ff11956d298cd29/chrome/browser/chromeos/policy/cloud_external_data_manager_base.cc
[modify] https://crrev.com/aef0ea84b8dacfd9b642002f7ff11956d298cd29/components/policy/core/common/async_policy_loader.cc
[modify] https://crrev.com/aef0ea84b8dacfd9b642002f7ff11956d298cd29/components/policy/core/common/async_policy_loader.h
[modify] https://crrev.com/aef0ea84b8dacfd9b642002f7ff11956d298cd29/components/policy/core/common/cloud/component_cloud_policy_service.cc
[modify] https://crrev.com/aef0ea84b8dacfd9b642002f7ff11956d298cd29/components/policy/core/common/cloud/component_cloud_policy_store.cc
[modify] https://crrev.com/aef0ea84b8dacfd9b642002f7ff11956d298cd29/components/policy/core/common/config_dir_policy_loader.cc
[modify] https://crrev.com/aef0ea84b8dacfd9b642002f7ff11956d298cd29/components/policy/core/common/config_dir_policy_loader.h
[modify] https://crrev.com/aef0ea84b8dacfd9b642002f7ff11956d298cd29/components/policy/core/common/schema_map.cc
[modify] https://crrev.com/aef0ea84b8dacfd9b642002f7ff11956d298cd29/components/policy/core/common/schema_map.h
[modify] https://crrev.com/aef0ea84b8dacfd9b642002f7ff11956d298cd29/components/policy/core/common/schema_registry.cc
[modify] https://crrev.com/aef0ea84b8dacfd9b642002f7ff11956d298cd29/components/policy/core/common/schema_registry.h

Comment 3 by emaxx@chromium.org, Jul 12 2017

Status: Started (was: Assigned)
►

Sign in to add a comment