From efd7993ed7e23ea5abb2dbad5e62bde549d1915a Mon Sep 17 00:00:00 2001 From: Alex Maitland Date: Sun, 9 Apr 2023 21:02:45 +1000 Subject: [PATCH 1/6] JavaScript Binding - Cache objects on a per browser basis Issue #2306 --- .../CefAppUnmanagedWrapper.cpp | 42 +++------ .../CefAppUnmanagedWrapper.h | 4 +- .../RenderProcess/JavaScriptObjectCache.cs | 94 +++++++++++++++++++ 3 files changed, 109 insertions(+), 31 deletions(-) create mode 100644 CefSharp/RenderProcess/JavaScriptObjectCache.cs diff --git a/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.cpp b/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.cpp index a7b2ceb1f3..0af73368ab 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 38f8da2881..9f4bef3c90 100644 --- a/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h +++ b/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h @@ -35,7 +35,7 @@ namespace CefSharp CefString _jsBindingPropertyNameCamelCase; // The serialized registered object data waiting to be used. - gcroot^> _javascriptObjects; + gcroot _javascriptObjectCache; gcroot _registerBoundObjectRegistry; @@ -49,7 +49,7 @@ namespace CefSharp _onBrowserDestroyed = onBrowserDestroyed; _browserWrappers = gcnew ConcurrentDictionary(); _focusedNodeChangedEnabled = enableFocusedNodeChanged; - _javascriptObjects = gcnew Dictionary(); + _javascriptObjectCache = gcnew JavaScriptObjectCache(); _registerBoundObjectRegistry = gcnew RegisterBoundObjectRegistry(); _legacyBindingEnabled = false; _jsBindingPropertyName = "CefSharp"; diff --git a/CefSharp/RenderProcess/JavaScriptObjectCache.cs b/CefSharp/RenderProcess/JavaScriptObjectCache.cs new file mode 100644 index 0000000000..2e6eae258f --- /dev/null +++ b/CefSharp/RenderProcess/JavaScriptObjectCache.cs @@ -0,0 +1,94 @@ +// 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; +using CefSharp.Internals; + +namespace CefSharp.RenderProcess +{ + /// + /// JavaScriptObjectCache is used in the RenderProcess to cache + /// JavaScript bound objects at a CefBrowser level + /// + public class JavaScriptObjectCache + { + private readonly Dictionary> cache + = new Dictionary>(); + + /// + /// Remove the Browser specific Cache + /// + /// browser Id + public void ClearCache(int browserId) + { + cache.Remove(browserId); + } + + /// + /// Insert or Update the within the Cache + /// + /// browser id + /// JavaScript object + /// + public void InsertOrUpdate(int browserId, IList javascriptObjects) + { + var dict = GetCacheInternal(browserId); + + foreach (var obj in javascriptObjects) + { + if (dict.ContainsKey(obj.Name)) + { + dict.Remove(obj.Name); + } + + dict.Add(obj.Name, obj); + } + } + + /// + /// Gets a collection of s + /// for the given + /// + /// browser Id + /// Collection of current bound objects for the browser + /// + public ICollection GetCacheValues(int browserId) + { + if (cache.TryGetValue(browserId, out var dict)) + { + return dict.Values; + } + + return new List(); + } + + /// + /// Gets the browser specific cache (dictionary) based on it's Id + /// + /// browser Id + /// Dictionary of cache 's. + /// + public Dictionary GetCache(int browserId) + { + var dict = GetCacheInternal(browserId); + + return dict; + } + + private Dictionary GetCacheInternal(int browserId) + { + Dictionary dict; + + if (!cache.TryGetValue(browserId, out dict)) + { + dict = new Dictionary(); + + cache.Add(browserId, dict); + } + + return dict; + } + } +} From 79c9b4529546aa6b8dde239cc2c77c7e9c29f3ed Mon Sep 17 00:00:00 2001 From: Alex Maitland Date: Wed, 12 Apr 2023 13:38:46 +1000 Subject: [PATCH 2/6] Legacy and PerBrowser implementations --- .../CefAppUnmanagedWrapper.h | 4 +- CefSharp/Internals/IJavaScriptObjectCache.cs | 42 ++++++++++++++++ .../Internals/LegacyJavaScriptObjectCache.cs | 50 +++++++++++++++++++ .../PerBrowserJavaScriptObjectCache.cs} | 38 ++++---------- 4 files changed, 103 insertions(+), 31 deletions(-) create mode 100644 CefSharp/Internals/IJavaScriptObjectCache.cs create mode 100644 CefSharp/Internals/LegacyJavaScriptObjectCache.cs rename CefSharp/{RenderProcess/JavaScriptObjectCache.cs => Internals/PerBrowserJavaScriptObjectCache.cs} (53%) diff --git a/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h b/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h index 9f4bef3c90..04be3c8d0b 100644 --- a/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h +++ b/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h @@ -35,7 +35,7 @@ namespace CefSharp CefString _jsBindingPropertyNameCamelCase; // The serialized registered object data waiting to be used. - gcroot _javascriptObjectCache; + gcroot _javascriptObjectCache; gcroot _registerBoundObjectRegistry; @@ -49,7 +49,7 @@ namespace CefSharp _onBrowserDestroyed = onBrowserDestroyed; _browserWrappers = gcnew ConcurrentDictionary(); _focusedNodeChangedEnabled = enableFocusedNodeChanged; - _javascriptObjectCache = gcnew JavaScriptObjectCache(); + _javascriptObjectCache = gcnew LegacyJavaScriptObjectCache(); _registerBoundObjectRegistry = gcnew RegisterBoundObjectRegistry(); _legacyBindingEnabled = false; _jsBindingPropertyName = "CefSharp"; diff --git a/CefSharp/Internals/IJavaScriptObjectCache.cs b/CefSharp/Internals/IJavaScriptObjectCache.cs new file mode 100644 index 0000000000..16ba921112 --- /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. + /// + Dictionary 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, IList javascriptObjects); + } +} diff --git a/CefSharp/Internals/LegacyJavaScriptObjectCache.cs b/CefSharp/Internals/LegacyJavaScriptObjectCache.cs new file mode 100644 index 0000000000..704a730391 --- /dev/null +++ b/CefSharp/Internals/LegacyJavaScriptObjectCache.cs @@ -0,0 +1,50 @@ +// 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, IList javascriptObjects) + { + foreach (var obj in javascriptObjects) + { + if (cache.ContainsKey(obj.Name)) + { + cache.Remove(obj.Name); + } + + cache.Add(obj.Name, obj); + } + } + + /// + public ICollection GetCacheValues(int browserId) + { + return cache.Values; + } + + /// + public Dictionary GetCache(int browserId) + { + return cache; + } + } +} diff --git a/CefSharp/RenderProcess/JavaScriptObjectCache.cs b/CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs similarity index 53% rename from CefSharp/RenderProcess/JavaScriptObjectCache.cs rename to CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs index 2e6eae258f..2cb8fd0617 100644 --- a/CefSharp/RenderProcess/JavaScriptObjectCache.cs +++ b/CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs @@ -4,35 +4,26 @@ using System; using System.Collections.Generic; -using CefSharp.Internals; -namespace CefSharp.RenderProcess +namespace CefSharp.Internals { /// - /// JavaScriptObjectCache is used in the RenderProcess to cache - /// JavaScript bound objects at a CefBrowser level + /// Render Process JavaScript Binding (JSB) object cache + /// Stores bound objects per CefBrowser. /// - public class JavaScriptObjectCache + public class PerBrowserJavaScriptObjectCache : IJavaScriptObjectCache { private readonly Dictionary> cache = new Dictionary>(); - /// - /// Remove the Browser specific Cache - /// - /// browser Id + /// public void ClearCache(int browserId) { cache.Remove(browserId); } - /// - /// Insert or Update the within the Cache - /// - /// browser id - /// JavaScript object - /// - public void InsertOrUpdate(int browserId, IList javascriptObjects) + /// + public void InsertOrUpdate(int browserId, IList javascriptObjects) { var dict = GetCacheInternal(browserId); @@ -47,13 +38,7 @@ public void InsertOrUpdate(int browserId, IList javascriptObj } } - /// - /// Gets a collection of s - /// for the given - /// - /// browser Id - /// Collection of current bound objects for the browser - /// + /// public ICollection GetCacheValues(int browserId) { if (cache.TryGetValue(browserId, out var dict)) @@ -64,12 +49,7 @@ public ICollection GetCacheValues(int browserId) return new List(); } - /// - /// Gets the browser specific cache (dictionary) based on it's Id - /// - /// browser Id - /// Dictionary of cache 's. - /// + /// public Dictionary GetCache(int browserId) { var dict = GetCacheInternal(browserId); From 83192683679f543c5ed4905e4d5361b5a259c114 Mon Sep 17 00:00:00 2001 From: Alex Maitland Date: Tue, 18 Apr 2023 10:10:03 +1000 Subject: [PATCH 3/6] JSB - Add optional command line cache config --- .../CefAppUnmanagedWrapper.h | 12 ++++++++++-- CefSharp.BrowserSubprocess.Core/SubProcess.h | 3 ++- CefSharp/Internals/CefSharpArguments.cs | 1 + 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h b/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h index 04be3c8d0b..ecda32cae3 100644 --- a/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h +++ b/CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h @@ -42,18 +42,26 @@ namespace CefSharp 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; - _javascriptObjectCache = gcnew LegacyJavaScriptObjectCache(); _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/SubProcess.h b/CefSharp.BrowserSubprocess.Core/SubProcess.h index 8f988f561b..dfcd52838f 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/Internals/CefSharpArguments.cs b/CefSharp/Internals/CefSharpArguments.cs index 707c817aea..6684e57609 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"; } From a9c37fcddfb7f47c6950a4b35b067d9c680405ef Mon Sep 17 00:00:00 2001 From: Alex Maitland Date: Fri, 21 Apr 2023 15:27:57 +1000 Subject: [PATCH 4/6] Apply suggestions from code review Co-authored-by: campersau --- CefSharp/Internals/LegacyJavaScriptObjectCache.cs | 7 +------ CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs | 7 +------ 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/CefSharp/Internals/LegacyJavaScriptObjectCache.cs b/CefSharp/Internals/LegacyJavaScriptObjectCache.cs index 704a730391..4866759379 100644 --- a/CefSharp/Internals/LegacyJavaScriptObjectCache.cs +++ b/CefSharp/Internals/LegacyJavaScriptObjectCache.cs @@ -26,12 +26,7 @@ public void InsertOrUpdate(int browserId, IList javascriptObje { foreach (var obj in javascriptObjects) { - if (cache.ContainsKey(obj.Name)) - { - cache.Remove(obj.Name); - } - - cache.Add(obj.Name, obj); + cache[obj.Name] = obj; } } diff --git a/CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs b/CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs index 2cb8fd0617..a347eab0d3 100644 --- a/CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs +++ b/CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs @@ -29,12 +29,7 @@ public void InsertOrUpdate(int browserId, IList javascriptObje foreach (var obj in javascriptObjects) { - if (dict.ContainsKey(obj.Name)) - { - dict.Remove(obj.Name); - } - - dict.Add(obj.Name, obj); + dict[obj.Name] = obj; } } From 2f9070f5b6fffa020c4a15228c22e20a9f9ba269 Mon Sep 17 00:00:00 2001 From: amaitland <307872+amaitland@users.noreply.github.com> Date: Sun, 11 May 2025 09:46:42 +1000 Subject: [PATCH 5/6] Add more xunit tests --- .../BindObjectAsyncHandler.h | 4 +- .../RegisterBoundObjectHandler.h | 4 +- .../LegacyJavaScriptObjectCacheTests.cs | 136 ++++++++++++++++++ .../PerBrowserJavaScriptObjectCacheTests.cs | 128 +++++++++++++++++ CefSharp/Internals/IJavaScriptObjectCache.cs | 4 +- .../Internals/LegacyJavaScriptObjectCache.cs | 4 +- .../PerBrowserJavaScriptObjectCache.cs | 6 +- 7 files changed, 275 insertions(+), 11 deletions(-) create mode 100644 CefSharp.Test/JavascriptBinding/LegacyJavaScriptObjectCacheTests.cs create mode 100644 CefSharp.Test/JavascriptBinding/PerBrowserJavaScriptObjectCacheTests.cs diff --git a/CefSharp.BrowserSubprocess.Core/BindObjectAsyncHandler.h b/CefSharp.BrowserSubprocess.Core/BindObjectAsyncHandler.h index 9ece880b2b..1c72b9fede 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/RegisterBoundObjectHandler.h b/CefSharp.BrowserSubprocess.Core/RegisterBoundObjectHandler.h index 525bf19538..e3373d7c65 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.Test/JavascriptBinding/LegacyJavaScriptObjectCacheTests.cs b/CefSharp.Test/JavascriptBinding/LegacyJavaScriptObjectCacheTests.cs new file mode 100644 index 0000000000..c8014914e4 --- /dev/null +++ b/CefSharp.Test/JavascriptBinding/LegacyJavaScriptObjectCacheTests.cs @@ -0,0 +1,136 @@ +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() + { + // Arrange + var cache = new LegacyJavaScriptObjectCache(); + var javascriptObjects = new List + { + new JavascriptObject { Name = "Object1" }, + new JavascriptObject { Name = "Object2" } + }; + + // Act + cache.InsertOrUpdate(BrowserId, javascriptObjects); + + // Assert + var cachedValues = cache.GetCacheValues(BrowserId); + Assert.Contains(javascriptObjects[0], cachedValues); + Assert.Contains(javascriptObjects[1], cachedValues); + } + + [Fact] + public void GetCacheValuesShouldReturnAllCachedObjects() + { + // Arrange + var cache = new LegacyJavaScriptObjectCache(); + var javascriptObjects = new List + { + new JavascriptObject { Name = "Object1" }, + new JavascriptObject { Name = "Object2" } + }; + cache.InsertOrUpdate(BrowserId, javascriptObjects); + + // Act + var cachedValues = cache.GetCacheValues(BrowserId); + + // Assert + Assert.Equal(2, cachedValues.Count); + } + + [Fact] + public void GetCacheShouldReturnUnderlyingDictionary() + { + // Arrange + var cache = new LegacyJavaScriptObjectCache(); + var javascriptObjects = new List + { + new JavascriptObject { Name = "Object1" } + }; + cache.InsertOrUpdate(BrowserId, javascriptObjects); + + // Act + var cachedDictionary = cache.GetCache(BrowserId); + + // Assert + Assert.Single(cachedDictionary); + Assert.True(cachedDictionary.ContainsKey("Object1")); + } + + [Fact] + public void InsertOrUpdateShouldReplaceExistingObjects() + { + // Arrange + var cache = new LegacyJavaScriptObjectCache(); + var initialObjects = new List + { + new JavascriptObject { Name = "Object1" } + }; + var updatedObjects = new List + { + new JavascriptObject { Name = "Object1" } + }; + cache.InsertOrUpdate(BrowserId, initialObjects); + + // Act + cache.InsertOrUpdate(BrowserId, updatedObjects); + + // Assert + var cachedValues = cache.GetCacheValues(BrowserId); + Assert.DoesNotContain(initialObjects[0], cachedValues); + Assert.Contains(updatedObjects[0], cachedValues); + } + + [Fact] + public void InsertOrUpdateShouldAppendObjectsWithDifferentNames() + { + // Arrange + var cache = new LegacyJavaScriptObjectCache(); + var initialObjects = new List + { + new JavascriptObject { Name = "Object1" } + }; + var updatedObjects = new List + { + new JavascriptObject { Name = "Object2" } + }; + cache.InsertOrUpdate(BrowserId, initialObjects); + + // Act + cache.InsertOrUpdate(BrowserId, updatedObjects); + + // Assert + var cachedValues = cache.GetCacheValues(BrowserId); + Assert.Contains(initialObjects[0], cachedValues); + Assert.Contains(updatedObjects[0], cachedValues); + } + + [Fact] + public void ClearCacheShouldDoNothing() + { + // Arrange + var cache = new LegacyJavaScriptObjectCache(); + var javascriptObjects = new List + { + new JavascriptObject { Name = "Object1" } + }; + cache.InsertOrUpdate(BrowserId, javascriptObjects); + + // Act + cache.ClearCache(BrowserId); + + // Assert + 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 0000000000..f9952036e2 --- /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/IJavaScriptObjectCache.cs b/CefSharp/Internals/IJavaScriptObjectCache.cs index 16ba921112..649a850e2b 100644 --- a/CefSharp/Internals/IJavaScriptObjectCache.cs +++ b/CefSharp/Internals/IJavaScriptObjectCache.cs @@ -22,7 +22,7 @@ public interface IJavaScriptObjectCache /// browser Id /// Dictionary of cache 's. /// - Dictionary GetCache(int browserId); + IDictionary GetCache(int browserId); /// /// Gets a collection of s /// for the given @@ -37,6 +37,6 @@ public interface IJavaScriptObjectCache /// browser id /// JavaScript object /// - void InsertOrUpdate(int browserId, IList javascriptObjects); + void InsertOrUpdate(int browserId, IReadOnlyCollection javascriptObjects); } } diff --git a/CefSharp/Internals/LegacyJavaScriptObjectCache.cs b/CefSharp/Internals/LegacyJavaScriptObjectCache.cs index 4866759379..c45ffefc07 100644 --- a/CefSharp/Internals/LegacyJavaScriptObjectCache.cs +++ b/CefSharp/Internals/LegacyJavaScriptObjectCache.cs @@ -22,7 +22,7 @@ public void ClearCache(int browserId) } /// - public void InsertOrUpdate(int browserId, IList javascriptObjects) + public void InsertOrUpdate(int browserId, IReadOnlyCollection javascriptObjects) { foreach (var obj in javascriptObjects) { @@ -37,7 +37,7 @@ public ICollection GetCacheValues(int browserId) } /// - public Dictionary GetCache(int browserId) + public IDictionary GetCache(int browserId) { return cache; } diff --git a/CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs b/CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs index a347eab0d3..8f546fe747 100644 --- a/CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs +++ b/CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs @@ -23,7 +23,7 @@ public void ClearCache(int browserId) } /// - public void InsertOrUpdate(int browserId, IList javascriptObjects) + public void InsertOrUpdate(int browserId, IReadOnlyCollection javascriptObjects) { var dict = GetCacheInternal(browserId); @@ -45,14 +45,14 @@ public ICollection GetCacheValues(int browserId) } /// - public Dictionary GetCache(int browserId) + public IDictionary GetCache(int browserId) { var dict = GetCacheInternal(browserId); return dict; } - private Dictionary GetCacheInternal(int browserId) + private IDictionary GetCacheInternal(int browserId) { Dictionary dict; From 687ef7c95dd2d899312683620b353917dfb7129c Mon Sep 17 00:00:00 2001 From: amaitland <307872+amaitland@users.noreply.github.com> Date: Sun, 11 May 2025 09:50:51 +1000 Subject: [PATCH 6/6] Remove comments for consistency --- .../LegacyJavaScriptObjectCacheTests.cs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/CefSharp.Test/JavascriptBinding/LegacyJavaScriptObjectCacheTests.cs b/CefSharp.Test/JavascriptBinding/LegacyJavaScriptObjectCacheTests.cs index c8014914e4..8a3d6aef81 100644 --- a/CefSharp.Test/JavascriptBinding/LegacyJavaScriptObjectCacheTests.cs +++ b/CefSharp.Test/JavascriptBinding/LegacyJavaScriptObjectCacheTests.cs @@ -11,7 +11,6 @@ public class LegacyJavaScriptObjectCacheTests [Fact] public void InsertOrUpdateShouldAddObjectsToCache() { - // Arrange var cache = new LegacyJavaScriptObjectCache(); var javascriptObjects = new List { @@ -19,10 +18,8 @@ public void InsertOrUpdateShouldAddObjectsToCache() new JavascriptObject { Name = "Object2" } }; - // Act cache.InsertOrUpdate(BrowserId, javascriptObjects); - // Assert var cachedValues = cache.GetCacheValues(BrowserId); Assert.Contains(javascriptObjects[0], cachedValues); Assert.Contains(javascriptObjects[1], cachedValues); @@ -31,7 +28,6 @@ public void InsertOrUpdateShouldAddObjectsToCache() [Fact] public void GetCacheValuesShouldReturnAllCachedObjects() { - // Arrange var cache = new LegacyJavaScriptObjectCache(); var javascriptObjects = new List { @@ -40,17 +36,14 @@ public void GetCacheValuesShouldReturnAllCachedObjects() }; cache.InsertOrUpdate(BrowserId, javascriptObjects); - // Act var cachedValues = cache.GetCacheValues(BrowserId); - // Assert Assert.Equal(2, cachedValues.Count); } [Fact] public void GetCacheShouldReturnUnderlyingDictionary() { - // Arrange var cache = new LegacyJavaScriptObjectCache(); var javascriptObjects = new List { @@ -58,10 +51,8 @@ public void GetCacheShouldReturnUnderlyingDictionary() }; cache.InsertOrUpdate(BrowserId, javascriptObjects); - // Act var cachedDictionary = cache.GetCache(BrowserId); - // Assert Assert.Single(cachedDictionary); Assert.True(cachedDictionary.ContainsKey("Object1")); } @@ -69,7 +60,6 @@ public void GetCacheShouldReturnUnderlyingDictionary() [Fact] public void InsertOrUpdateShouldReplaceExistingObjects() { - // Arrange var cache = new LegacyJavaScriptObjectCache(); var initialObjects = new List { @@ -81,10 +71,8 @@ public void InsertOrUpdateShouldReplaceExistingObjects() }; cache.InsertOrUpdate(BrowserId, initialObjects); - // Act cache.InsertOrUpdate(BrowserId, updatedObjects); - // Assert var cachedValues = cache.GetCacheValues(BrowserId); Assert.DoesNotContain(initialObjects[0], cachedValues); Assert.Contains(updatedObjects[0], cachedValues); @@ -93,7 +81,6 @@ public void InsertOrUpdateShouldReplaceExistingObjects() [Fact] public void InsertOrUpdateShouldAppendObjectsWithDifferentNames() { - // Arrange var cache = new LegacyJavaScriptObjectCache(); var initialObjects = new List { @@ -105,10 +92,8 @@ public void InsertOrUpdateShouldAppendObjectsWithDifferentNames() }; cache.InsertOrUpdate(BrowserId, initialObjects); - // Act cache.InsertOrUpdate(BrowserId, updatedObjects); - // Assert var cachedValues = cache.GetCacheValues(BrowserId); Assert.Contains(initialObjects[0], cachedValues); Assert.Contains(updatedObjects[0], cachedValues); @@ -117,7 +102,6 @@ public void InsertOrUpdateShouldAppendObjectsWithDifferentNames() [Fact] public void ClearCacheShouldDoNothing() { - // Arrange var cache = new LegacyJavaScriptObjectCache(); var javascriptObjects = new List { @@ -125,10 +109,8 @@ public void ClearCacheShouldDoNothing() }; cache.InsertOrUpdate(BrowserId, javascriptObjects); - // Act cache.ClearCache(BrowserId); - // Assert var cachedValues = cache.GetCacheValues(BrowserId); Assert.Contains(javascriptObjects[0], cachedValues); }