Merge pull request #6674 from electron/session-no-gc

Do not garbage collect sessions
This commit is contained in:
Cheng Zhao 2016-08-01 20:40:55 +09:00 committed by GitHub
commit d56057b186
4 changed files with 27 additions and 21 deletions

View file

@ -11,7 +11,6 @@
#include "atom/common/native_mate_converters/file_path_converter.h" #include "atom/common/native_mate_converters/file_path_converter.h"
#include "atom/common/native_mate_converters/gurl_converter.h" #include "atom/common/native_mate_converters/gurl_converter.h"
#include "atom/common/node_includes.h" #include "atom/common/node_includes.h"
#include "base/memory/linked_ptr.h"
#include "base/message_loop/message_loop.h" #include "base/message_loop/message_loop.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "native_mate/dictionary.h" #include "native_mate/dictionary.h"
@ -56,7 +55,7 @@ namespace {
using WrapDownloadItemCallback = base::Callback<void(v8::Local<v8::Value>)>; using WrapDownloadItemCallback = base::Callback<void(v8::Local<v8::Value>)>;
WrapDownloadItemCallback g_wrap_download_item; WrapDownloadItemCallback g_wrap_download_item;
std::map<uint32_t, linked_ptr<v8::Global<v8::Value>>> g_download_item_objects; std::map<uint32_t, v8::Global<v8::Object>> g_download_item_objects;
} // namespace } // namespace
@ -76,9 +75,7 @@ DownloadItem::~DownloadItem() {
} }
// Remove from the global map. // Remove from the global map.
auto iter = g_download_item_objects.find(weak_map_id()); g_download_item_objects.erase(weak_map_id());
if (iter != g_download_item_objects.end())
g_download_item_objects.erase(iter);
} }
void DownloadItem::OnDownloadUpdated(content::DownloadItem* item) { void DownloadItem::OnDownloadUpdated(content::DownloadItem* item) {
@ -202,8 +199,8 @@ mate::Handle<DownloadItem> DownloadItem::Create(
g_wrap_download_item.Run(handle.ToV8()); g_wrap_download_item.Run(handle.ToV8());
// Reference this object in case it got garbage collected. // Reference this object in case it got garbage collected.
g_download_item_objects[handle->weak_map_id()] = make_linked_ptr( g_download_item_objects[handle->weak_map_id()] =
new v8::Global<v8::Value>(isolate, handle.ToV8())); v8::Global<v8::Object>(isolate, handle.ToV8());
return handle; return handle;
} }

View file

@ -4,6 +4,7 @@
#include "atom/browser/api/atom_api_session.h" #include "atom/browser/api/atom_api_session.h"
#include <map>
#include <string> #include <string>
#include <vector> #include <vector>
@ -173,6 +174,9 @@ const char kPersistPrefix[] = "persist:";
using WrapSessionCallback = base::Callback<void(v8::Local<v8::Value>)>; using WrapSessionCallback = base::Callback<void(v8::Local<v8::Value>)>;
WrapSessionCallback g_wrap_session; WrapSessionCallback g_wrap_session;
// Referenced session objects.
std::map<uint32_t, v8::Global<v8::Object>> g_sessions;
class ResolveProxyHelper { class ResolveProxyHelper {
public: public:
ResolveProxyHelper(AtomBrowserContext* browser_context, ResolveProxyHelper(AtomBrowserContext* browser_context,
@ -346,6 +350,7 @@ Session::Session(v8::Isolate* isolate, AtomBrowserContext* browser_context)
Session::~Session() { Session::~Session() {
content::BrowserContext::GetDownloadManager(browser_context())-> content::BrowserContext::GetDownloadManager(browser_context())->
RemoveObserver(this); RemoveObserver(this);
g_sessions.erase(weak_map_id());
} }
void Session::OnDownloadCreated(content::DownloadManager* manager, void Session::OnDownloadCreated(content::DownloadManager* manager,
@ -537,6 +542,12 @@ mate::Handle<Session> Session::CreateFrom(
auto handle = mate::CreateHandle( auto handle = mate::CreateHandle(
isolate, new Session(isolate, browser_context)); isolate, new Session(isolate, browser_context));
g_wrap_session.Run(handle.ToV8()); g_wrap_session.Run(handle.ToV8());
// The Sessions should never be garbage collected, since the common pattern is
// to use partition strings, instead of using the Session object directly.
g_sessions[handle->weak_map_id()] =
v8::Global<v8::Object>(isolate, handle.ToV8());
return handle; return handle;
} }

View file

@ -9,7 +9,7 @@
#include <utility> #include <utility>
#include <vector> #include <vector>
#include "base/memory/linked_ptr.h" #include "base/macros.h"
#include "v8/include/v8.h" #include "v8/include/v8.h"
namespace atom { namespace atom {
@ -26,16 +26,16 @@ class KeyWeakMap {
KeyWeakMap() {} KeyWeakMap() {}
virtual ~KeyWeakMap() { virtual ~KeyWeakMap() {
for (const auto& p : map_) for (auto& p : map_)
p.second.second->ClearWeak(); p.second.second.ClearWeak();
} }
// Sets the object to WeakMap with the given |key|. // Sets the object to WeakMap with the given |key|.
void Set(v8::Isolate* isolate, const K& key, v8::Local<v8::Object> object) { void Set(v8::Isolate* isolate, const K& key, v8::Local<v8::Object> object) {
auto value = make_linked_ptr(new v8::Global<v8::Object>(isolate, object));
KeyObject key_object = {key, this}; KeyObject key_object = {key, this};
auto& p = map_[key] = std::make_pair(key_object, value); auto& p = map_[key] =
value->SetWeak(&(p.first), OnObjectGC, v8::WeakCallbackType::kParameter); std::make_pair(key_object, v8::Global<v8::Object>(isolate, object));
p.second.SetWeak(&(p.first), OnObjectGC, v8::WeakCallbackType::kParameter);
} }
// Gets the object from WeakMap by its |key|. // Gets the object from WeakMap by its |key|.
@ -44,7 +44,7 @@ class KeyWeakMap {
if (iter == map_.end()) if (iter == map_.end())
return v8::MaybeLocal<v8::Object>(); return v8::MaybeLocal<v8::Object>();
else else
return v8::Local<v8::Object>::New(isolate, *(iter->second.second)); return v8::Local<v8::Object>::New(isolate, iter->second.second);
} }
// Whethere there is an object with |key| in this WeakMap. // Whethere there is an object with |key| in this WeakMap.
@ -56,10 +56,8 @@ class KeyWeakMap {
std::vector<v8::Local<v8::Object>> Values(v8::Isolate* isolate) const { std::vector<v8::Local<v8::Object>> Values(v8::Isolate* isolate) const {
std::vector<v8::Local<v8::Object>> keys; std::vector<v8::Local<v8::Object>> keys;
keys.reserve(map_.size()); keys.reserve(map_.size());
for (const auto& iter : map_) { for (const auto& it : map_)
const auto& value = *(iter.second.second); keys.emplace_back(v8::Local<v8::Object>::New(isolate, it.second.second));
keys.emplace_back(v8::Local<v8::Object>::New(isolate, value));
}
return keys; return keys;
} }
@ -69,7 +67,7 @@ class KeyWeakMap {
if (iter == map_.end()) if (iter == map_.end())
return; return;
iter->second.second->ClearWeak(); iter->second.second.ClearWeak();
map_.erase(iter); map_.erase(iter);
} }
@ -82,7 +80,7 @@ class KeyWeakMap {
// Map of stored objects. // Map of stored objects.
std::unordered_map< std::unordered_map<
K, std::pair<KeyObject, linked_ptr<v8::Global<v8::Object>>>> map_; K, std::pair<KeyObject, v8::Global<v8::Object>>> map_;
DISALLOW_COPY_AND_ASSIGN(KeyWeakMap); DISALLOW_COPY_AND_ASSIGN(KeyWeakMap);
}; };

View file

@ -2,7 +2,7 @@ const {EventEmitter} = require('events')
const {app} = require('electron') const {app} = require('electron')
const {fromPartition, _setWrapSession} = process.atomBinding('session') const {fromPartition, _setWrapSession} = process.atomBinding('session')
// Returns the default session. // Public API.
Object.defineProperties(exports, { Object.defineProperties(exports, {
defaultSession: { defaultSession: {
enumerable: true, enumerable: true,