extensions/browser/extensions_test_browser_context.cc - Issue 355223002: Add ExtensionsTestBrowserContext class for test.

Side by Side Diff: extensions/browser/extensions_test_browser_context.cc

Issue 355223002: Add ExtensionsTestBrowserContext class for test. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Created 6 years, 5 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

OLD	NEW
(Empty)
	1 // Copyright 2014 The Chromium Authors. All rights reserved.

	2 // Use of this source code is governed by a BSD-style license that can be

	3 // found in the LICENSE file.

	4

	5 #include "extensions/browser/extensions_test_browser_context.h"

	6

	7 #include "components/keyed_service/content/browser_context_dependency_manager.h"

	8 #include "extensions/browser/extensions_browser_client.h"

	9 #include "extensions/browser/test_extensions_browser_client.h"

	10

	11 namespace extensions {

	12

	13 ExtensionsTestBrowserContext::ExtensionsTestBrowserContext() {

	14 BrowserContextDependencyManager::GetInstance()

	15 ->CreateBrowserContextServicesForTest(this);

	16 ExtensionsBrowserClient::Set(new TestExtensionsBrowserClient(this));
	Yoyo Zhou 2014/07/18 01:47:13 It looks like this is leaked. Can you mean the Tes It looks like this is leaked. Can you mean the TestExtensionsBrowserClient a member of this class? limasdf 2014/07/18 02:22:25 This is necessary because from the ChromeExtension Show quoted text On 2014/07/18 01:47:13, Yoyo Zhou wrote: > It looks like this is leaked. Can you mean the TestExtensionsBrowserClient a > member of this class? This is necessary because from the ChromeExtensionsBrowserClient::GetPrefServiceForContext, ChromeExtensionsBrowserClient::GetOriginalContext, ChromeExtensionsBrowserClient::GetOfftheRecordContext do 'static_cast' to Profile. and call Profile::GetPrefs(), GetOriginalProfile(), HasOfftheRecordProfile(). but BrowserContext doesn't have those methods. So there's crash. Yeah, we eventually have to fix ChromeExtensionsBrowserClinet to handle pure BrowserContext. I think. Yoyo Zhou 2014/07/18 02:38:45 Sorry, I meant to write "make" instead of "mean" a Show quoted text On 2014/07/18 02:22:25, Sungguk Lim wrote: > On 2014/07/18 01:47:13, Yoyo Zhou wrote: > > It looks like this is leaked. Can you mean the TestExtensionsBrowserClient a > > member of this class? > > This is necessary because from the > ChromeExtensionsBrowserClient::GetPrefServiceForContext, > ChromeExtensionsBrowserClient::GetOriginalContext, > ChromeExtensionsBrowserClient::GetOfftheRecordContext do 'static_cast' to > Profile. and call Profile::GetPrefs(), GetOriginalProfile(), > HasOfftheRecordProfile(). but BrowserContext doesn't have those methods. So > there's crash. > > Yeah, we eventually have to fix ChromeExtensionsBrowserClinet to handle pure > BrowserContext. I think. Sorry, I meant to write "make" instead of "mean" above. I mean the part: new TestExtensionsBrowserClient... This TestExtensionsBrowserClient never gets deleted. But if we make it a member of ExtensionsTestBrowserContext, and we call ExtensionsBrowserClient::Set(&test_extensions_browser_client_); that would fix it. limasdf 2014/07/18 06:48:12 Done. Yes exactly, there was a leak. Fixed. Show quoted text On 2014/07/18 02:38:45, Yoyo Zhou wrote: > On 2014/07/18 02:22:25, Sungguk Lim wrote: > > On 2014/07/18 01:47:13, Yoyo Zhou wrote: > > > It looks like this is leaked. Can you mean the TestExtensionsBrowserClient a > > > member of this class? > > > > This is necessary because from the > > ChromeExtensionsBrowserClient::GetPrefServiceForContext, > > ChromeExtensionsBrowserClient::GetOriginalContext, > > ChromeExtensionsBrowserClient::GetOfftheRecordContext do 'static_cast' to > > Profile. and call Profile::GetPrefs(), GetOriginalProfile(), > > HasOfftheRecordProfile(). but BrowserContext doesn't have those methods. So > > there's crash. > > > > Yeah, we eventually have to fix ChromeExtensionsBrowserClinet to handle pure > > BrowserContext. I think. > > Sorry, I meant to write "make" instead of "mean" above. > > I mean the part: > new TestExtensionsBrowserClient... > This TestExtensionsBrowserClient never gets deleted. But if we make it a member > of ExtensionsTestBrowserContext, and we call > ExtensionsBrowserClient::Set(&test_extensions_browser_client_); > that would fix it. Done. Yes exactly, there was a leak. Fixed.
	17 }

	18

	19 ExtensionsTestBrowserContext::~ExtensionsTestBrowserContext() {

	20 BrowserContextDependencyManager::GetInstance()

	21 ->DestroyBrowserContextServices(this);

	22 ExtensionsBrowserClient::Set(NULL);

	23 }

	24

	25 } // namespace extensions

OLD	NEW

« no previous file with comments | « extensions/browser/extensions_test_browser_context.h ('k') | extensions/browser/image_loader_unittest.cc » ('j') | no next file with comments »