fix: use webContentsId with contextId together
After after using `processId-contextCounter` as contextId, it may happen that contexts in different WebContents sharing the same renderer process get the same contextId. Using webContentsId as part of key in ObjectsRegistry can fix this.
This commit is contained in:
parent
136cf389e8
commit
04b7c77951
7 changed files with 22 additions and 53 deletions
|
@ -1,19 +0,0 @@
|
|||
// Copyright (c) 2018 GitHub, Inc.
|
||||
// Use of this source code is governed by the MIT license that can be
|
||||
// found in the LICENSE file.
|
||||
|
||||
#include "atom/common/context_counter.h"
|
||||
|
||||
namespace atom {
|
||||
|
||||
namespace {
|
||||
|
||||
int g_context_id = 0;
|
||||
|
||||
} // namespace
|
||||
|
||||
int GetNextContextId() {
|
||||
return ++g_context_id;
|
||||
}
|
||||
|
||||
} // namespace atom
|
|
@ -1,15 +0,0 @@
|
|||
// Copyright (c) 2018 GitHub, Inc.
|
||||
// Use of this source code is governed by the MIT license that can be
|
||||
// found in the LICENSE file.
|
||||
|
||||
#ifndef ATOM_COMMON_CONTEXT_COUNTER_H_
|
||||
#define ATOM_COMMON_CONTEXT_COUNTER_H_
|
||||
|
||||
namespace atom {
|
||||
|
||||
// Increase the context counter, and return current count.
|
||||
int GetNextContextId();
|
||||
|
||||
} // namespace atom
|
||||
|
||||
#endif // ATOM_COMMON_CONTEXT_COUNTER_H_
|
|
@ -8,7 +8,6 @@
|
|||
#include <vector>
|
||||
|
||||
#include "atom/common/color_util.h"
|
||||
#include "atom/common/context_counter.h"
|
||||
#include "atom/common/native_mate_converters/value_converter.h"
|
||||
#include "atom/common/options_switches.h"
|
||||
#include "atom/renderer/atom_autofill_agent.h"
|
||||
|
@ -94,9 +93,9 @@ RendererClientBase::~RendererClientBase() {}
|
|||
void RendererClientBase::DidCreateScriptContext(
|
||||
v8::Handle<v8::Context> context,
|
||||
content::RenderFrame* render_frame) {
|
||||
// global.setHidden("contextId", `${processId}-${GetNextContextId()}`)
|
||||
// global.setHidden("contextId", `${processId}-${++nextContextId}`)
|
||||
std::string context_id = base::StringPrintf(
|
||||
"%" CrPRIdPid "-%d", base::GetCurrentProcId(), GetNextContextId());
|
||||
"%" CrPRIdPid "-%d", base::GetCurrentProcId(), ++next_context_id_);
|
||||
v8::Isolate* isolate = context->GetIsolate();
|
||||
v8::Local<v8::String> key = mate::StringToSymbol(isolate, "contextId");
|
||||
v8::Local<v8::Private> private_key = v8::Private::ForApi(isolate, key);
|
||||
|
|
|
@ -57,6 +57,9 @@ class RendererClientBase : public content::ContentRendererClient {
|
|||
private:
|
||||
std::unique_ptr<PreferencesManager> preferences_manager_;
|
||||
bool isolated_world_;
|
||||
|
||||
// An increasing ID used for indentifying an V8 context in this process.
|
||||
int next_context_id_ = 0;
|
||||
};
|
||||
|
||||
} // namespace atom
|
||||
|
|
|
@ -460,8 +460,6 @@
|
|||
'atom/common/color_util.h',
|
||||
'atom/common/common_message_generator.cc',
|
||||
'atom/common/common_message_generator.h',
|
||||
'atom/common/context_counter.cc',
|
||||
'atom/common/context_counter.h',
|
||||
'atom/common/crash_reporter/crash_reporter.cc',
|
||||
'atom/common/crash_reporter/crash_reporter.h',
|
||||
'atom/common/crash_reporter/crash_reporter_linux.cc',
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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) {
|
||||
|
|
Loading…
Reference in a new issue