Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitd78d01a

Browse files
authored
Core - Rewrite RenderProcess to avoid using OnBrowserDestroyed for JS eval (#5197)
- JavascriptCallbackRegistry - Get BrowserId From V8Context- Render Process - Rewrite to store root object wrapper by frame instead of per browser- Don't store JavascriptRootObjectWrapper in the browserWrapper as CEF is calling destroyed more than it should.- Revert "Tests - Simplify ShouldFailWhenIsObjectCachedCalledWithInvalidObjectName"This reverts commitf8a81ac.---------Co-authored-by: amaitland <307872+amaitland@users.noreply.github.com>
1 parentd699b6d commitd78d01a

File tree

9 files changed

+59
-121
lines changed

9 files changed

+59
-121
lines changed

‎CefSharp.BrowserSubprocess.Core/BindObjectAsyncHandler.h‎

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -26,21 +26,21 @@ namespace CefSharp
2626
private:
2727
gcroot<RegisterBoundObjectRegistry^> _callbackRegistry;
2828
gcroot<Dictionary<String^, JavascriptObject^>^> _javascriptObjects;
29-
gcroot<CefBrowserWrapper^>_browserWrapper;
29+
gcroot<JavascriptRootObjectWrapper^>_javascriptRootObjectWrapper;
3030

3131
public:
32-
BindObjectAsyncHandler(RegisterBoundObjectRegistry^ callbackRegistery, Dictionary<String^, JavascriptObject^>^ javascriptObjects,CefBrowserWrapper^ browserWrapper)
32+
BindObjectAsyncHandler(RegisterBoundObjectRegistry^ callbackRegistery, Dictionary<String^, JavascriptObject^>^ javascriptObjects,JavascriptRootObjectWrapper^ javascriptRootObjectWrapper)
3333
{
3434
_callbackRegistry = callbackRegistery;
3535
_javascriptObjects = javascriptObjects;
36-
_browserWrapper =browserWrapper;
36+
_javascriptRootObjectWrapper =javascriptRootObjectWrapper;
3737
}
3838

3939
~BindObjectAsyncHandler()
4040
{
4141
_callbackRegistry =nullptr;
4242
_javascriptObjects =nullptr;
43-
_browserWrapper =nullptr;
43+
_javascriptRootObjectWrapper =nullptr;
4444
}
4545

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

146146
returntrue;
147147
}
148148

149149
auto browser = context->GetBrowser();
150150

151-
auto rootObjectWrappers = _browserWrapper->JavascriptRootObjectWrappers;
152-
153-
JavascriptRootObjectWrapper^ rootObject;
154-
if (!rootObjectWrappers->TryGetValue(StringUtils::ToClr(frame->GetIdentifier()), rootObject))
155-
{
156-
#ifdef NETCOREAPP
157-
rootObject = gcnewJavascriptRootObjectWrapper(browser->GetIdentifier());
158-
#else
159-
rootObject = gcnewJavascriptRootObjectWrapper(browser->GetIdentifier(), _browserWrapper->BrowserProcess);
160-
#endif
161-
rootObjectWrappers->TryAdd(StringUtils::ToClr(frame->GetIdentifier()), rootObject);
162-
}
151+
JavascriptRootObjectWrapper^ rootObject = _javascriptRootObjectWrapper;
163152

164153
//Cached objects only contains a list of objects not already bound
165154
rootObject->Bind(cachedObjects, context->GetGlobal());

‎CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.cpp‎

Lines changed: 28 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ namespace CefSharp
156156

