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

Commit

Permalink
fix browser context destruction
Browse files Browse the repository at this point in the history 
temporary workaround for brave/browser-laptop#2335
auditors: @bbondy
  • Loading branch information
@bridiver
bridiver committed Jun 30, 2016
1 parent 6e31e0e commit a9e8942
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 64 deletions.
130 changes: 80 additions & 50 deletions atom/browser/atom_browser_context.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,22 +44,24 @@
#include "atom/browser/extensions/atom_extension_system_factory.h"
#include "atom/browser/extensions/atom_extensions_network_delegate.h"
#include "components/prefs/json_pref_store.h"
#include "components/prefs/pref_change_registrar.h"
#include "components/prefs/pref_filter.h"
#include "chrome/browser/chrome_notification_types.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "components/syncable_prefs/pref_service_syncable.h"
#include "components/syncable_prefs/pref_service_syncable_factory.h"
#include "components/user_prefs/user_prefs.h"
#include "content/public/browser/dom_storage_context.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/notification_source.h"
#include "content/public/browser/storage_partition.h"
#include "extensions/browser/extension_pref_store.h"
#include "extensions/browser/extension_pref_value_map_factory.h"
#include "extensions/browser/extension_prefs.h"
#include "extensions/browser/extension_protocols.h"
#include "extensions/browser/extension_system.h"
#include "extensions/browser/extensions_browser_client.h"
#include "net/cookies/cookie_store.h"
#endif

