Skip to content

Core - Rewrite RenderProcess to avoid using OnBrowserDestroyed for JS eval #5197

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.

Sign up for GitHub

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

Merged
amaitland merged 8 commits into from
Dec 3, 2025
Merged

Core - Rewrite RenderProcess to avoid using OnBrowserDestroyed for JS eval #5197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8b44739
JavascriptCallbackRegistry - Get BrowserId From V8Context
amaitland Nov 28, 2025
13c87c0
Render Process - Rewrite to store root object wrapper by frame instea…
amaitland Nov 30, 2025
da9cbe5
Reduce code duplication
amaitland Nov 30, 2025
7a347f5
Bug fix and review suggestion
amaitland Nov 30, 2025
f220c3d
Modify failing test
amaitland Dec 2, 2025
a07b5e1
Revert "Tests - Simplify ShouldFailWhenIsObjectCachedCalledWithInvali…
amaitland Dec 2, 2025
c052def
Minor typo in test assert
amaitland Dec 2, 2025
1e21f47
Action review comment
amaitland Dec 2, 2025
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
25 changes: 7 additions & 18 deletions CefSharp.BrowserSubprocess.Core/BindObjectAsyncHandler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,21 @@ namespace CefSharp
private:
gcroot<RegisterBoundObjectRegistry^> _callbackRegistry;
gcroot<Dictionary<String^, JavascriptObject^>^> _javascriptObjects;
gcroot<CefBrowserWrapper^> _browserWrapper;
gcroot<JavascriptRootObjectWrapper^> _javascriptRootObjectWrapper;

public:
BindObjectAsyncHandler(RegisterBoundObjectRegistry^ callbackRegistery, Dictionary<String^, JavascriptObject^>^ javascriptObjects, CefBrowserWrapper^ browserWrapper)
BindObjectAsyncHandler(RegisterBoundObjectRegistry^ callbackRegistery, Dictionary<String^, JavascriptObject^>^ javascriptObjects, JavascriptRootObjectWrapper^ javascriptRootObjectWrapper)
{
_callbackRegistry = callbackRegistery;
_javascriptObjects = javascriptObjects;
_browserWrapper = browserWrapper;
_javascriptRootObjectWrapper = javascriptRootObjectWrapper;
}

~BindObjectAsyncHandler()
{
_callbackRegistry = nullptr;
_javascriptObjects = nullptr;
_browserWrapper = nullptr;
_javascriptRootObjectWrapper = nullptr;
}