157157
//TODO: JSB: Split functions into their own classes
158158
//Browser wrapper is only used for BindObjectAsync
159-
auto bindObjAsyncFunction =CefV8Value::CreateFunction(kBindObjectAsync,newBindObjectAsyncHandler(_registerBoundObjectRegistry, _javascriptObjects,browserWrapper));
159+
auto bindObjAsyncFunction =CefV8Value::CreateFunction(kBindObjectAsync,newBindObjectAsyncHandler(_registerBoundObjectRegistry, _javascriptObjects,rootObject));
160160
auto unBindObjFunction =CefV8Value::CreateFunction(kDeleteBoundObject,newRegisterBoundObjectHandler(_javascriptObjects));
161161
auto removeObjectFromCacheFunction =CefV8Value::CreateFunction(kRemoveObjectFromCache,newRegisterBoundObjectHandler(_javascriptObjects));
162162
auto isObjectCachedFunction =CefV8Value::CreateFunction(kIsObjectCached,newRegisterBoundObjectHandler(_javascriptObjects));
@@ -220,16 +220,14 @@ namespace CefSharp
220220

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

223-
autobrowserWrapper =FindBrowserWrapper(browser->GetIdentifier());
223+
autorootObjectWrappers =_jsRootObjectWrappersByFrameId;
224224

225-
//If we no longer have abrowser wrapper reference then there's nothing we can do
226-
if (browserWrapper ==nullptr)
225+
//If we no longer have a_jsRootObjectWrappersByFrameId reference then there's nothing we can do
226+
if (Object::ReferenceEquals(rootObjectWrappers,nullptr))
227227
{
228228
return;
229229
}
230230