using content::BrowserThread;
Expand Down Expand Up @@ -100,36 +102,43 @@ AtomBrowserContext::AtomBrowserContext(const std::string& partition,
#endif
partition_(partition) {
if (in_memory) {
original_context_ = AtomBrowserContext::From(partition, false).get();
original_context_ = static_cast<AtomBrowserContext*>(
AtomBrowserContext::From(partition, false).get());
original_context()->otr_context_ = this;
}
}

AtomBrowserContext::~AtomBrowserContext() {
#if defined(ENABLE_EXTENSIONS)
bool prefs_loaded = user_prefs_->GetInitializationStatus() !=
PrefService::INITIALIZATION_STATUS_WAITING;

if (prefs_loaded) {
user_prefs_->CommitPendingWrite();
}

NotifyWillBeDestroyed(this);
content::NotificationService::current()->Notify(
chrome::NOTIFICATION_PROFILE_DESTROYED,
content::Source<AtomBrowserContext>(this),
content::NotificationService::NoDetails());

if (user_prefs_registrar_.get())
user_prefs_registrar_->RemoveAll();
if (!IsOffTheRecord()) {
// temporary fix for https://github.com/brave/browser-laptop/issues/2335
// TODO(brivdiver) - it seems like something is holding onto a reference to
// url_request_context_getter or the url_request_context and is preventing
// it from being destroyed
url_request_context_getter()->GetURLRequestContext()->cookie_store()->
FlushStore(base::Closure());

bool prefs_loaded = user_prefs_->GetInitializationStatus() !=
PrefService::INITIALIZATION_STATUS_WAITING;

if (prefs_loaded) {
user_prefs_->CommitPendingWrite();
}

user_prefs_registrar_.RemoveAll();
}

if (otr_context_.get()) {
// destroy the otr profile
auto user_prefs = user_prefs::UserPrefs::Get(otr_context_.get());
if (user_prefs)
user_prefs->ClearMutableValues();
otr_context_ = NULL;
} else {
ExtensionPrefValueMapFactory::GetForBrowserContext(this)->
ClearAllIncognitoSessionOnlyPreferences();
}
Expand All @@ -147,21 +156,34 @@ AtomBrowserContext* AtomBrowserContext::original_context() {
if (!IsOffTheRecord()) {
return this;
}
return static_cast<AtomBrowserContext*>(original_context_);
return original_context_;
}

AtomBrowserContext* AtomBrowserContext::otr_context() {
if (IsOffTheRecord()) {
return this;
}

if (otr_context_.get()) {
return static_cast<AtomBrowserContext*>(otr_context_.get());
if (!otr_context_.get()) {
return otr_context_.get();
}

return nullptr;
}

bool AtomBrowserContext::RegisterPrefChangeCallback(
const std::string path, const base::Closure& obs, bool overwrite) {
if (IsOffTheRecord()) {
return original_context()->RegisterPrefChangeCallback(path, obs, overwrite);
} else {
if (overwrite || !user_prefs_registrar_.IsObserved(path)) {
user_prefs_registrar_.Add(path, obs);
return true;
}
return false;
}
}

std::string AtomBrowserContext::GetUserAgent() {
Browser* browser = Browser::Get();
std::string name = RemoveWhitespace(browser->GetName());
Expand Down Expand Up @@ -284,47 +306,43 @@ void AtomBrowserContext::RegisterPrefs(PrefRegistrySimple* pref_registry) {

#if defined(ENABLE_EXTENSIONS)
void AtomBrowserContext::RegisterUserPrefs() {
scoped_refptr<PrefStore> extension_prefs = new ExtensionPrefStore(
PrefStore* extension_prefs = new ExtensionPrefStore(
ExtensionPrefValueMapFactory::GetForBrowserContext(original_context()),
IsOffTheRecord());
user_prefs_registrar_.reset(new PrefChangeRegistrar());

bool async = false;

if (IsOffTheRecord()) {
user_prefs_.reset(
original_context()->user_prefs()->CreateIncognitoPrefService(
extension_prefs.get(), overlay_pref_names_));
extension_prefs, overlay_pref_names_));
user_prefs::UserPrefs::Set(this, user_prefs_.get());
} else {
extensions::AtomBrowserClientExtensionsPart::RegisteryProfilePrefs(
pref_registry_.get());
extensions::ExtensionPrefs::RegisterProfilePrefs(pref_registry_.get());

BrowserContextDependencyManager::GetInstance()->
RegisterProfilePrefsForServices(this, pref_registry_.get());

// create profile prefs
base::FilePath filepath = GetPath().Append(
FILE_PATH_LITERAL("UserPrefs"));
scoped_refptr<base::SequencedTaskRunner> task_runner =
JsonPrefStore::GetTaskRunnerForFile(
filepath, BrowserThread::GetBlockingPool());
scoped_refptr<JsonPrefStore> pref_store =
new JsonPrefStore(filepath, task_runner, std::unique_ptr<PrefFilter>());

// prepare factory
syncable_prefs::PrefServiceSyncableFactory factory;
factory.set_async(async);
factory.set_extension_prefs(extension_prefs);
factory.set_user_prefs(pref_store);
user_prefs_ = factory.CreateSyncable(pref_registry_.get());
user_prefs::UserPrefs::Set(this, user_prefs_.get());
user_prefs_registrar_->Init(user_prefs_.get());
OnPrefsLoaded(true);
return;
}

extensions::AtomBrowserClientExtensionsPart::RegisteryProfilePrefs(
pref_registry_.get());
extensions::ExtensionPrefs::RegisterProfilePrefs(pref_registry_.get());

BrowserContextDependencyManager::GetInstance()->
RegisterProfilePrefsForServices(this, pref_registry_.get());

// create profile prefs
base::FilePath filepath = GetPath().Append(
FILE_PATH_LITERAL("UserPrefs"));
scoped_refptr<base::SequencedTaskRunner> task_runner =
JsonPrefStore::GetTaskRunnerForFile(
filepath, BrowserThread::GetBlockingPool());
scoped_refptr<JsonPrefStore> pref_store =
new JsonPrefStore(filepath, task_runner, std::unique_ptr<PrefFilter>());

// prepare factory
bool async = true;
syncable_prefs::PrefServiceSyncableFactory factory;
factory.set_async(async);
factory.set_extension_prefs(extension_prefs);
factory.set_user_prefs(pref_store);
user_prefs_ = factory.CreateSyncable(pref_registry_.get());
user_prefs::UserPrefs::Set(this, user_prefs_.get());
user_prefs_registrar_->Init(user_prefs_.get());

if (async) {
user_prefs_->AddPrefInitObserver(base::Bind(
&AtomBrowserContext::OnPrefsLoaded, base::Unretained(this)));
Expand All @@ -340,8 +358,15 @@ void AtomBrowserContext::OnPrefsLoaded(bool success) {
BrowserContextDependencyManager::GetInstance()->
CreateBrowserContextServices(this);

if (extensions::ExtensionsBrowserClient::Get() && !IsOffTheRecord()) {
extensions::ExtensionSystem::Get(this)->InitForRegularProfile(true);
if (!IsOffTheRecord()) {
if (extensions::ExtensionsBrowserClient::Get()) {
extensions::ExtensionSystem::Get(this)->InitForRegularProfile(true);
}

user_prefs_registrar_.Init(user_prefs_.get());

content::BrowserContext::GetDefaultStoragePartition(this)->
GetDOMStorageContext()->SetSaveSessionStorageOnDisk();
}

content::NotificationService::current()->Notify(
Expand All @@ -351,6 +376,11 @@ void AtomBrowserContext::OnPrefsLoaded(bool success) {
}
#endif

content::ResourceContext* AtomBrowserContext::GetResourceContext() {
content::BrowserContext::EnsureResourceContextInitialized(this);
return brightray::BrowserContext::GetResourceContext();
}

} // namespace atom

namespace brightray {
Expand Down
15 changes: 9 additions & 6 deletions atom/browser/atom_browser_context.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
#include "base/memory/weak_ptr.h"
#include "brightray/browser/browser_context.h"

class PrefChangeRegistrar;
#if defined(ENABLE_EXTENSIONS)
#include "components/prefs/pref_change_registrar.h"
#endif

namespace syncable_prefs {
class PrefServiceSyncable;
Expand Down Expand Up @@ -49,6 +51,7 @@ class AtomBrowserContext : public brightray::BrowserContext {
content::DownloadManagerDelegate* GetDownloadManagerDelegate() override;
content::BrowserPluginGuestManager* GetGuestManager() override;
content::PermissionManager* GetPermissionManager() override;
content::ResourceContext* GetResourceContext() override;

// brightray::BrowserContext:
void RegisterPrefs(PrefRegistrySimple* pref_registry) override;
Expand All @@ -66,8 +69,8 @@ class AtomBrowserContext : public brightray::BrowserContext {
syncable_prefs::PrefServiceSyncable* user_prefs() const {
return user_prefs_.get(); }

PrefChangeRegistrar* user_prefs_change_registrar() const {
return user_prefs_registrar_.get(); }
bool RegisterPrefChangeCallback(
const std::string path, const base::Closure& obs, bool overwrite);

const std::string& partition() const { return partition_; }

Expand All @@ -81,7 +84,7 @@ class AtomBrowserContext : public brightray::BrowserContext {
void OnPrefsLoaded(bool success);
scoped_refptr<user_prefs::PrefRegistrySyncable> pref_registry_;
std::unique_ptr<syncable_prefs::PrefServiceSyncable> user_prefs_;
std::unique_ptr<PrefChangeRegistrar> user_prefs_registrar_;
PrefChangeRegistrar user_prefs_registrar_;
std::vector<const char*> overlay_pref_names_;
#endif

Expand All @@ -93,8 +96,8 @@ class AtomBrowserContext : public brightray::BrowserContext {
AtomCertVerifier* cert_verifier_;
AtomNetworkDelegate* network_delegate_;

brightray::BrowserContext* original_context_;
scoped_refptr<brightray::BrowserContext> otr_context_;
AtomBrowserContext* original_context_;
scoped_refptr<AtomBrowserContext> otr_context_;
const std::string partition_;

DISALLOW_COPY_AND_ASSIGN(AtomBrowserContext);
Expand Down
13 changes: 5 additions & 8 deletions atom/browser/extensions/atom_browser_client_extensions_part.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,13 +204,9 @@ void AtomBrowserClientExtensionsPart::RenderProcessWillLaunch(
host->AddFilter(new IOThreadExtensionMessageFilter(id, context));
extension_web_request_api_helpers::SendExtensionWebRequestStatusToHost(host);

auto user_prefs_registrar = context->user_prefs_change_registrar();
if (!user_prefs_registrar->IsObserved("content_settings")) {
user_prefs_registrar->Add(
"content_settings",
base::Bind(&AtomBrowserClientExtensionsPart::UpdateContentSettings,
base::Unretained(this)));
}
context->RegisterPrefChangeCallback("content_settings",
base::Bind(&AtomBrowserClientExtensionsPart::UpdateContentSettings,
base::Unretained(this)), false);
UpdateContentSettingsForHost(host->GetID());
}

Expand Down Expand Up @@ -270,7 +266,8 @@ void AtomBrowserClientExtensionsPart::SiteInstanceDeleting(
if (!registry)
return;

render_process_hosts_.erase(site_instance->GetProcess()->GetID());
if (render_process_hosts_[site_instance->GetProcess()->GetID()])
render_process_hosts_.erase(site_instance->GetProcess()->GetID());

const Extension* extension =
registry->enabled_extensions().GetExtensionOrAppByURL(
Expand Down

1 comment on commit a9e8942

@bbondy
Copy link
Member

@bbondy bbondy commented on a9e8942 Jun 30, 2016

Choose a reason for hiding this comment

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

wfm for now

Please sign in to comment.