diff --git a/atom/common/api/atom_api_v8_util.cc b/atom/common/api/atom_api_v8_util.cc index 7eda0e667c9..3d80db0c0a4 100644 --- a/atom/common/api/atom_api_v8_util.cc +++ b/atom/common/api/atom_api_v8_util.cc @@ -12,20 +12,10 @@ #include "atom/common/native_mate_converters/gurl_converter.h" #include "atom/common/node_includes.h" #include "base/hash.h" -#include "base/process/process_handle.h" -#include "base/strings/stringprintf.h" #include "native_mate/dictionary.h" #include "url/origin.h" #include "v8/include/v8-profiler.h" -// This is defined in later versions of Chromium, remove this if you see -// compiler complaining duplicate defines. -#if defined(OS_WIN) || defined(OS_FUCHSIA) -#define CrPRIdPid "ld" -#else -#define CrPRIdPid "d" -#endif - namespace std { // The hash function used by DoubleIDWeakMap. @@ -100,16 +90,6 @@ int32_t GetObjectHash(v8::Local object) { return object->GetIdentityHash(); } -std::string GetContextID(v8::Isolate* isolate) { - // When a page is reloaded, V8 and blink may have optimizations that do not - // free blink::WebLocalFrame and v8::Context and reuse them for the new page, - // while we always recreate node::Environment when a page is loaded. - // So the only reliable way to return an identity for a page, is to return the - // address of the node::Environment instance. - node::Environment* env = node::Environment::GetCurrent(isolate); - return base::StringPrintf("%" CrPRIdPid "-%p", base::GetCurrentProcId(), env); -} - void TakeHeapSnapshot(v8::Isolate* isolate) { isolate->GetHeapProfiler()->TakeHeapSnapshot(); } @@ -132,7 +112,6 @@ void Initialize(v8::Local exports, dict.SetMethod("setHiddenValue", &SetHiddenValue); dict.SetMethod("deleteHiddenValue", &DeleteHiddenValue); dict.SetMethod("getObjectHash", &GetObjectHash); - dict.SetMethod("getContextId", &GetContextID); dict.SetMethod("takeHeapSnapshot", &TakeHeapSnapshot); dict.SetMethod("setRemoteCallbackFreer", &atom::RemoteCallbackFreer::BindTo); dict.SetMethod("setRemoteObjectFreer", &atom::RemoteObjectFreer::BindTo); diff --git a/atom/renderer/atom_renderer_client.cc b/atom/renderer/atom_renderer_client.cc index 2794ce0ad92..204f17c2cf0 100644 --- a/atom/renderer/atom_renderer_client.cc +++ b/atom/renderer/atom_renderer_client.cc @@ -79,6 +79,8 @@ void AtomRendererClient::RunScriptsAtDocumentEnd( void AtomRendererClient::DidCreateScriptContext( v8::Handle context, content::RenderFrame* render_frame) { + RendererClientBase::DidCreateScriptContext(context, render_frame); + // Only allow node integration for the main frame, unless it is a devtools // extension page. if (!render_frame->IsMainFrame() && !IsDevToolsExtension(render_frame)) diff --git a/atom/renderer/atom_sandboxed_renderer_client.cc b/atom/renderer/atom_sandboxed_renderer_client.cc index 042a89bf839..67df00ad2c1 100644 --- a/atom/renderer/atom_sandboxed_renderer_client.cc +++ b/atom/renderer/atom_sandboxed_renderer_client.cc @@ -97,7 +97,7 @@ void InitializeBindings(v8::Local binding, base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); if (command_line->HasSwitch(switches::kGuestInstanceID)) b.Set(options::kGuestInstanceID, - command_line->GetSwitchValueASCII(switches::kGuestInstanceID)); + command_line->GetSwitchValueASCII(switches::kGuestInstanceID)); } class AtomSandboxedRenderFrameObserver : public AtomRenderFrameObserver { @@ -153,6 +153,8 @@ void AtomSandboxedRendererClient::RenderViewCreated( void AtomSandboxedRendererClient::DidCreateScriptContext( v8::Handle context, content::RenderFrame* render_frame) { + RendererClientBase::DidCreateScriptContext(context, render_frame); + // Only allow preload for the main frame or // For devtools we still want to run the preload_bundle script if (!render_frame->IsMainFrame() && !IsDevTools(render_frame)) diff --git a/atom/renderer/renderer_client_base.cc b/atom/renderer/renderer_client_base.cc index aca4f936031..3aa4f305f9c 100644 --- a/atom/renderer/renderer_client_base.cc +++ b/atom/renderer/renderer_client_base.cc @@ -17,7 +17,9 @@ #include "atom/renderer/guest_view_container.h" #include "atom/renderer/preferences_manager.h" #include "base/command_line.h" +#include "base/process/process_handle.h" #include "base/strings/string_split.h" +#include "base/strings/stringprintf.h" #include "chrome/renderer/media/chrome_key_systems.h" #include "chrome/renderer/pepper/pepper_helper.h" #include "chrome/renderer/printing/print_web_view_helper.h" @@ -46,6 +48,14 @@ #include "atom/common/atom_constants.h" #endif // defined(ENABLE_PDF_VIEWER) +// This is defined in later versions of Chromium, remove this if you see +// compiler complaining duplicate defines. +#if defined(OS_WIN) || defined(OS_FUCHSIA) +#define CrPRIdPid "ld" +#else +#define CrPRIdPid "d" +#endif + namespace atom { namespace { @@ -80,6 +90,19 @@ RendererClientBase::RendererClientBase() { RendererClientBase::~RendererClientBase() {} +void RendererClientBase::DidCreateScriptContext( + v8::Handle context, + content::RenderFrame* render_frame) { + // global.setHidden("contextId", `${processId}-${++nextContextId}`) + std::string context_id = base::StringPrintf( + "%" CrPRIdPid "-%d", base::GetCurrentProcId(), ++next_context_id_); + v8::Isolate* isolate = context->GetIsolate(); + v8::Local key = mate::StringToSymbol(isolate, "contextId"); + v8::Local private_key = v8::Private::ForApi(isolate, key); + v8::Local value = mate::ConvertToV8(isolate, context_id); + context->Global()->SetPrivate(context, private_key, value); +} + void RendererClientBase::AddRenderBindings( v8::Isolate* isolate, v8::Local binding_object) { diff --git a/atom/renderer/renderer_client_base.h b/atom/renderer/renderer_client_base.h index 3dea78cee77..2b13f8b038c 100644 --- a/atom/renderer/renderer_client_base.h +++ b/atom/renderer/renderer_client_base.h @@ -21,7 +21,7 @@ class RendererClientBase : public content::ContentRendererClient { ~RendererClientBase() override; virtual void DidCreateScriptContext(v8::Handle context, - content::RenderFrame* render_frame) = 0; + content::RenderFrame* render_frame); virtual void WillReleaseScriptContext(v8::Handle context, content::RenderFrame* render_frame) = 0; virtual void DidClearWindowObject(content::RenderFrame* render_frame); @@ -57,6 +57,9 @@ class RendererClientBase : public content::ContentRendererClient { private: std::unique_ptr preferences_manager_; bool isolated_world_; + + // An increasing ID used for indentifying an V8 context in this process. + int next_context_id_ = 0; }; } // namespace atom diff --git a/lib/browser/objects-registry.js b/lib/browser/objects-registry.js index b2667454152..c6afe261615 100644 --- a/lib/browser/objects-registry.js +++ b/lib/browser/objects-registry.js @@ -11,7 +11,7 @@ class ObjectsRegistry { this.storage = {} // Stores the IDs of objects referenced by WebContents. - // (webContentsId) => [id] + // (webContentsContextId) => [id] this.owners = {} } @@ -22,9 +22,10 @@ class ObjectsRegistry { const id = this.saveToStorage(obj) // Add object to the set of referenced objects. - let owner = this.owners[contextId] + const webContentsContextId = `${webContents.id}-${contextId}` + let owner = this.owners[webContentsContextId] if (!owner) { - owner = this.owners[contextId] = new Set() + owner = this.owners[webContentsContextId] = new Set() this.registerDeleteListener(webContents, contextId) } if (!owner.has(id)) { @@ -44,8 +45,9 @@ class ObjectsRegistry { // Dereference an object according to its ID. // Note that an object may be double-freed (cleared when page is reloaded, and // then garbage collected in old page). - remove (contextId, id) { - let owner = this.owners[contextId] + remove (webContents, contextId, id) { + const webContentsContextId = `${webContents.id}-${contextId}` + let owner = this.owners[webContentsContextId] if (owner) { // Remove the reference in owner. owner.delete(id) @@ -55,13 +57,14 @@ class ObjectsRegistry { } // Clear all references to objects refrenced by the WebContents. - clear (contextId) { - let owner = this.owners[contextId] + clear (webContents, contextId) { + const webContentsContextId = `${webContents.id}-${contextId}` + let owner = this.owners[webContentsContextId] if (!owner) return for (let id of owner) this.dereference(id) - delete this.owners[contextId] + delete this.owners[webContentsContextId] } // Private: Saves the object into storage and assigns an ID for it. @@ -91,13 +94,13 @@ class ObjectsRegistry { } } - // Private: Clear the storage when webContents is reloaded/navigated. + // Private: Clear the storage when renderer process is destoryed. registerDeleteListener (webContents, contextId) { const processId = webContents.getProcessId() const listener = (event, deletedProcessId) => { if (deletedProcessId === processId) { webContents.removeListener('render-view-deleted', listener) - this.clear(contextId) + this.clear(webContents, contextId) } } webContents.on('render-view-deleted', listener) diff --git a/lib/browser/rpc-server.js b/lib/browser/rpc-server.js index 043ddf37f66..a9abe72e624 100644 --- a/lib/browser/rpc-server.js +++ b/lib/browser/rpc-server.js @@ -394,12 +394,12 @@ ipcMain.on('ELECTRON_BROWSER_MEMBER_GET', function (event, contextId, id, name) }) ipcMain.on('ELECTRON_BROWSER_DEREFERENCE', function (event, contextId, id) { - objectsRegistry.remove(contextId, id) + objectsRegistry.remove(event.sender, contextId, id) }) -ipcMain.on('ELECTRON_BROWSER_CONTEXT_RELEASE', (e, contextId) => { - objectsRegistry.clear(contextId) - e.returnValue = null +ipcMain.on('ELECTRON_BROWSER_CONTEXT_RELEASE', (event, contextId) => { + objectsRegistry.clear(event.sender, contextId) + event.returnValue = null }) ipcMain.on('ELECTRON_BROWSER_GUEST_WEB_CONTENTS', function (event, contextId, guestInstanceId) { diff --git a/lib/renderer/api/remote.js b/lib/renderer/api/remote.js index 55359bad5c4..59117f8f2dd 100644 --- a/lib/renderer/api/remote.js +++ b/lib/renderer/api/remote.js @@ -10,7 +10,7 @@ const callbacksRegistry = new CallbacksRegistry() const remoteObjectCache = v8Util.createIDWeakMap() // An unique ID that can represent current context. -const contextId = v8Util.getContextId() +const contextId = v8Util.getHiddenValue(global, 'contextId') // Notify the main process when current context is going to be released. // Note that when the renderer process is destroyed, the message may not be diff --git a/lib/renderer/web-view/web-view.js b/lib/renderer/web-view/web-view.js index 59099117759..bea1a5d4200 100644 --- a/lib/renderer/web-view/web-view.js +++ b/lib/renderer/web-view/web-view.js @@ -9,7 +9,7 @@ const webViewConstants = require('./web-view-constants') const hasProp = {}.hasOwnProperty // An unique ID that can represent current context. -const contextId = v8Util.getContextId() +const contextId = v8Util.getHiddenValue(global, 'contextId') // ID generator. let nextId = 0