bool Execute(const CefString& name, CefRefPtr<CefV8Value> object, const CefV8ValueList& arguments, CefRefPtr<CefV8Value>& retval, CefString& exception) override
Expand Down Expand Up @@ -139,27 +139,16 @@ namespace CefSharp
//https://github.com/cefsharp/CefSharp/issues/3470
if (objectCount > 0 && cachedObjects->Count == objectCount && ignoreCache == false)
{
if (Object::ReferenceEquals(_browserWrapper, nullptr))
if (Object::ReferenceEquals(_javascriptRootObjectWrapper, nullptr))
{
exception = "BindObjectAsyncHandler::Execute - Browser wrapper null, unable to bind objects";
exception = "BindObjectAsyncHandler::Execute - _javascriptRootObjectWrapper null, unable to bind objects";

return true;
}

auto browser = context->GetBrowser();

auto rootObjectWrappers = _browserWrapper->JavascriptRootObjectWrappers;

JavascriptRootObjectWrapper^ rootObject;
if (!rootObjectWrappers->TryGetValue(StringUtils::ToClr(frame->GetIdentifier()), rootObject))
{
#ifdef NETCOREAPP
rootObject = gcnew JavascriptRootObjectWrapper(browser->GetIdentifier());
#else
rootObject = gcnew JavascriptRootObjectWrapper(browser->GetIdentifier(), _browserWrapper->BrowserProcess);
#endif
rootObjectWrappers->TryAdd(StringUtils::ToClr(frame->GetIdentifier()), rootObject);
}
JavascriptRootObjectWrapper^ rootObject = _javascriptRootObjectWrapper;

//Cached objects only contains a list of objects not already bound
rootObject->Bind(cachedObjects, context->GetGlobal());
Expand Down
107 changes: 28 additions & 79 deletions CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ namespace CefSharp

//TODO: JSB: Split functions into their own classes
//Browser wrapper is only used for BindObjectAsync
auto bindObjAsyncFunction = CefV8Value::CreateFunction(kBindObjectAsync, new BindObjectAsyncHandler(_registerBoundObjectRegistry, _javascriptObjects, browserWrapper));
auto bindObjAsyncFunction = CefV8Value::CreateFunction(kBindObjectAsync, new BindObjectAsyncHandler(_registerBoundObjectRegistry, _javascriptObjects, rootObject));
auto unBindObjFunction = CefV8Value::CreateFunction(kDeleteBoundObject, new RegisterBoundObjectHandler(_javascriptObjects));
auto removeObjectFromCacheFunction = CefV8Value::CreateFunction(kRemoveObjectFromCache, new RegisterBoundObjectHandler(_javascriptObjects));
auto isObjectCachedFunction = CefV8Value::CreateFunction(kIsObjectCached, new RegisterBoundObjectHandler(_javascriptObjects));
Expand Down Expand Up @@ -220,16 +220,14 @@ namespace CefSharp

frame->SendProcessMessage(CefProcessId::PID_BROWSER, contextReleasedMessage);

auto browserWrapper = FindBrowserWrapper(browser->GetIdentifier());
auto rootObjectWrappers = _jsRootObjectWrappersByFrameId;

//If we no longer have a browser wrapper reference then there's nothing we can do
if (browserWrapper == nullptr)
//If we no longer have a _jsRootObjectWrappersByFrameId reference then there's nothing we can do
if (Object::ReferenceEquals(rootObjectWrappers, nullptr))
{
return;
}

auto rootObjectWrappers = browserWrapper->JavascriptRootObjectWrappers;

JavascriptRootObjectWrapper^ wrapper;
if (rootObjectWrappers->TryRemove(StringUtils::ToClr(frame->GetIdentifier()), wrapper))
{
Expand Down Expand Up @@ -305,23 +303,24 @@ namespace CefSharp

JavascriptRootObjectWrapper^ CefAppUnmanagedWrapper::GetJsRootObjectWrapper(int browserId, CefString& frameId)
{
auto browserWrapper = FindBrowserWrapper(browserId);
auto rootObjectWrappers = _jsRootObjectWrappersByFrameId;

if (browserWrapper == nullptr)
if (Object::ReferenceEquals(rootObjectWrappers, nullptr))
{
return nullptr;
}

auto rootObjectWrappers = browserWrapper->JavascriptRootObjectWrappers;
auto frameIdClr = StringUtils::ToClr(frameId);

JavascriptRootObjectWrapper^ rootObject;
if (!rootObjectWrappers->TryGetValue(frameIdClr, rootObject))
{
#ifdef NETCOREAPP
rootObject = gcnew JavascriptRootObjectWrapper(browserId);
rootObject = gcnew JavascriptRootObjectWrapper();
#else
rootObject = gcnew JavascriptRootObjectWrapper(browserId, browserWrapper->BrowserProcess);
auto browserWrapper = FindBrowserWrapper(browserId);

rootObject = gcnew JavascriptRootObjectWrapper(browserWrapper == nullptr ? nullptr : browserWrapper->BrowserProcess);
#endif
rootObjectWrappers->TryAdd(frameIdClr, rootObject);
}
Expand Down Expand Up @@ -350,49 +349,6 @@ namespace CefSharp
auto name = message->GetName();
auto argList = message->GetArgumentList();

auto browserWrapper = FindBrowserWrapper(browser->GetIdentifier());
//Error handling for missing/closed browser
if (browserWrapper == nullptr)
{
if (name == kJavascriptCallbackDestroyRequest ||
name == kJavascriptRootObjectResponse ||
name == kJavascriptAsyncMethodCallResponse)
{
//If we can't find the browser wrapper then we'll just
//ignore this as it's likely already been disposed of
return true;
}

CefString responseName;
if (name == kEvaluateJavascriptRequest)
{
responseName = kEvaluateJavascriptResponse;
}
else if (name == kJavascriptCallbackRequest)
{
responseName = kJavascriptCallbackResponse;
}
else
{
//TODO: Should be throw an exception here? It's likely that only a CefSharp developer would see this
// when they added a new message and haven't yet implemented the render process functionality.
throw gcnew Exception("Unsupported message type");
}

auto callbackId = GetInt64(argList, 0);
auto response = CefProcessMessage::Create(responseName);
auto responseArgList = response->GetArgumentList();
auto errorMessage = String::Format("Request BrowserId : {0} not found it's likely the browser is already closed", browser->GetIdentifier());

//success: false
responseArgList->SetBool(0, false);
SetInt64(responseArgList, 1, callbackId);
responseArgList->SetString(2, StringUtils::ToNative(errorMessage));
frame->SendProcessMessage(sourceProcessId, response);

return true;
}

//these messages are roughly handled the same way
if (name == kEvaluateJavascriptRequest || name == kJavascriptCallbackRequest)
{
Expand All @@ -415,27 +371,13 @@ namespace CefSharp
auto frameId = StringUtils::ToClr(frame->GetIdentifier());
int64_t callbackId = GetInt64(argList, 0);

//NOTE: In the rare case when when OnContextCreated hasn't been called we need to manually create the rootObjectWrapper
//It appears that OnContextCreated is only called for pages that have javascript on them, which makes sense
//as without javascript there is no need for a context.
JavascriptRootObjectWrapper^ rootObjectWrapper = GetJsRootObjectWrapper(browser->GetIdentifier(), frame->GetIdentifier());

if (name == kEvaluateJavascriptRequest)
{
JavascriptRootObjectWrapper^ rootObjectWrapper;
browserWrapper->JavascriptRootObjectWrappers->TryGetValue(frameId, rootObjectWrapper);

//NOTE: In the rare case when when OnContextCreated hasn't been called we need to manually create the rootObjectWrapper
//It appears that OnContextCreated is only called for pages that have javascript on them, which makes sense
//as without javascript there is no need for a context.
if (rootObjectWrapper == nullptr)
{
#ifdef NETCOREAPP
rootObjectWrapper = gcnew JavascriptRootObjectWrapper(browser->GetIdentifier());
#else
rootObjectWrapper = gcnew JavascriptRootObjectWrapper(browser->GetIdentifier(), browserWrapper->BrowserProcess);
#endif

browserWrapper->JavascriptRootObjectWrappers->TryAdd(frameId, rootObjectWrapper);
}

auto callbackRegistry = rootObjectWrapper->CallbackRegistry;

auto script = argList->GetString(1);
auto scriptUrl = argList->GetString(2);
auto startLine = argList->GetInt(3);
Expand Down Expand Up @@ -480,8 +422,17 @@ namespace CefSharp
}
else
{
auto responseArgList = response->GetArgumentList();
SerializeV8Object(result, responseArgList, 2, callbackRegistry);
auto callbackRegistry = rootObjectWrapper == nullptr ? nullptr : rootObjectWrapper->CallbackRegistry;

if (callbackRegistry == nullptr)
{
errorMessage = StringUtils::ToNative("The callback registry for Frame " + frameId + " is no longer available.");
}
else
{
auto responseArgList = response->GetArgumentList();
SerializeV8Object(result, responseArgList, 2, callbackRegistry);
}
}
}
else
Expand All @@ -506,8 +457,6 @@ namespace CefSharp
}
else
{
JavascriptRootObjectWrapper^ rootObjectWrapper;
browserWrapper->JavascriptRootObjectWrappers->TryGetValue(frameId, rootObjectWrapper);
auto callbackRegistry = rootObjectWrapper == nullptr ? nullptr : rootObjectWrapper->CallbackRegistry;
if (callbackRegistry == nullptr)
{
Expand Down Expand Up @@ -616,7 +565,7 @@ namespace CefSharp
{
auto jsCallbackId = GetInt64(argList, 0);
JavascriptRootObjectWrapper^ rootObjectWrapper;
browserWrapper->JavascriptRootObjectWrappers->TryGetValue(StringUtils::ToClr(frame->GetIdentifier()), rootObjectWrapper);
_jsRootObjectWrappersByFrameId->TryGetValue(StringUtils::ToClr(frame->GetIdentifier()), rootObjectWrapper);
if (rootObjectWrapper != nullptr && rootObjectWrapper->CallbackRegistry != nullptr)
{
rootObjectWrapper->CallbackRegistry->Deregister(jsCallbackId);
Expand Down Expand Up @@ -728,7 +677,7 @@ namespace CefSharp
auto callbackId = GetInt64(argList, 0);

JavascriptRootObjectWrapper^ rootObjectWrapper;
browserWrapper->JavascriptRootObjectWrappers->TryGetValue(frameId, rootObjectWrapper);
_jsRootObjectWrappersByFrameId->TryGetValue(frameId, rootObjectWrapper);

if (rootObjectWrapper != nullptr)
{
Expand Down
13 changes: 13 additions & 0 deletions CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ namespace CefSharp
gcroot<Action<CefBrowserWrapper^>^> _onBrowserCreated;
gcroot<Action<CefBrowserWrapper^>^> _onBrowserDestroyed;
gcroot<ConcurrentDictionary<int, CefBrowserWrapper^>^> _browserWrappers;
gcroot<ConcurrentDictionary<String^, JavascriptRootObjectWrapper^>^> _jsRootObjectWrappersByFrameId;
bool _focusedNodeChangedEnabled;
bool _legacyBindingEnabled;
bool _jsBindingApiEnabled = true;
Expand All @@ -48,6 +49,7 @@ namespace CefSharp
_onBrowserCreated = onBrowserCreated;
_onBrowserDestroyed = onBrowserDestroyed;
_browserWrappers = gcnew ConcurrentDictionary<int, CefBrowserWrapper^>();
_jsRootObjectWrappersByFrameId = gcnew ConcurrentDictionary<String^, JavascriptRootObjectWrapper^>();
_focusedNodeChangedEnabled = enableFocusedNodeChanged;
_javascriptObjects = gcnew Dictionary<String^, JavascriptObject^>();
_registerBoundObjectRegistry = gcnew RegisterBoundObjectRegistry();
Expand All @@ -67,6 +69,17 @@ namespace CefSharp

_browserWrappers = nullptr;
}

if (!Object::ReferenceEquals(_jsRootObjectWrappersByFrameId, nullptr))
{
for each (JavascriptRootObjectWrapper^ rootObject in Enumerable::OfType<JavascriptRootObjectWrapper^>(_jsRootObjectWrappersByFrameId))
{
delete rootObject;
}

_jsRootObjectWrappersByFrameId = nullptr;
}

delete _onBrowserCreated;
delete _onBrowserDestroyed;
}
Expand Down
16 changes: 0 additions & 16 deletions CefSharp.BrowserSubprocess.Core/CefBrowserWrapper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,12 @@ namespace CefSharp
private:
MCefRefPtr<CefBrowser> _cefBrowser;

internal:
//Frame Identifier is used as Key
property ConcurrentDictionary<String^, JavascriptRootObjectWrapper^>^ JavascriptRootObjectWrappers;

public:
CefBrowserWrapper(CefRefPtr<CefBrowser> cefBrowser)
{
_cefBrowser = cefBrowser.get();
BrowserId = cefBrowser->GetIdentifier();
IsPopup = cefBrowser->IsPopup();

JavascriptRootObjectWrappers = gcnew ConcurrentDictionary<String^, JavascriptRootObjectWrapper^>();
}

!CefBrowserWrapper()
Expand All @@ -51,16 +45,6 @@ namespace CefSharp
~CefBrowserWrapper()
{
this->!CefBrowserWrapper();

if (JavascriptRootObjectWrappers != nullptr)
{
for each (KeyValuePair<String^, JavascriptRootObjectWrapper^> entry in JavascriptRootObjectWrappers)
{
delete entry.Value;
}

JavascriptRootObjectWrappers = nullptr;
}
}

property int BrowserId;
Expand Down
3 changes: 2 additions & 1 deletion CefSharp.BrowserSubprocess.Core/JavascriptCallbackRegistry.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ namespace CefSharp
JavascriptCallbackWrapper^ wrapper = gcnew JavascriptCallbackWrapper(value, context);
_callbacks->TryAdd(newId, wrapper);


auto result = gcnew JavascriptCallback();
result->Id = newId;
result->BrowserId = _browserId;
result->BrowserId = context->GetBrowser()->GetIdentifier();
result->FrameId = StringUtils::ToClr(context->GetFrame()->GetIdentifier());
return result;
}
Expand Down
3 changes: 1 addition & 2 deletions CefSharp.BrowserSubprocess.Core/JavascriptCallbackRegistry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,13 @@ namespace CefSharp
//Is static so ids are unique to this process, which is required until #1984 is implemented
//and callbacks are disposed of properly between contexts
static Int64 _lastId;
int _browserId;
ConcurrentDictionary<Int64, JavascriptCallbackWrapper^>^ _callbacks;

internal:
JavascriptCallbackWrapper^ FindWrapper(int64_t id);

public:
JavascriptCallbackRegistry(int browserId) : _browserId(browserId)
JavascriptCallbackRegistry()
{
_callbacks = gcnew ConcurrentDictionary<Int64, JavascriptCallbackWrapper^>();
}
Expand Down
6 changes: 3 additions & 3 deletions CefSharp.BrowserSubprocess.Core/JavascriptRootObjectWrapper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,17 @@ namespace CefSharp

public:
#ifdef NETCOREAPP
JavascriptRootObjectWrapper(int browserId)
JavascriptRootObjectWrapper()
#else
JavascriptRootObjectWrapper(int browserId, IBrowserProcess^ browserProcess)
JavascriptRootObjectWrapper(IBrowserProcess^ browserProcess)
#endif
{
#ifndef NETCOREAPP
_browserProcess = browserProcess;
_wrappedObjects = gcnew List<JavascriptObjectWrapper^>();
#endif
_wrappedAsyncObjects = gcnew List<JavascriptAsyncObjectWrapper^>();
_callbackRegistry = gcnew JavascriptCallbackRegistry(browserId);
_callbackRegistry = gcnew JavascriptCallbackRegistry();
_methodCallbacks = gcnew Dictionary<int64_t, JavascriptAsyncMethodCallback^>();
}

Expand Down
Loading