diff --git a/CefSharp.BrowserSubprocess.Core/BindObjectAsyncHandler.h b/CefSharp.BrowserSubprocess.Core/BindObjectAsyncHandler.h index 9ece880b2..1c72b9fed 100644 --- a/CefSharp.BrowserSubprocess.Core/BindObjectAsyncHandler.h +++ b/CefSharp.BrowserSubprocess.Core/BindObjectAsyncHandler.h @@ -25,11 +25,11 @@ namespace CefSharp { private: gcroot _callbackRegistry; - gcroot^> _javascriptObjects; + gcroot^> _javascriptObjects; gcroot _browserWrapper; public: - BindObjectAsyncHandler(RegisterBoundObjectRegistry^ callbackRegistery, Dictionary^ javascriptObjects, CefBrowserWrapper^ browserWrapper) + BindObjectAsyncHandler(RegisterBoundObjectRegistry^ callbackRegistery, IDictionary^ javascriptObjects, CefBrowserWrapper^ browserWrapper) { _callbackRegistry = callbackRegistery; _javascriptObjects = javascriptObjects; diff --git a/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.cpp b/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.cpp index a7b2ceb1f..0af73368a 100644 --- a/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.cpp +++ b/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.cpp @@ -82,20 +82,7 @@ namespace CefSharp { auto javascriptObjects = DeserializeJsObjects(objects, 0); - for each (JavascriptObject ^ obj in Enumerable::OfType(javascriptObjects)) - { - //Using LegacyBinding with multiple ChromiumWebBrowser instances that share the same - //render process and using LegacyBinding will cause problems for the limited caching implementation - //that exists at the moment, for now we'll remove an object if already exists, same behaviour - //as the new binding method. - //TODO: This should be removed when https://github.com/cefsharp/CefSharp/issues/2306 - //Is complete as objects will be stored at the browser level - if (_javascriptObjects->ContainsKey(obj->JavascriptName)) - { - _javascriptObjects->Remove(obj->JavascriptName); - } - _javascriptObjects->Add(obj->JavascriptName, obj); - } + _javascriptObjectCache->InsertOrUpdate(browser->GetIdentifier(), javascriptObjects); } } @@ -118,6 +105,8 @@ namespace CefSharp _onBrowserDestroyed->Invoke(wrapper); delete wrapper; } + + _javascriptObjectCache->ClearCache(browser->GetIdentifier()); }; void CefAppUnmanagedWrapper::OnContextCreated(CefRefPtr browser, CefRefPtr frame, CefRefPtr context) @@ -141,9 +130,11 @@ namespace CefSharp if (_legacyBindingEnabled) { - if (_javascriptObjects->Count > 0 && rootObject != nullptr) + auto values = _javascriptObjectCache->GetCacheValues(browser->GetIdentifier()); + + if (values->Count > 0 && rootObject != nullptr) { - rootObject->Bind(_javascriptObjects->Values, context->GetGlobal()); + rootObject->Bind(values, context->GetGlobal()); } } @@ -153,13 +144,14 @@ namespace CefSharp auto global = context->GetGlobal(); auto browserWrapper = FindBrowserWrapper(browser->GetIdentifier()); auto processId = System::Diagnostics::Process::GetCurrentProcess()->Id; + auto objectCache = _javascriptObjectCache->GetCache(browser->GetIdentifier()); //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 unBindObjFunction = CefV8Value::CreateFunction(kDeleteBoundObject, new RegisterBoundObjectHandler(_javascriptObjects)); - auto removeObjectFromCacheFunction = CefV8Value::CreateFunction(kRemoveObjectFromCache, new RegisterBoundObjectHandler(_javascriptObjects)); - auto isObjectCachedFunction = CefV8Value::CreateFunction(kIsObjectCached, new RegisterBoundObjectHandler(_javascriptObjects)); + auto bindObjAsyncFunction = CefV8Value::CreateFunction(kBindObjectAsync, new BindObjectAsyncHandler(_registerBoundObjectRegistry, objectCache, browserWrapper)); + auto unBindObjFunction = CefV8Value::CreateFunction(kDeleteBoundObject, new RegisterBoundObjectHandler(objectCache)); + auto removeObjectFromCacheFunction = CefV8Value::CreateFunction(kRemoveObjectFromCache, new RegisterBoundObjectHandler(objectCache)); + auto isObjectCachedFunction = CefV8Value::CreateFunction(kIsObjectCached, new RegisterBoundObjectHandler(objectCache)); auto postMessageFunction = CefV8Value::CreateFunction(kPostMessage, new JavascriptPostMessageHandler(rootObject == nullptr ? nullptr : rootObject->CallbackRegistry)); auto promiseHandlerFunction = CefV8Value::CreateFunction(kSendEvalScriptResponse, new JavascriptPromiseHandler()); @@ -633,15 +625,7 @@ namespace CefSharp auto javascriptObjects = DeserializeJsObjects(argList, 1); //Caching of JavascriptObjects - //TODO: JSB Should caching be configurable? On a per object basis? - for each (JavascriptObject ^ obj in Enumerable::OfType(javascriptObjects)) - { - if (_javascriptObjects->ContainsKey(obj->JavascriptName)) - { - _javascriptObjects->Remove(obj->JavascriptName); - } - _javascriptObjects->Add(obj->JavascriptName, obj); - } + _javascriptObjectCache->InsertOrUpdate(browser->GetIdentifier(), javascriptObjects); auto rootObject = GetJsRootObjectWrapper(browser->GetIdentifier(), frame->GetIdentifier()); diff --git a/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h b/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h index 38f8da288..ecda32cae 100644 --- a/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h +++ b/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h @@ -35,25 +35,33 @@ namespace CefSharp CefString _jsBindingPropertyNameCamelCase; // The serialized registered object data waiting to be used. - gcroot^> _javascriptObjects; + gcroot _javascriptObjectCache; gcroot _registerBoundObjectRegistry; public: static const CefString kPromiseCreatorScript; - CefAppUnmanagedWrapper(IRenderProcessHandler^ handler, List^ schemes, bool enableFocusedNodeChanged, Action^ onBrowserCreated, Action^ onBrowserDestroyed) : SubProcessApp(schemes) + CefAppUnmanagedWrapper(IRenderProcessHandler^ handler, List^ schemes, bool jsbCachePerBrowser, bool enableFocusedNodeChanged, Action^ onBrowserCreated, Action^ onBrowserDestroyed) : SubProcessApp(schemes) { _handler = handler; _onBrowserCreated = onBrowserCreated; _onBrowserDestroyed = onBrowserDestroyed; _browserWrappers = gcnew ConcurrentDictionary(); _focusedNodeChangedEnabled = enableFocusedNodeChanged; - _javascriptObjects = gcnew Dictionary(); _registerBoundObjectRegistry = gcnew RegisterBoundObjectRegistry(); _legacyBindingEnabled = false; _jsBindingPropertyName = "CefSharp"; _jsBindingPropertyNameCamelCase = "cefSharp"; + + if (jsbCachePerBrowser) + { + _javascriptObjectCache = gcnew PerBrowserJavaScriptObjectCache(); + } + else + { + _javascriptObjectCache = gcnew LegacyJavaScriptObjectCache(); + } } ~CefAppUnmanagedWrapper() diff --git a/CefSharp.BrowserSubprocess.Core/RegisterBoundObjectHandler.h b/CefSharp.BrowserSubprocess.Core/RegisterBoundObjectHandler.h index 525bf1953..e3373d7c6 100644 --- a/CefSharp.BrowserSubprocess.Core/RegisterBoundObjectHandler.h +++ b/CefSharp.BrowserSubprocess.Core/RegisterBoundObjectHandler.h @@ -27,10 +27,10 @@ namespace CefSharp private class RegisterBoundObjectHandler : public CefV8Handler { private: - gcroot^> _javascriptObjects; + gcroot^> _javascriptObjects; public: - RegisterBoundObjectHandler(Dictionary^ javascriptObjects) + RegisterBoundObjectHandler(IDictionary^ javascriptObjects) { _javascriptObjects = javascriptObjects; } diff --git a/CefSharp.BrowserSubprocess.Core/SubProcess.h b/CefSharp.BrowserSubprocess.Core/SubProcess.h index 8f988f561..dfcd52838 100644 --- a/CefSharp.BrowserSubprocess.Core/SubProcess.h +++ b/CefSharp.BrowserSubprocess.Core/SubProcess.h @@ -33,9 +33,10 @@ namespace CefSharp auto onBrowserCreated = gcnew Action(this, &SubProcess::OnBrowserCreated); auto onBrowserDestroyed = gcnew Action(this, &SubProcess::OnBrowserDestroyed); auto schemes = CefCustomScheme::ParseCommandLineArguments(args); + auto jsbCachePerBrowser = CommandLineArgsParser::HasArgument(args, CefSharpArguments::PerBrowserJavaScriptObjectCache); auto enableFocusedNodeChanged = CommandLineArgsParser::HasArgument(args, CefSharpArguments::FocusedNodeChangedEnabledArgument); - _cefApp = new CefAppUnmanagedWrapper(handler, schemes, enableFocusedNodeChanged, onBrowserCreated, onBrowserDestroyed); + _cefApp = new CefAppUnmanagedWrapper(handler, schemes, jsbCachePerBrowser, enableFocusedNodeChanged, onBrowserCreated, onBrowserDestroyed); } !SubProcess() diff --git a/CefSharp.Test/JavascriptBinding/LegacyJavaScriptObjectCacheTests.cs b/CefSharp.Test/JavascriptBinding/LegacyJavaScriptObjectCacheTests.cs new file mode 100644 index 000000000..8a3d6aef8 --- /dev/null +++ b/CefSharp.Test/JavascriptBinding/LegacyJavaScriptObjectCacheTests.cs @@ -0,0 +1,118 @@ +using System.Collections.Generic; +using CefSharp.Internals; +using Xunit; + +namespace CefSharp.Test.JavascriptBinding +{ + public class LegacyJavaScriptObjectCacheTests + { + private const int BrowserId = 1; + + [Fact] + public void InsertOrUpdateShouldAddObjectsToCache() + { + var cache = new LegacyJavaScriptObjectCache(); + var javascriptObjects = new List + { + new JavascriptObject { Name = "Object1" }, + new JavascriptObject { Name = "Object2" } + }; + + cache.InsertOrUpdate(BrowserId, javascriptObjects); + + var cachedValues = cache.GetCacheValues(BrowserId); + Assert.Contains(javascriptObjects[0], cachedValues); + Assert.Contains(javascriptObjects[1], cachedValues); + } + + [Fact] + public void GetCacheValuesShouldReturnAllCachedObjects() + { + var cache = new LegacyJavaScriptObjectCache(); + var javascriptObjects = new List + { + new JavascriptObject { Name = "Object1" }, + new JavascriptObject { Name = "Object2" } + }; + cache.InsertOrUpdate(BrowserId, javascriptObjects); + + var cachedValues = cache.GetCacheValues(BrowserId); + + Assert.Equal(2, cachedValues.Count); + } + + [Fact] + public void GetCacheShouldReturnUnderlyingDictionary() + { + var cache = new LegacyJavaScriptObjectCache(); + var javascriptObjects = new List + { + new JavascriptObject { Name = "Object1" } + }; + cache.InsertOrUpdate(BrowserId, javascriptObjects); + + var cachedDictionary = cache.GetCache(BrowserId); + + Assert.Single(cachedDictionary); + Assert.True(cachedDictionary.ContainsKey("Object1")); + } + + [Fact] + public void InsertOrUpdateShouldReplaceExistingObjects() + { + var cache = new LegacyJavaScriptObjectCache(); + var initialObjects = new List + { + new JavascriptObject { Name = "Object1" } + }; + var updatedObjects = new List + { + new JavascriptObject { Name = "Object1" } + }; + cache.InsertOrUpdate(BrowserId, initialObjects); + + cache.InsertOrUpdate(BrowserId, updatedObjects); + + var cachedValues = cache.GetCacheValues(BrowserId); + Assert.DoesNotContain(initialObjects[0], cachedValues); + Assert.Contains(updatedObjects[0], cachedValues); + } + + [Fact] + public void InsertOrUpdateShouldAppendObjectsWithDifferentNames() + { + var cache = new LegacyJavaScriptObjectCache(); + var initialObjects = new List + { + new JavascriptObject { Name = "Object1" } + }; + var updatedObjects = new List + { + new JavascriptObject { Name = "Object2" } + }; + cache.InsertOrUpdate(BrowserId, initialObjects); + + cache.InsertOrUpdate(BrowserId, updatedObjects); + + var cachedValues = cache.GetCacheValues(BrowserId); + Assert.Contains(initialObjects[0], cachedValues); + Assert.Contains(updatedObjects[0], cachedValues); + } + + [Fact] + public void ClearCacheShouldDoNothing() + { + var cache = new LegacyJavaScriptObjectCache(); + var javascriptObjects = new List + { + new JavascriptObject { Name = "Object1" } + }; + cache.InsertOrUpdate(BrowserId, javascriptObjects); + + cache.ClearCache(BrowserId); + + var cachedValues = cache.GetCacheValues(BrowserId); + Assert.Contains(javascriptObjects[0], cachedValues); + } + } +} diff --git a/CefSharp.Test/JavascriptBinding/PerBrowserJavaScriptObjectCacheTests.cs b/CefSharp.Test/JavascriptBinding/PerBrowserJavaScriptObjectCacheTests.cs new file mode 100644 index 000000000..f9952036e --- /dev/null +++ b/CefSharp.Test/JavascriptBinding/PerBrowserJavaScriptObjectCacheTests.cs @@ -0,0 +1,128 @@ +using System.Collections.Generic; +using System.Linq; +using CefSharp.Internals; +using Xunit; + +namespace CefSharp.Test.JavascriptBinding +{ + public class PerBrowserJavaScriptObjectCacheTests + { + private const int TestBrowserId = 1; + + [Fact] + public void ClearCacheRemovesCacheForBrowserId() + { + var cache = new PerBrowserJavaScriptObjectCache(); + cache.InsertOrUpdate(TestBrowserId, new List + { + new JavascriptObject { Name = "TestObject" } + }); + + cache.ClearCache(TestBrowserId); + + Assert.Empty(cache.GetCacheValues(TestBrowserId)); + } + + [Fact] + public void InsertOrUpdateAddsOrUpdatesObjectsInCache() + { + var cache = new PerBrowserJavaScriptObjectCache(); + var javascriptObjects = new List + { + new JavascriptObject { Name = "Object1" }, + new JavascriptObject { Name = "Object2" } + }; + + cache.InsertOrUpdate(TestBrowserId, javascriptObjects); + + var cachedObjects = cache.GetCacheValues(TestBrowserId); + Assert.Equal(2, cachedObjects.Count); + } + + [Fact] + public void GetCacheValuesReturnsEmptyCollectionWhenNoCacheExists() + { + var cache = new PerBrowserJavaScriptObjectCache(); + + var cachedObjects = cache.GetCacheValues(TestBrowserId); + + Assert.Empty(cachedObjects); + } + + [Fact] + public void GetCacheReturnsCacheDictionaryForBrowserId() + { + var cache = new PerBrowserJavaScriptObjectCache(); + var javascriptObjects = new List + { + new JavascriptObject { Name = "Object1" } + }; + cache.InsertOrUpdate(TestBrowserId, javascriptObjects); + + var cachedDictionary = cache.GetCache(TestBrowserId); + + Assert.Single(cachedDictionary); + Assert.True(cachedDictionary.ContainsKey("Object1")); + } + + [Fact] + public void ClearCacheDoesNotAffectOtherBrowserIds() + { + var cache = new PerBrowserJavaScriptObjectCache(); + cache.InsertOrUpdate(TestBrowserId, new List + { + new JavascriptObject { Name = "Object1" } + }); + cache.InsertOrUpdate(2, new List + { + new JavascriptObject { Name = "Object2" } + }); + + cache.ClearCache(TestBrowserId); + + Assert.Empty(cache.GetCacheValues(TestBrowserId)); + Assert.NotEmpty(cache.GetCacheValues(2)); + } + + [Fact] + public void InsertOrUpdateOverwritesExistingCacheForBrowserId() + { + var cache = new PerBrowserJavaScriptObjectCache(); + + cache.InsertOrUpdate(TestBrowserId, new List + { + new JavascriptObject { Name = "NewObject", Value = 1 } + }); + + cache.InsertOrUpdate(TestBrowserId, new List + { + new JavascriptObject { Name = "NewObject", Value = 2 } + }); + + var cachedObjects = cache.GetCacheValues(TestBrowserId); + Assert.Single(cachedObjects); + Assert.Equal(2, cachedObjects.First().Value); + } + + [Fact] + public void GetCacheReturnsEmptyForNonExistentBrowserId() + { + var cache = new PerBrowserJavaScriptObjectCache(); + + var cachedDictionary = cache.GetCache(999); + + Assert.Empty(cachedDictionary); + } + + [Fact] + public void InsertOrUpdateHandlesEmptyObjectList() + { + var cache = new PerBrowserJavaScriptObjectCache(); + + cache.InsertOrUpdate(TestBrowserId, new List()); + + var cachedObjects = cache.GetCacheValues(TestBrowserId); + Assert.Empty(cachedObjects); + } + } +} diff --git a/CefSharp/Internals/CefSharpArguments.cs b/CefSharp/Internals/CefSharpArguments.cs index 707c817ae..6684e5760 100644 --- a/CefSharp/Internals/CefSharpArguments.cs +++ b/CefSharp/Internals/CefSharpArguments.cs @@ -10,6 +10,7 @@ public static class CefSharpArguments public const string HostProcessIdArgument = "--host-process-id"; public const string CustomSchemeArgument = "--custom-scheme"; public const string FocusedNodeChangedEnabledArgument = "--focused-node-enabled"; + public const string PerBrowserJavaScriptObjectCache = "--jsb-cache-perbrowser"; public const string SubProcessTypeArgument = "--type"; public const string ExitIfParentProcessClosed = "--cefsharpexitsub"; } diff --git a/CefSharp/Internals/IJavaScriptObjectCache.cs b/CefSharp/Internals/IJavaScriptObjectCache.cs new file mode 100644 index 000000000..649a850e2 --- /dev/null +++ b/CefSharp/Internals/IJavaScriptObjectCache.cs @@ -0,0 +1,42 @@ +// Copyright © 2023 The CefSharp Authors. All rights reserved. +// +// Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. + +using System.Collections.Generic; + +namespace CefSharp.Internals +{ + /// + /// Render Process JavaScript Binding (JSB) object cache + /// + public interface IJavaScriptObjectCache + { + /// + /// Remove the Browser specific Cache + /// + /// browser Id + void ClearCache(int browserId); + /// + /// Gets the browser specific cache (dictionary) based on it's Id + /// + /// browser Id + /// Dictionary of cache 's. + /// + IDictionary GetCache(int browserId); + /// + /// Gets a collection of s + /// for the given + /// + /// browser Id + /// Collection of current bound objects for the browser + /// + ICollection GetCacheValues(int browserId); + /// + /// Insert or Update the within the Cache + /// + /// browser id + /// JavaScript object + /// + void InsertOrUpdate(int browserId, IReadOnlyCollection javascriptObjects); + } +} diff --git a/CefSharp/Internals/LegacyJavaScriptObjectCache.cs b/CefSharp/Internals/LegacyJavaScriptObjectCache.cs new file mode 100644 index 000000000..c45ffefc0 --- /dev/null +++ b/CefSharp/Internals/LegacyJavaScriptObjectCache.cs @@ -0,0 +1,45 @@ +// Copyright © 2023 The CefSharp Authors. All rights reserved. +// +// Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. + +using System.Collections.Generic; + +namespace CefSharp.Internals +{ + /// + /// Render Process JavaScript Binding (JSB) object cache + /// Legacy Behaviour, objects are cache per process. + /// + public class LegacyJavaScriptObjectCache : IJavaScriptObjectCache + { + private readonly Dictionary cache + = new Dictionary(); + + /// + public void ClearCache(int browserId) + { + // NO OP + } + + /// + public void InsertOrUpdate(int browserId, IReadOnlyCollection javascriptObjects) + { + foreach (var obj in javascriptObjects) + { + cache[obj.Name] = obj; + } + } + + /// + public ICollection GetCacheValues(int browserId) + { + return cache.Values; + } + + /// + public IDictionary GetCache(int browserId) + { + return cache; + } + } +} diff --git a/CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs b/CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs new file mode 100644 index 000000000..8f546fe74 --- /dev/null +++ b/CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs @@ -0,0 +1,69 @@ +// Copyright © 2023 The CefSharp Authors. All rights reserved. +// +// Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. + +using System; +using System.Collections.Generic; + +namespace CefSharp.Internals +{ + /// + /// Render Process JavaScript Binding (JSB) object cache + /// Stores bound objects per CefBrowser. + /// + public class PerBrowserJavaScriptObjectCache : IJavaScriptObjectCache + { + private readonly Dictionary> cache + = new Dictionary>(); + + /// + public void ClearCache(int browserId) + { + cache.Remove(browserId); + } + + /// + public void InsertOrUpdate(int browserId, IReadOnlyCollection javascriptObjects) + { + var dict = GetCacheInternal(browserId); + + foreach (var obj in javascriptObjects) + { + dict[obj.Name] = obj; + } + } + + /// + public ICollection GetCacheValues(int browserId) + { + if (cache.TryGetValue(browserId, out var dict)) + { + return dict.Values; + } + + return new List(); + } + + /// + public IDictionary GetCache(int browserId) + { + var dict = GetCacheInternal(browserId); + + return dict; + } + + private IDictionary GetCacheInternal(int browserId) + { + Dictionary dict; + + if (!cache.TryGetValue(browserId, out dict)) + { + dict = new Dictionary(); + + cache.Add(browserId, dict); + } + + return dict; + } + } +}