Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: browser contexts live forever #25001

Merged
merged 1 commit into from
Aug 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions shell/browser/api/electron_api_app.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1127,10 +1127,10 @@ void App::ImportCertificate(gin_helper::ErrorThrower thrower,
return;
}

auto browser_context = ElectronBrowserContext::From("", false);
auto* browser_context = ElectronBrowserContext::From("", false);
if (!certificate_manager_model_) {
CertificateManagerModel::Create(
browser_context.get(),
browser_context,
base::BindOnce(&App::OnCertificateManagerModelCreated,
base::Unretained(this), std::move(options),
std::move(callback)));
Expand Down
51 changes: 25 additions & 26 deletions shell/browser/api/electron_api_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ Session::Session(v8::Isolate* isolate, ElectronBrowserContext* browser_context)

#if BUILDFLAG(ENABLE_BUILTIN_SPELLCHECKER)
SpellcheckService* service =
SpellcheckServiceFactory::GetForContext(browser_context_.get());
SpellcheckServiceFactory::GetForContext(browser_context_);
if (service) {
service->SetHunspellObserver(this);
}
Expand All @@ -288,7 +288,7 @@ Session::~Session() {

#if BUILDFLAG(ENABLE_BUILTIN_SPELLCHECKER)
SpellcheckService* service =
SpellcheckServiceFactory::GetForContext(browser_context_.get());
SpellcheckServiceFactory::GetForContext(browser_context_);
if (service) {
service->SetHunspellObserver(nullptr);
}
Expand Down Expand Up @@ -350,7 +350,7 @@ v8::Local<v8::Promise> Session::GetCacheSize() {
gin_helper::Promise<int64_t> promise(isolate);
auto handle = promise.GetHandle();

content::BrowserContext::GetDefaultStoragePartition(browser_context_.get())
content::BrowserContext::GetDefaultStoragePartition(browser_context_)
->GetNetworkContext()
->ComputeHttpCacheSize(
base::Time(), base::Time::Max(),
Expand All @@ -374,7 +374,7 @@ v8::Local<v8::Promise> Session::ClearCache() {
gin_helper::Promise<void> promise(isolate);
auto handle = promise.GetHandle();

content::BrowserContext::GetDefaultStoragePartition(browser_context_.get())
content::BrowserContext::GetDefaultStoragePartition(browser_context_)
->GetNetworkContext()
->ClearHttpCache(base::Time(), base::Time::Max(), nullptr,
base::BindOnce(gin_helper::Promise<void>::ResolvePromise,
Expand Down Expand Up @@ -470,17 +470,17 @@ void Session::EnableNetworkEmulation(const gin_helper::Dictionary& options) {
conditions->latency = base::TimeDelta::FromMillisecondsD(latency);
}

auto* network_context = content::BrowserContext::GetDefaultStoragePartition(
browser_context_.get())
->GetNetworkContext();
auto* network_context =
content::BrowserContext::GetDefaultStoragePartition(browser_context_)
->GetNetworkContext();
network_context->SetNetworkConditions(network_emulation_token_,
std::move(conditions));
}

void Session::DisableNetworkEmulation() {
auto* network_context = content::BrowserContext::GetDefaultStoragePartition(
browser_context_.get())
->GetNetworkContext();
auto* network_context =
content::BrowserContext::GetDefaultStoragePartition(browser_context_)
->GetNetworkContext();
network_context->SetNetworkConditions(
network_emulation_token_, network::mojom::NetworkConditions::New());
}
Expand All @@ -500,7 +500,7 @@ void Session::SetCertVerifyProc(v8::Local<v8::Value> val,
std::make_unique<CertVerifierClient>(proc),
cert_verifier_client_remote.InitWithNewPipeAndPassReceiver());
}
content::BrowserContext::GetDefaultStoragePartition(browser_context_.get())
content::BrowserContext::GetDefaultStoragePartition(browser_context_)
->GetNetworkContext()
->SetCertVerifierClient(std::move(cert_verifier_client_remote));

Expand Down Expand Up @@ -552,7 +552,7 @@ v8::Local<v8::Promise> Session::ClearHostResolverCache(gin::Arguments* args) {
gin_helper::Promise<void> promise(isolate);
v8::Local<v8::Promise> handle = promise.GetHandle();

content::BrowserContext::GetDefaultStoragePartition(browser_context_.get())
content::BrowserContext::GetDefaultStoragePartition(browser_context_)
->GetNetworkContext()
->ClearHostCache(nullptr,
base::BindOnce(gin_helper::Promise<void>::ResolvePromise,
Expand All @@ -566,7 +566,7 @@ v8::Local<v8::Promise> Session::ClearAuthCache() {
gin_helper::Promise<void> promise(isolate);
v8::Local<v8::Promise> handle = promise.GetHandle();

content::BrowserContext::GetDefaultStoragePartition(browser_context_.get())
content::BrowserContext::GetDefaultStoragePartition(browser_context_)
->GetNetworkContext()
->ClearHttpAuthCache(
base::Time(),
Expand All @@ -592,9 +592,9 @@ void Session::AllowNTLMCredentialsForDomains(const std::string& domains) {
void Session::SetUserAgent(const std::string& user_agent,
gin::Arguments* args) {
browser_context_->SetUserAgent(user_agent);
auto* network_context = content::BrowserContext::GetDefaultStoragePartition(
browser_context_.get())
->GetNetworkContext();
auto* network_context =
content::BrowserContext::GetDefaultStoragePartition(browser_context_)
->GetNetworkContext();
network_context->SetUserAgent(user_agent);

std::string accept_lang;
Expand Down Expand Up @@ -780,10 +780,9 @@ v8::Local<v8::Value> Session::NetLog(v8::Isolate* isolate) {
return net_log_.Get(isolate);
}

static void StartPreconnectOnUI(
scoped_refptr<ElectronBrowserContext> browser_context,
const GURL& url,
int num_sockets_to_preconnect) {
static void StartPreconnectOnUI(ElectronBrowserContext* browser_context,
const GURL& url,
int num_sockets_to_preconnect) {
std::vector<predictors::PreconnectRequest> requests = {
{url::Origin::Create(url), num_sockets_to_preconnect,
net::NetworkIsolationKey()}};
Expand Down Expand Up @@ -814,7 +813,7 @@ void Session::Preconnect(const gin_helper::Dictionary& options,
DCHECK_GT(num_sockets_to_preconnect, 0);
base::PostTask(
FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(&StartPreconnectOnUI, base::RetainedRef(browser_context_),
base::BindOnce(&StartPreconnectOnUI, base::Unretained(browser_context_),
url, num_sockets_to_preconnect));
}

Expand Down Expand Up @@ -859,7 +858,7 @@ v8::Local<v8::Promise> Session::ListWordsInSpellCheckerDictionary() {
v8::Local<v8::Promise> handle = promise.GetHandle();

SpellcheckService* spellcheck =
SpellcheckServiceFactory::GetForContext(browser_context_.get());
SpellcheckServiceFactory::GetForContext(browser_context_);

if (!spellcheck)
promise.RejectWithErrorMessage(
Expand All @@ -885,7 +884,7 @@ bool Session::AddWordToSpellCheckerDictionary(const std::string& word) {
return false;

SpellcheckService* service =
SpellcheckServiceFactory::GetForContext(browser_context_.get());
SpellcheckServiceFactory::GetForContext(browser_context_);
if (!service)
return false;

Expand All @@ -906,7 +905,7 @@ bool Session::RemoveWordFromSpellCheckerDictionary(const std::string& word) {
return false;

SpellcheckService* service =
SpellcheckServiceFactory::GetForContext(browser_context_.get());
SpellcheckServiceFactory::GetForContext(browser_context_);
if (!service)
return false;

Expand Down Expand Up @@ -954,7 +953,7 @@ gin::Handle<Session> Session::CreateFrom(
gin::Handle<Session> Session::FromPartition(v8::Isolate* isolate,
const std::string& partition,
base::DictionaryValue options) {
scoped_refptr<ElectronBrowserContext> browser_context;
ElectronBrowserContext* browser_context;
if (partition.empty()) {
browser_context =
ElectronBrowserContext::From("", false, std::move(options));
Expand All @@ -967,7 +966,7 @@ gin::Handle<Session> Session::FromPartition(v8::Isolate* isolate,
browser_context =
ElectronBrowserContext::From(partition, true, std::move(options));
}
return CreateFrom(isolate, browser_context.get());
return CreateFrom(isolate, browser_context);
}

gin::ObjectTemplateBuilder Session::GetObjectTemplateBuilder(
Expand Down
6 changes: 2 additions & 4 deletions shell/browser/api/electron_api_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,7 @@ class Session : public gin::Wrappable<Session>,
const std::string& partition,
base::DictionaryValue options = base::DictionaryValue());

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

// gin::Wrappable
static gin::WrapperInfo kWrapperInfo;
Expand Down Expand Up @@ -157,7 +155,7 @@ class Session : public gin::Wrappable<Session>,
// The client id to enable the network throttler.
base::UnguessableToken network_emulation_token_;

scoped_refptr<ElectronBrowserContext> browser_context_;
ElectronBrowserContext* browser_context_;

DISALLOW_COPY_AND_ASSIGN(Session);
};
Expand Down
10 changes: 2 additions & 8 deletions shell/browser/common_web_contents_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -250,14 +250,8 @@ void CommonWebContentsDelegate::ResetManagedWebContents(bool async) {
// is destroyed.
// //electron/patches/chromium/content_browser_main_loop.patch
// is required to get the right quit closure for the main message loop.
base::ThreadTaskRunnerHandle::Get()->PostNonNestableTask(
FROM_HERE,
base::BindOnce(
[](scoped_refptr<ElectronBrowserContext> browser_context,
std::unique_ptr<InspectableWebContents> web_contents) {
web_contents.reset();
},
base::RetainedRef(browser_context_), std::move(web_contents_)));
base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE,
web_contents_.release());
} else {
web_contents_.reset();
}
Expand Down
3 changes: 1 addition & 2 deletions shell/browser/common_web_contents_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,7 @@ class CommonWebContentsDelegate : public content::WebContentsDelegate,

scoped_refptr<DevToolsFileSystemIndexer> devtools_file_system_indexer_;

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

// The stored InspectableWebContents object.
// Notice that web_contents_ must be placed after dialog_manager_, so we can
Expand Down
27 changes: 16 additions & 11 deletions shell/browser/electron_browser_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "base/command_line.h"
#include "base/files/file_path.h"
#include "base/no_destructor.h"
#include "base/path_service.h"
#include "base/strings/string_util.h"
#include "base/task/post_task.h"
Expand Down Expand Up @@ -91,15 +92,17 @@ std::string MakePartitionName(const std::string& input) {
} // namespace

// static
ElectronBrowserContext::BrowserContextMap
ElectronBrowserContext::browser_context_map_;
ElectronBrowserContext::BrowserContextMap&
ElectronBrowserContext::browser_context_map() {
static base::NoDestructor<ElectronBrowserContext::BrowserContextMap>
browser_context_map;
return *browser_context_map;
}

ElectronBrowserContext::ElectronBrowserContext(const std::string& partition,
bool in_memory,
base::DictionaryValue options)
: base::RefCountedDeleteOnSequence<ElectronBrowserContext>(
base::ThreadTaskRunnerHandle::Get()),
in_memory_pref_store_(nullptr),
: in_memory_pref_store_(nullptr),
storage_policy_(new SpecialStoragePolicy),
protocol_registry_(new ProtocolRegistry),
in_memory_(in_memory),
Expand Down Expand Up @@ -444,19 +447,21 @@ ResolveProxyHelper* ElectronBrowserContext::GetResolveProxyHelper() {
}

// static
scoped_refptr<ElectronBrowserContext> ElectronBrowserContext::From(
ElectronBrowserContext* ElectronBrowserContext::From(
const std::string& partition,
bool in_memory,
base::DictionaryValue options) {
PartitionKey key(partition, in_memory);
auto* browser_context = browser_context_map_[key].get();
if (browser_context)
return scoped_refptr<ElectronBrowserContext>(browser_context);
ElectronBrowserContext* browser_context = browser_context_map()[key].get();
if (browser_context) {
return browser_context;
}

auto* new_context =
new ElectronBrowserContext(partition, in_memory, std::move(options));
browser_context_map_[key] = new_context->GetWeakPtr();
return scoped_refptr<ElectronBrowserContext>(new_context);
browser_context_map()[key] =
std::unique_ptr<ElectronBrowserContext>(new_context);
return new_context;
}

} // namespace electron
23 changes: 7 additions & 16 deletions shell/browser/electron_browser_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include <string>
#include <vector>

#include "base/memory/ref_counted_delete_on_sequence.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/predictors/preconnect_manager.h"
#include "content/public/browser/browser_context.h"
Expand Down Expand Up @@ -50,8 +49,7 @@ class WebViewManager;
class ProtocolRegistry;

class ElectronBrowserContext
: public base::RefCountedDeleteOnSequence<ElectronBrowserContext>,
public content::BrowserContext,
: public content::BrowserContext,
public network::mojom::TrustedURLLoaderAuthClient {
public:
// partition_id => browser_context
Expand All @@ -73,19 +71,17 @@ class ElectronBrowserContext
}
};
using BrowserContextMap =
std::map<PartitionKey, base::WeakPtr<ElectronBrowserContext>>;
std::map<PartitionKey, std::unique_ptr<ElectronBrowserContext>>;

// 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<ElectronBrowserContext> From(
static ElectronBrowserContext* From(
const std::string& partition,
bool in_memory,
base::DictionaryValue options = base::DictionaryValue());

static BrowserContextMap browser_context_map() {
return browser_context_map_;
}
static BrowserContextMap& browser_context_map();

void SetUserAgent(const std::string& user_agent);
std::string GetUserAgent() const;
Expand Down Expand Up @@ -151,15 +147,12 @@ class ElectronBrowserContext
return protocol_registry_.get();
}

protected:
ElectronBrowserContext(const std::string& partition,
bool in_memory,
base::DictionaryValue options);
~ElectronBrowserContext() override;

private:
friend class base::RefCountedDeleteOnSequence<ElectronBrowserContext>;
friend class base::DeleteHelper<ElectronBrowserContext>;
ElectronBrowserContext(const std::string& partition,
bool in_memory,
base::DictionaryValue options);

void OnLoaderCreated(int32_t request_id,
mojo::PendingReceiver<network::mojom::TrustedAuthClient>
Expand All @@ -168,8 +161,6 @@ class ElectronBrowserContext
// Initialize pref registry.
void InitPrefs();

static BrowserContextMap browser_context_map_;

ValueMapPrefStore* in_memory_pref_store_;

std::unique_ptr<content::ResourceContext> resource_context_;
Expand Down
2 changes: 2 additions & 0 deletions shell/browser/electron_browser_main_parts.cc
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,8 @@ void ElectronBrowserMainParts::PostMainMessageLoopRun() {
js_env_->OnMessageLoopDestroying();
node_env_.reset();

ElectronBrowserContext::browser_context_map().clear();

fake_browser_process_->PostMainMessageLoopRun();
content::DevToolsAgentHost::StopRemoteDebuggingPipeHandler();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ bool ElectronExtensionsBrowserClient::AreExtensionsDisabled(
}

bool ElectronExtensionsBrowserClient::IsValidContext(BrowserContext* context) {
auto context_map = ElectronBrowserContext::browser_context_map();
auto& context_map = ElectronBrowserContext::browser_context_map();
for (auto const& entry : context_map) {
if (entry.second && entry.second.get() == context)
return true;
Expand Down Expand Up @@ -113,7 +113,7 @@ BrowserContext* ElectronExtensionsBrowserClient::GetOriginalContext(
BrowserContext* context) {
DCHECK(context);
if (context->IsOffTheRecord()) {
return ElectronBrowserContext::From("", false).get();
return ElectronBrowserContext::From("", false);
} else {
return context;
}
Expand Down Expand Up @@ -311,7 +311,7 @@ void ElectronExtensionsBrowserClient::BroadcastEventToRenderers(

std::unique_ptr<extensions::Event> event(
new extensions::Event(histogram_value, event_name, std::move(args)));
auto context_map = ElectronBrowserContext::browser_context_map();
auto& context_map = ElectronBrowserContext::browser_context_map();
for (auto const& entry : context_map) {
if (entry.second) {
extensions::EventRouter::Get(entry.second.get())
Expand Down
Loading