231-
auto rootObjectWrappers = browserWrapper->JavascriptRootObjectWrappers;
232-
233231
JavascriptRootObjectWrapper^ wrapper;
234232
if (rootObjectWrappers->TryRemove(StringUtils::ToClr(frame->GetIdentifier()), wrapper))
235233
{
@@ -305,23 +303,24 @@ namespace CefSharp
305303

306304
JavascriptRootObjectWrapper^ CefAppUnmanagedWrapper::GetJsRootObjectWrapper(int browserId, CefString& frameId)
307305
{
308-
autobrowserWrapper =FindBrowserWrapper(browserId);
306+
autorootObjectWrappers =_jsRootObjectWrappersByFrameId;
309307

310-
if (browserWrapper ==nullptr)
308+
if (Object::ReferenceEquals(rootObjectWrappers,nullptr))
311309
{
312310
returnnullptr;
313311
}
314312

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

318315
JavascriptRootObjectWrapper^ rootObject;
319316
if (!rootObjectWrappers->TryGetValue(frameIdClr, rootObject))
320317
{
321318
#ifdef NETCOREAPP
322-
rootObject = gcnewJavascriptRootObjectWrapper(browserId);
319+
rootObject = gcnewJavascriptRootObjectWrapper();
323320
#else
324-
rootObject = gcnewJavascriptRootObjectWrapper(browserId, browserWrapper->BrowserProcess);
321+
auto browserWrapper =FindBrowserWrapper(browserId);
322+
323+
rootObject = gcnewJavascriptRootObjectWrapper(browserWrapper ==nullptr ?nullptr : browserWrapper->BrowserProcess);
325324
#endif
326325
rootObjectWrappers->TryAdd(frameIdClr, rootObject);
327326
}
@@ -350,49 +349,6 @@ namespace CefSharp
350349
auto name = message->GetName();
351350
auto argList = message->GetArgumentList();
352351

353-
auto browserWrapper =FindBrowserWrapper(browser->GetIdentifier());
354-
//Error handling for missing/closed browser
355-
if (browserWrapper ==nullptr)
356-
{
357-
if (name ==kJavascriptCallbackDestroyRequest ||
358-
name ==kJavascriptRootObjectResponse ||
359-
name ==kJavascriptAsyncMethodCallResponse)
360-
{
361-
//If we can't find the browser wrapper then we'll just
362-
//ignore this as it's likely already been disposed of
363-
returntrue;
364-
}
365-
366-
CefString responseName;
367-
if (name ==kEvaluateJavascriptRequest)
368-
{
369-
responseName =kEvaluateJavascriptResponse;
370-
}
371-
elseif (name ==kJavascriptCallbackRequest)
372-
{
373-
responseName =kJavascriptCallbackResponse;
374-
}
375-
else
376-
{
377-
//TODO: Should be throw an exception here? It's likely that only a CefSharp developer would see this
378-
// when they added a new message and haven't yet implemented the render process functionality.
379-
throw gcnewException("Unsupported message type");
380-
}
381-
382-
auto callbackId =GetInt64(argList,0);
383-
auto response =CefProcessMessage::Create(responseName);
384-
auto responseArgList = response->GetArgumentList();
385-
auto errorMessage =String::Format("Request BrowserId : {0} not found it's likely the browser is already closed", browser->GetIdentifier());
386-
387-
//success: false
388-
responseArgList->SetBool(0,false);
389-
SetInt64(responseArgList,1, callbackId);
390-
responseArgList->SetString(2,StringUtils::ToNative(errorMessage));
391-
frame->SendProcessMessage(sourceProcessId, response);
392-
393-
returntrue;
394-
}
395-
396352
//these messages are roughly handled the same way
397353
if (name ==kEvaluateJavascriptRequest || name ==kJavascriptCallbackRequest)
398354
{
@@ -415,27 +371,13 @@ namespace CefSharp
415371
auto frameId =StringUtils::ToClr(frame->GetIdentifier());
416372
int64_t callbackId =GetInt64(argList,0);
417373

374+
//NOTE: In the rare case when when OnContextCreated hasn't been called we need to manually create the rootObjectWrapper
375+
//It appears that OnContextCreated is only called for pages that have javascript on them, which makes sense
376+
//as without javascript there is no need for a context.
377+
JavascriptRootObjectWrapper^ rootObjectWrapper =GetJsRootObjectWrapper(browser->GetIdentifier(), frame->GetIdentifier());
378+
418379
if (name ==kEvaluateJavascriptRequest)
419380
{
420-
JavascriptRootObjectWrapper^ rootObjectWrapper;
421-
browserWrapper->JavascriptRootObjectWrappers->TryGetValue(frameId, rootObjectWrapper);
422-
423-
//NOTE: In the rare case when when OnContextCreated hasn't been called we need to manually create the rootObjectWrapper
424-
//It appears that OnContextCreated is only called for pages that have javascript on them, which makes sense
425-
//as without javascript there is no need for a context.
426-
if (rootObjectWrapper ==nullptr)
427-
{
428-
#ifdef NETCOREAPP
429-
rootObjectWrapper = gcnewJavascriptRootObjectWrapper(browser->GetIdentifier());
430-
#else
431-
rootObjectWrapper = gcnewJavascriptRootObjectWrapper(browser->GetIdentifier(), browserWrapper->BrowserProcess);
432-
#endif
433-
434-
browserWrapper->JavascriptRootObjectWrappers->TryAdd(frameId, rootObjectWrapper);
435-
}
436-
437-
auto callbackRegistry = rootObjectWrapper->CallbackRegistry;
438-
439381
auto script = argList->GetString(1);
440382
auto scriptUrl = argList->GetString(2);
441383
auto startLine = argList->GetInt(3);
@@ -480,8 +422,17 @@ namespace CefSharp
480422
}
481423
else
482424
{
483-
auto responseArgList = response->GetArgumentList();
484-
SerializeV8Object(result, responseArgList,2, callbackRegistry);
425+
auto callbackRegistry = rootObjectWrapper ==nullptr ?nullptr : rootObjectWrapper->CallbackRegistry;
426+
427+
if (callbackRegistry ==nullptr)
428+
{
429+
errorMessage =StringUtils::ToNative("The callback registry for Frame" + frameId +" is no longer available.");
430+
}
431+
else
432+
{
433+
auto responseArgList = response->GetArgumentList();
434+
SerializeV8Object(result, responseArgList,2, callbackRegistry);
435+
}
485436
}
486437
}
487438
else
@@ -506,8 +457,6 @@ namespace CefSharp
506457
}
507458
else
508459
{
509-
JavascriptRootObjectWrapper^ rootObjectWrapper;
510-
browserWrapper->JavascriptRootObjectWrappers->TryGetValue(frameId, rootObjectWrapper);
511460
auto callbackRegistry = rootObjectWrapper ==nullptr ?nullptr : rootObjectWrapper->CallbackRegistry;
512461
if (callbackRegistry ==nullptr)
513462
{
@@ -616,7 +565,7 @@ namespace CefSharp
616565
{
617566
auto jsCallbackId =GetInt64(argList,0);
618567
JavascriptRootObjectWrapper^ rootObjectWrapper;
619-
browserWrapper->JavascriptRootObjectWrappers->TryGetValue(StringUtils::ToClr(frame->GetIdentifier()), rootObjectWrapper);
568+
_jsRootObjectWrappersByFrameId->TryGetValue(StringUtils::ToClr(frame->GetIdentifier()), rootObjectWrapper);
620569
if (rootObjectWrapper !=nullptr && rootObjectWrapper->CallbackRegistry !=nullptr)
621570
{
622571
rootObjectWrapper->CallbackRegistry->Deregister(jsCallbackId);
@@ -728,7 +677,7 @@ namespace CefSharp
728677
auto callbackId =GetInt64(argList,0);
729678

730679
JavascriptRootObjectWrapper^ rootObjectWrapper;
731-
browserWrapper->JavascriptRootObjectWrappers->TryGetValue(frameId, rootObjectWrapper);
680+
_jsRootObjectWrappersByFrameId->TryGetValue(frameId, rootObjectWrapper);
732681

733682
if (rootObjectWrapper !=nullptr)
734683
{

‎CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ namespace CefSharp
2626
gcroot<Action<CefBrowserWrapper^>^> _onBrowserCreated;
2727
gcroot<Action<CefBrowserWrapper^>^> _onBrowserDestroyed;
2828
gcroot<ConcurrentDictionary<int, CefBrowserWrapper^>^> _browserWrappers;
29+
gcroot<ConcurrentDictionary<String^, JavascriptRootObjectWrapper^>^> _jsRootObjectWrappersByFrameId;
2930
bool _focusedNodeChangedEnabled;
3031
bool _legacyBindingEnabled;
3132
bool _jsBindingApiEnabled =true;
@@ -48,6 +49,7 @@ namespace CefSharp
4849
_onBrowserCreated = onBrowserCreated;
4950
_onBrowserDestroyed = onBrowserDestroyed;
5051
_browserWrappers = gcnew ConcurrentDictionary<int, CefBrowserWrapper^>();
52+
_jsRootObjectWrappersByFrameId = gcnew ConcurrentDictionary<String^, JavascriptRootObjectWrapper^>();
5153
_focusedNodeChangedEnabled = enableFocusedNodeChanged;
5254
_javascriptObjects = gcnew Dictionary<String^, JavascriptObject^>();
5355
_registerBoundObjectRegistry = gcnewRegisterBoundObjectRegistry();
@@ -67,6 +69,17 @@ namespace CefSharp
6769

6870
_browserWrappers =nullptr;
6971
}
72+
73+
if (!Object::ReferenceEquals(_jsRootObjectWrappersByFrameId,nullptr))
74+
{
75+
foreach (JavascriptRootObjectWrapper^ rootObject in Enumerable::OfType<JavascriptRootObjectWrapper^>(_jsRootObjectWrappersByFrameId))
76+
{
77+
delete rootObject;
78+
}
79+
80+
_jsRootObjectWrappersByFrameId =nullptr;
81+
}
82+
7083
delete _onBrowserCreated;
7184
delete _onBrowserDestroyed;
7285
}

‎CefSharp.BrowserSubprocess.Core/CefBrowserWrapper.h‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,18 +29,12 @@ namespace CefSharp
2929
private:
3030
MCefRefPtr<CefBrowser> _cefBrowser;
3131

32-
internal:
33-
//Frame Identifier is used as Key
34-
property ConcurrentDictionary<String^, JavascriptRootObjectWrapper^>^ JavascriptRootObjectWrappers;
35-
3632
public:
3733
CefBrowserWrapper(CefRefPtr<CefBrowser> cefBrowser)
3834
{
3935
_cefBrowser = cefBrowser.get();
4036
BrowserId = cefBrowser->GetIdentifier();
4137
IsPopup = cefBrowser->IsPopup();
42-
43-
JavascriptRootObjectWrappers = gcnew ConcurrentDictionary<String^, JavascriptRootObjectWrapper^>();
4438
}
4539

4640
!CefBrowserWrapper()
@@ -51,16 +45,6 @@ namespace CefSharp
5145
~CefBrowserWrapper()
5246
{
5347
this->!CefBrowserWrapper();
54-
55-
if (JavascriptRootObjectWrappers !=nullptr)
56-
{
57-
foreach (KeyValuePair<String^, JavascriptRootObjectWrapper^> entry in JavascriptRootObjectWrappers)
58-
{
59-
delete entry.Value;
60-
}
61-
62-
JavascriptRootObjectWrappers =nullptr;
63-
}
6448
}
6549

6650
propertyint BrowserId;

‎CefSharp.BrowserSubprocess.Core/JavascriptCallbackRegistry.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,10 @@ namespace CefSharp
1818
JavascriptCallbackWrapper^ wrapper = gcnewJavascriptCallbackWrapper(value, context);
1919
_callbacks->TryAdd(newId, wrapper);
2020

21+
2122
auto result = gcnewJavascriptCallback();
2223
result->Id = newId;
23-
result->BrowserId =_browserId;
24+
result->BrowserId =context->GetBrowser()->GetIdentifier();
2425
result->FrameId =StringUtils::ToClr(context->GetFrame()->GetIdentifier());
2526
return result;
2627
}

‎CefSharp.BrowserSubprocess.Core/JavascriptCallbackRegistry.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,13 @@ namespace CefSharp
2020
//Is static so ids are unique to this process, which is required until #1984 is implemented
2121
//and callbacks are disposed of properly between contexts
2222
static Int64 _lastId;
23-
int _browserId;
2423
ConcurrentDictionary<Int64, JavascriptCallbackWrapper^>^ _callbacks;
2524

2625
internal:
2726
JavascriptCallbackWrapper^ FindWrapper(int64_t id);
2827

2928
public:
30-
JavascriptCallbackRegistry(int browserId) : _browserId(browserId)
29+
JavascriptCallbackRegistry()
3130
{
3231
_callbacks = gcnew ConcurrentDictionary<Int64, JavascriptCallbackWrapper^>();
3332
}

‎CefSharp.BrowserSubprocess.Core/JavascriptRootObjectWrapper.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,17 +57,17 @@ namespace CefSharp
5757

5858
public:
5959
#ifdef NETCOREAPP
60-
JavascriptRootObjectWrapper(int browserId)
60+
JavascriptRootObjectWrapper()
6161
#else
62-
JavascriptRootObjectWrapper(int browserId,IBrowserProcess^ browserProcess)
62+
JavascriptRootObjectWrapper(IBrowserProcess^ browserProcess)
6363
#endif
6464
{
6565
#ifndef NETCOREAPP
6666
_browserProcess = browserProcess;
6767
_wrappedObjects = gcnew List<JavascriptObjectWrapper^>();
6868
#endif
6969
_wrappedAsyncObjects = gcnew List<JavascriptAsyncObjectWrapper^>();
70-
_callbackRegistry = gcnewJavascriptCallbackRegistry(browserId);
70+
_callbackRegistry = gcnewJavascriptCallbackRegistry();
7171
_methodCallbacks = gcnew Dictionary<int64_t, JavascriptAsyncMethodCallback^>();
7272
}
7373

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp