Commit 382570ad authored by Derek Schuff's avatar Derek Schuff Committed by Commit Bot

Add DCHECKs for correct thread usage of KeyedService code

Bug: 701326
Change-Id: If652eb76fabc09cabdd61865cdb409f72c140392
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1748083Reviewed-by: default avatarLutz Justen <ljusten@chromium.org>
Reviewed-by: default avatarCait Phillips <caitkp@chromium.org>
Commit-Queue: Derek Schuff <dschuff@chromium.org>
Cr-Commit-Position: refs/heads/master@{#686622}
parent fc3a6edf
...@@ -60,9 +60,6 @@ KeyedService* BrowserContextKeyedServiceFactory::GetServiceForBrowserContext( ...@@ -60,9 +60,6 @@ KeyedService* BrowserContextKeyedServiceFactory::GetServiceForBrowserContext(
content::BrowserContext* content::BrowserContext*
BrowserContextKeyedServiceFactory::GetBrowserContextToUse( BrowserContextKeyedServiceFactory::GetBrowserContextToUse(
content::BrowserContext* context) const { content::BrowserContext* context) const {
// TODO(crbug.com/701326): This DCHECK should be moved to GetContextToUse().
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// Safe default for Incognito mode: no service. // Safe default for Incognito mode: no service.
if (context->IsOffTheRecord()) if (context->IsOffTheRecord())
return nullptr; return nullptr;
...@@ -103,6 +100,7 @@ bool BrowserContextKeyedServiceFactory::IsOffTheRecord(void* context) const { ...@@ -103,6 +100,7 @@ bool BrowserContextKeyedServiceFactory::IsOffTheRecord(void* context) const {
} }
void* BrowserContextKeyedServiceFactory::GetContextToUse(void* context) const { void* BrowserContextKeyedServiceFactory::GetContextToUse(void* context) const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
AssertContextWasntDestroyed(context); AssertContextWasntDestroyed(context);
return GetBrowserContextToUse(static_cast<content::BrowserContext*>(context)); return GetBrowserContextToUse(static_cast<content::BrowserContext*>(context));
} }
......
...@@ -61,9 +61,6 @@ RefcountedBrowserContextKeyedServiceFactory::GetServiceForBrowserContext( ...@@ -61,9 +61,6 @@ RefcountedBrowserContextKeyedServiceFactory::GetServiceForBrowserContext(
content::BrowserContext* content::BrowserContext*
RefcountedBrowserContextKeyedServiceFactory::GetBrowserContextToUse( RefcountedBrowserContextKeyedServiceFactory::GetBrowserContextToUse(
content::BrowserContext* context) const { content::BrowserContext* context) const {
// TODO(crbug.com/701326): This DCHECK should be moved to GetContextToUse().
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// Safe default for Incognito mode: no service. // Safe default for Incognito mode: no service.
if (context->IsOffTheRecord()) if (context->IsOffTheRecord())
return nullptr; return nullptr;
...@@ -105,6 +102,7 @@ bool RefcountedBrowserContextKeyedServiceFactory::IsOffTheRecord( ...@@ -105,6 +102,7 @@ bool RefcountedBrowserContextKeyedServiceFactory::IsOffTheRecord(
void* RefcountedBrowserContextKeyedServiceFactory::GetContextToUse( void* RefcountedBrowserContextKeyedServiceFactory::GetContextToUse(
void* context) const { void* context) const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
AssertContextWasntDestroyed(context); AssertContextWasntDestroyed(context);
return GetBrowserContextToUse(static_cast<content::BrowserContext*>(context)); return GetBrowserContextToUse(static_cast<content::BrowserContext*>(context));
} }
......
// Copyright 2014 The Chromium Authors. All rights reserved. // Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
...@@ -34,9 +35,7 @@ void KeyedServiceBaseFactory::DependsOn(KeyedServiceBaseFactory* rhs) { ...@@ -34,9 +35,7 @@ void KeyedServiceBaseFactory::DependsOn(KeyedServiceBaseFactory* rhs) {
} }
void KeyedServiceBaseFactory::AssertContextWasntDestroyed(void* context) const { void KeyedServiceBaseFactory::AssertContextWasntDestroyed(void* context) const {
// TODO(crbug.com/701326): We should DCHECK(CalledOnValidThread()) here, but DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// currently some code doesn't do service getting on the main thread.
// This needs to be fixed and DCHECK should be restored here.
dependency_manager_->AssertContextWasntDestroyed(context); dependency_manager_->AssertContextWasntDestroyed(context);
} }
......
...@@ -54,9 +54,6 @@ KeyedService* BrowserStateKeyedServiceFactory::GetServiceForBrowserState( ...@@ -54,9 +54,6 @@ KeyedService* BrowserStateKeyedServiceFactory::GetServiceForBrowserState(
web::BrowserState* BrowserStateKeyedServiceFactory::GetBrowserStateToUse( web::BrowserState* BrowserStateKeyedServiceFactory::GetBrowserStateToUse(
web::BrowserState* context) const { web::BrowserState* context) const {
// TODO(crbug.com/701326): This DCHECK should be moved to GetContextToUse().
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// Safe default for Incognito mode: no service. // Safe default for Incognito mode: no service.
if (context->IsOffTheRecord()) if (context->IsOffTheRecord())
return nullptr; return nullptr;
...@@ -92,6 +89,7 @@ bool BrowserStateKeyedServiceFactory::IsOffTheRecord(void* context) const { ...@@ -92,6 +89,7 @@ bool BrowserStateKeyedServiceFactory::IsOffTheRecord(void* context) const {
} }
void* BrowserStateKeyedServiceFactory::GetContextToUse(void* context) const { void* BrowserStateKeyedServiceFactory::GetContextToUse(void* context) const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
AssertContextWasntDestroyed(context); AssertContextWasntDestroyed(context);
return GetBrowserStateToUse(static_cast<web::BrowserState*>(context)); return GetBrowserStateToUse(static_cast<web::BrowserState*>(context));
} }
......
...@@ -60,9 +60,6 @@ RefcountedBrowserStateKeyedServiceFactory::GetServiceForBrowserState( ...@@ -60,9 +60,6 @@ RefcountedBrowserStateKeyedServiceFactory::GetServiceForBrowserState(
web::BrowserState* web::BrowserState*
RefcountedBrowserStateKeyedServiceFactory::GetBrowserStateToUse( RefcountedBrowserStateKeyedServiceFactory::GetBrowserStateToUse(
web::BrowserState* context) const { web::BrowserState* context) const {
// TODO(crbug.com/701326): This DCHECK should be moved to GetContextToUse().
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// Safe default for Incognito mode: no service. // Safe default for Incognito mode: no service.
if (context->IsOffTheRecord()) if (context->IsOffTheRecord())
return nullptr; return nullptr;
...@@ -103,6 +100,7 @@ bool RefcountedBrowserStateKeyedServiceFactory::IsOffTheRecord( ...@@ -103,6 +100,7 @@ bool RefcountedBrowserStateKeyedServiceFactory::IsOffTheRecord(
void* RefcountedBrowserStateKeyedServiceFactory::GetContextToUse( void* RefcountedBrowserStateKeyedServiceFactory::GetContextToUse(
void* context) const { void* context) const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
AssertContextWasntDestroyed(context); AssertContextWasntDestroyed(context);
return GetBrowserStateToUse(static_cast<web::BrowserState*>(context)); return GetBrowserStateToUse(static_cast<web::BrowserState*>(context));
} }
......
...@@ -120,7 +120,5 @@ KeyedService* PolicyBlacklistFactory::BuildServiceInstanceFor( ...@@ -120,7 +120,5 @@ KeyedService* PolicyBlacklistFactory::BuildServiceInstanceFor(
content::BrowserContext* PolicyBlacklistFactory::GetBrowserContextToUse( content::BrowserContext* PolicyBlacklistFactory::GetBrowserContextToUse(
content::BrowserContext* context) const { content::BrowserContext* context) const {
// TODO(crbug.com/701326): This DCHECK should be moved to GetContextToUse().
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
return context; return context;
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment