Skip to content
This repository has been archived by the owner on Jan 4, 2019. It is now read-only.

Commit

Permalink
scoped_refptr -> std::unique_ptr to ensure proper destruction
Browse files Browse the repository at this point in the history
  • Loading branch information
bridiver committed Mar 23, 2017
1 parent 6c10d12 commit 7baccbe
Show file tree
Hide file tree
Showing 14 changed files with 70 additions and 49 deletions.
10 changes: 5 additions & 5 deletions atom/browser/api/atom_api_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ Session::~Session() {

std::string Session::Partition() {
return static_cast<brave::BraveBrowserContext*>(
browser_context_.get())->partition_with_prefix();
browser_context_)->partition_with_prefix();
}

void Session::OnDownloadCreated(content::DownloadManager* manager,
Expand Down Expand Up @@ -604,16 +604,16 @@ mate::Handle<Session> Session::CreateFrom(
mate::Handle<Session> Session::FromPartition(
v8::Isolate* isolate, const std::string& partition,
const base::DictionaryValue& options) {
scoped_refptr<AtomBrowserContext> browser_context =
AtomBrowserContext* browser_context =
brave::BraveBrowserContext::FromPartition(partition, options);

DCHECK(browser_context.get());
DCHECK(browser_context);
// TODO(bridiver) - this is a huge hack to deal with sync call
ScopedAllowWaitForLegacyWebViewApi wait_allowed;
static_cast<brave::BraveBrowserContext*>(
browser_context.get())->ready()->Wait();
browser_context)->ready()->Wait();

return CreateFrom(isolate, browser_context.get());
return CreateFrom(isolate, browser_context);
}

// static
Expand Down
4 changes: 2 additions & 2 deletions atom/browser/api/atom_api_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class Session: public mate::TrackableObject<Session>,
v8::Isolate* isolate, const std::string& partition,
const base::DictionaryValue& options = base::DictionaryValue());

AtomBrowserContext* browser_context() const { return browser_context_.get(); }
AtomBrowserContext* browser_context() const { return browser_context_; }

// mate::TrackableObject:
static void BuildPrototype(v8::Isolate* isolate,
Expand Down Expand Up @@ -106,7 +106,7 @@ class Session: public mate::TrackableObject<Session>,
// The X-DevTools-Emulate-Network-Conditions-Client-Id.
std::string devtools_network_emulation_client_id_;

scoped_refptr<AtomBrowserContext> browser_context_;
AtomBrowserContext* browser_context_;

DISALLOW_COPY_AND_ASSIGN(Session);
};
Expand Down
2 changes: 1 addition & 1 deletion atom/browser/api/atom_api_web_request.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ class WebRequest : public mate::TrackableObject<WebRequest>,
void SetListener(Method method, Event type, mate::Arguments* args);

private:
scoped_refptr<AtomBrowserContext> browser_context_;
AtomBrowserContext* browser_context_;
std::map<const net::URLFetcher*, FetchCallback> fetchers_;

DISALLOW_COPY_AND_ASSIGN(WebRequest);
Expand Down
2 changes: 1 addition & 1 deletion atom/browser/atom_browser_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class AtomBrowserContext : public brightray::BrowserContext {
// Get or create the BrowserContext according to its |partition| and
// |in_memory|. The |options| will be passed to constructor when there is no
// existing BrowserContext.
static scoped_refptr<AtomBrowserContext> From(
static AtomBrowserContext* From(
const std::string& partition, bool in_memory,
const base::DictionaryValue& options = base::DictionaryValue());

Expand Down
2 changes: 1 addition & 1 deletion atom/browser/atom_browser_main_parts.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class AtomBrowserMainParts : public brightray::BrowserMainParts {
void FreeAppDelegate();
#endif

scoped_refptr<brightray::BrowserContext> browser_context_;
brightray::BrowserContext* browser_context_;

// A fake BrowserProcess object that used to feed the source code from chrome.
std::unique_ptr<BrowserProcess> fake_browser_process_;
Expand Down
2 changes: 1 addition & 1 deletion atom/browser/common_web_contents_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ class CommonWebContentsDelegate
scoped_refptr<DevToolsFileSystemIndexer> devtools_file_system_indexer_;

// Make sure BrowserContext is alwasys destroyed after WebContents.
scoped_refptr<AtomBrowserContext> browser_context_;
AtomBrowserContext* browser_context_;

// The stored InspectableWebContents object.
// Notice that web_contents_ must be placed after dialog_manager_, so we can
Expand Down
20 changes: 10 additions & 10 deletions brave/browser/api/brave_api_extension.cc
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,12 @@ Extension::Extension(v8::Isolate* isolate,
BraveBrowserContext* browser_context)
: isolate_(isolate),
browser_context_(browser_context) {
extensions::ExtensionRegistry::Get(browser_context_.get())->AddObserver(this);
extensions::ExtensionRegistry::Get(browser_context_)->AddObserver(this);
}

Extension::~Extension() {
if (extensions::ExtensionRegistry::Get(browser_context_.get())) {
extensions::ExtensionRegistry::Get(browser_context_.get())->
if (extensions::ExtensionRegistry::Get(browser_context_)) {
extensions::ExtensionRegistry::Get(browser_context_)->
RemoveObserver(this);
}
}
Expand Down Expand Up @@ -183,7 +183,7 @@ void Extension::LoadOnFILEThread(const base::FilePath path,
void Extension::NotifyLoadOnUIThread(
scoped_refptr<extensions::Extension> extension) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
extensions::ExtensionSystem::Get(browser_context_.get())->ready().Post(
extensions::ExtensionSystem::Get(browser_context_)->ready().Post(
FROM_HERE,
base::Bind(&Extension::AddExtension,
// GetWeakPtr()
Expand Down Expand Up @@ -227,10 +227,10 @@ void Extension::Load(gin::Arguments* args) {

void Extension::AddExtension(scoped_refptr<extensions::Extension> extension) {
auto extension_service =
extensions::ExtensionSystem::Get(browser_context_.get())->
extensions::ExtensionSystem::Get(browser_context_)->
extension_service();
if (!extensions::ExtensionRegistry::Get(
browser_context_.get())->GetInstalledExtension(extension->id())) {
browser_context_)->GetInstalledExtension(extension->id())) {
extension_service->AddExtension(extension.get());
}
}
Expand Down Expand Up @@ -274,21 +274,21 @@ void Extension::OnExtensionUnloaded(content::BrowserContext* browser_context,

void Extension::Disable(const std::string& extension_id) {
auto extension_service =
extensions::ExtensionSystem::Get(browser_context_.get())->
extensions::ExtensionSystem::Get(browser_context_)->
extension_service();
if (extension_service && extensions::ExtensionRegistry::Get(
browser_context_.get())->GetInstalledExtension(extension_id)) {
browser_context_)->GetInstalledExtension(extension_id)) {
extension_service->DisableExtension(
extension_id, extensions::Extension::DISABLE_USER_ACTION);
}
}

void Extension::Enable(const std::string& extension_id) {
auto extension_service =
extensions::ExtensionSystem::Get(browser_context_.get())->
extensions::ExtensionSystem::Get(browser_context_)->
extension_service();
if (extension_service && extensions::ExtensionRegistry::Get(
browser_context_.get())->GetInstalledExtension(extension_id)) {
browser_context_)->GetInstalledExtension(extension_id)) {
extension_service->EnableExtension(
extension_id);
}
Expand Down
2 changes: 1 addition & 1 deletion brave/browser/api/brave_api_extension.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class Extension : public gin::Wrappable<Extension>,

private:
v8::Isolate* isolate_; // not owned
scoped_refptr<BraveBrowserContext> browser_context_;
BraveBrowserContext* browser_context_;

DISALLOW_COPY_AND_ASSIGN(Extension);
};
Expand Down
30 changes: 15 additions & 15 deletions brave/browser/brave_browser_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,12 @@ BraveBrowserContext::BraveBrowserContext(const std::string& partition,
if (options.GetString("parent_partition", &parent_partition)) {
has_parent_ = true;
original_context_ = static_cast<BraveBrowserContext*>(
atom::AtomBrowserContext::From(parent_partition, false).get());
atom::AtomBrowserContext::From(parent_partition, false));
}

if (in_memory) {
original_context_ = static_cast<BraveBrowserContext*>(
atom::AtomBrowserContext::From(partition, false).get());
atom::AtomBrowserContext::From(partition, false));
original_context_->otr_context_ = this;
}
CreateProfilePrefs(task_runner);
Expand All @@ -130,7 +130,7 @@ BraveBrowserContext::BraveBrowserContext(const std::string& partition,
BrowserThread::PostTask(
BrowserThread::IO, FROM_HERE,
base::Bind(&NotifyOTRProfileCreatedOnIOThread,
base::Unretained(original_context_.get()),
base::Unretained(original_context_),
base::Unretained(this)));
}
#endif
Expand Down Expand Up @@ -159,7 +159,7 @@ BraveBrowserContext::~BraveBrowserContext() {
user_prefs->ClearMutableValues();
#if BUILDFLAG(ENABLE_EXTENSIONS)
ExtensionPrefValueMapFactory::GetForBrowserContext(
original_context_.get())->ClearAllIncognitoSessionOnlyPreferences();
original_context_)->ClearAllIncognitoSessionOnlyPreferences();
#endif
}

Expand All @@ -180,10 +180,10 @@ BraveBrowserContext::~BraveBrowserContext() {

if (IsOffTheRecord()) {
#if BUILDFLAG(ENABLE_EXTENSIONS)
BrowserThread::PostTask(
BrowserThread::IO, FROM_HERE,
base::Bind(&NotifyOTRProfileDestroyedOnIOThread,
base::Unretained(original_context_.get()), base::Unretained(this)));
BrowserThread::PostTask(
BrowserThread::IO, FROM_HERE,
base::Bind(&NotifyOTRProfileDestroyedOnIOThread,
base::Unretained(original_context_), base::Unretained(this)));
#endif
}

Expand Down Expand Up @@ -218,7 +218,7 @@ bool BraveBrowserContext::HasParentContext() {

BraveBrowserContext* BraveBrowserContext::original_context() {
if (original_context_) {
return original_context_.get();
return original_context_;
}
return this;
}
Expand Down Expand Up @@ -256,7 +256,7 @@ void BraveBrowserContext::TrackZoomLevelsFromParent() {
// partitions, e.g. those used by WebViewGuests.
HostZoomMap* host_zoom_map = HostZoomMap::GetDefaultForBrowserContext(this);
HostZoomMap* parent_host_zoom_map =
HostZoomMap::GetDefaultForBrowserContext(original_context_.get());
HostZoomMap::GetDefaultForBrowserContext(original_context_);
host_zoom_map->CopyFrom(parent_host_zoom_map);
// Observe parent profile's HostZoomMap changes so they can also be applied
// to this profile's HostZoomMap.
Expand Down Expand Up @@ -506,9 +506,9 @@ std::string BraveBrowserContext::partition_with_prefix() {
return kPersistPrefix + canonical_partition;
}

scoped_refptr<atom::AtomBrowserContext> BraveBrowserContext::FromPartition(
atom::AtomBrowserContext* BraveBrowserContext::FromPartition(
const std::string& partition, const base::DictionaryValue& options) {
scoped_refptr<atom::AtomBrowserContext> browser_context;
atom::AtomBrowserContext* browser_context;
if (partition.empty()) {
return atom::AtomBrowserContext::From("", false, options);
}
Expand Down Expand Up @@ -564,12 +564,12 @@ void CreateProfileDirectory(base::SequencedTaskRunner* sequenced_task_runner,

// TODO(bridiver) find a better way to do this
// static
scoped_refptr<AtomBrowserContext> AtomBrowserContext::From(
AtomBrowserContext* AtomBrowserContext::From(
const std::string& partition, bool in_memory,
const base::DictionaryValue& options) {
auto browser_context = brightray::BrowserContext::Get(partition, in_memory);
if (browser_context)
return static_cast<AtomBrowserContext*>(browser_context.get());
return static_cast<AtomBrowserContext*>(browser_context);

// TODO(bridiver) - pass the path to initialize the browser context
// TODO(bridiver) - create these with the profile manager
Expand All @@ -592,7 +592,7 @@ scoped_refptr<AtomBrowserContext> AtomBrowserContext::From(
auto profile = new brave::BraveBrowserContext(partition, in_memory, options,
sequenced_task_runner);

if (profile == profile->GetOriginalProfile() &&
if (!profile->IsOffTheRecord() &&
!g_browser_process->profile_manager()->GetProfileByPath(
profile->GetPath())) {
g_browser_process->profile_manager()->AddProfile(profile);
Expand Down
4 changes: 2 additions & 2 deletions brave/browser/brave_browser_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class BraveBrowserContext : public Profile {
std::unique_ptr<content::ZoomLevelDelegate> CreateZoomLevelDelegate(
const base::FilePath& partition_path) override;

static scoped_refptr<atom::AtomBrowserContext> FromPartition(
static atom::AtomBrowserContext* FromPartition(
const std::string& partition, const base::DictionaryValue& options);

static BraveBrowserContext*
Expand Down Expand Up @@ -129,7 +129,7 @@ class BraveBrowserContext : public Profile {
std::unique_ptr<BravePermissionManager> permission_manager_;

bool has_parent_;
scoped_refptr<BraveBrowserContext> original_context_;
BraveBrowserContext* original_context_;
BraveBrowserContext* otr_context_;
const std::string partition_;
std::unique_ptr<base::WaitableEvent> ready_;
Expand Down
4 changes: 2 additions & 2 deletions brave/browser/guest_view/tab_view/tab_view_guest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -201,14 +201,14 @@ void TabViewGuest::CreateWebContents(
v8::Locker locker(isolate);
v8::HandleScope handle_scope(isolate);

scoped_refptr<atom::AtomBrowserContext> browser_context;
atom::AtomBrowserContext* browser_context;

std::string partition;
params.GetString("partition", &partition);
base::DictionaryValue partition_options;
browser_context =
brave::BraveBrowserContext::FromPartition(partition, partition_options);
content::WebContents::CreateParams create_params(browser_context.get());
content::WebContents::CreateParams create_params(browser_context);
create_params.guest_delegate = this;

mate::Dictionary options = mate::Dictionary::CreateEmpty(isolate);
Expand Down
26 changes: 24 additions & 2 deletions chromium_src/chrome/browser/profiles/profile_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,29 @@ ProfileManager::ProfileManager(const base::FilePath& user_data_dir)
}

ProfileManager::~ProfileManager() {
// destroy child profiles
ProfilesInfoMap::iterator itr = profiles_info_.begin();
while (itr != profiles_info_.end()) {
if (itr->second->profile->GetOriginalProfile() != itr->second->profile.get()) {
delete itr->second.release();
profiles_info_.erase(itr++);
} else {
++itr;
}
}

// destroy parent profiles
itr = profiles_info_.begin();
while (itr != profiles_info_.end()) {
// delete otr profile
if (itr->second->profile->HasOffTheRecordProfile())
delete itr->second->profile->GetOffTheRecordProfile();

delete itr->second.release();
profiles_info_.erase(itr++);
}

profiles_info_.clear();
}

// static
Expand Down Expand Up @@ -301,6 +324,5 @@ ProfileManager::ProfileInfo::ProfileInfo(
}

ProfileManager::ProfileInfo::~ProfileInfo() {
// TODO(bridiver) - should this even be a unique_ptr right now?
profile.release();
delete profile.release();
}
6 changes: 3 additions & 3 deletions vendor/brightray/browser/browser_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ class BrowserContext::ResourceContext : public content::ResourceContext {
BrowserContext::BrowserContextMap BrowserContext::browser_context_map_;

// static
scoped_refptr<BrowserContext> BrowserContext::Get(
BrowserContext* BrowserContext::Get(
const std::string& partition, bool in_memory) {
PartitionKey key(partition, in_memory);
if (browser_context_map_[key].get())
return make_scoped_refptr(browser_context_map_[key].get());
if (browser_context_map_[key])
return browser_context_map_[key].get();

return nullptr;
}
Expand Down
5 changes: 2 additions & 3 deletions vendor/brightray/browser/browser_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,12 @@ namespace brightray {

class PermissionManager;

class BrowserContext : public base::RefCounted<BrowserContext>,
public content::BrowserContext,
class BrowserContext : public content::BrowserContext,
public brightray::URLRequestContextGetter::Delegate {
public:
// Get the BrowserContext according to its |partition| and |in_memory|,
// empty pointer when be returned when there is no matching BrowserContext.
static scoped_refptr<BrowserContext> Get(
static BrowserContext* Get(
const std::string& partition, bool in_memory);

base::WeakPtr<BrowserContext> GetWeakPtr() {
Expand Down

1 comment on commit 7baccbe

@diracdeltas
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++ thanks

Please sign in to comment.