Javascript Binding - Store JavascriptObjects per CefBrowser

[amaitland]

Fixes: #2306

Summary:

• Historically JSB objects are cached per process, for shared render processes this can be problematic
  This optionally allows for a new Per Browser cache.

Use the following command line arg to opt into this behaviour.

--jsb-cache-perbrowser

// CefSettings
settings.CefCommandLineArgs.Add("jsb-cache-perbrowser");

Changes:

• Add cache abstraction to allow for legacy and new per browser configuration
• Add new CefSharp specific command line arg to optionally enable new code path
• Settings haven't been moved to per Browser as that requires additional code changes to pass in extraInfo to the Popups

How Has This Been Tested?

• New tests added

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Updated documentation

Checklist:

• Tested the code(if applicable)
• Commented my code
• Changed the documentation(if applicable)
• New files have a license disclaimer
• The formatting is consistent with the project (project supports .editorconfig)

Summary by CodeRabbit

• New Features

  • Added configurable JavaScript object caching with per-browser and legacy (process-wide) modes and a new command-line option to enable per-browser caching.

• Bug Fixes

  • Centralized cache management for JS-bound objects and improved cleanup when browsers are destroyed to reduce cross-browser leakage.

• Tests

  • Added tests covering legacy and per-browser cache behavior, isolation, update/replace semantics, and cache clearing.

• Documentation

  • Public API signatures and interfaces updated to expose the new caching options.

[AppVeyorBot]

✅ Build CefSharp 112.2.90-CI4758 completed (commit 91bd2be36a by @amaitland)

[AppVeyorBot]

✅ Build CefSharp 112.2.100-CI4779 completed (commit 04fe8b2185 by @amaitland)

[AppVeyorBot]

✅ Build CefSharp 115.3.110-CI4829 completed (commit c19986019f by @amaitland)

[amaitland]

This is required for #5001

I originally added a command line arg, I think this should be plan b as it is hard to test. If we can have it configured per ChromiumWebBrowser instance that would make life a lot easier to test.

[AppVeyorBot]

✅ Build CefSharp 132.3.10-CI5157 completed (commit 3c5f7865ac by @amaitland)

[amaitland]

I originally added a command line arg, I think this should be plan b as it is hard to test. If we can have it configured per ChromiumWebBrowser instance that would make life a lot easier to test.

This would require a totally different approach.

[coderabbitai]

Walkthrough

Introduces an IJavaScriptObjectCache abstraction with two implementations (legacy process-wide and per-browser), updates render-process code to use the cache, adds a command-line flag to select per-browser behavior, and includes unit tests for both cache implementations.

Changes

Cohort / File(s)	Change Summary
Cache abstraction & implementations CefSharp/Internals/IJavaScriptObjectCache.cs, CefSharp/Internals/LegacyJavaScriptObjectCache.cs, CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs	Added IJavaScriptObjectCache interface and two implementations: LegacyJavaScriptObjectCache (process-wide, ClearCache no-op) and PerBrowserJavaScriptObjectCache (per-browser dictionaries, supports ClearCache).
Argument flag CefSharp/Internals/CefSharpArguments.cs	Added constant PerBrowserJavaScriptObjectCache = "--jsb-cache-perbrowser".
Render-process wrapper & wiring CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h, CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.cpp, CefSharp.BrowserSubprocess.Core/SubProcess.h	Replaced direct dictionary usage with an IJavaScriptObjectCache member, added jsbCachePerBrowser constructor flag and conditional instantiation of the appropriate cache, updated code paths to call InsertOrUpdate, GetCacheValues, GetCache, and ClearCache.
Handler signatures CefSharp.BrowserSubprocess.Core/BindObjectAsyncHandler.h, CefSharp.BrowserSubprocess.Core/RegisterBoundObjectHandler.h	Changed member and constructor parameter types from concrete Dictionary<String^, JavascriptObject^>^ to IDictionary<String^, JavascriptObject^>^.
Tests CefSharp.Test/JavascriptBinding/LegacyJavaScriptObjectCacheTests.cs, CefSharp.Test/JavascriptBinding/PerBrowserJavaScriptObjectCacheTests.cs	Added unit tests covering insertion/updating, retrieval, clearing behavior, per-browser isolation, and edge cases for both cache implementations.

Sequence Diagram(s)

sequenceDiagram
    participant SubProcess
    participant CefAppUnmanagedWrapper
    participant IJavaScriptObjectCache
    participant Browser

    SubProcess->>CefAppUnmanagedWrapper: new(jsbCachePerBrowser, ...)
    CefAppUnmanagedWrapper->>IJavaScriptObjectCache: instantiate (Legacy or PerBrowser)
    Note right of IJavaScriptObjectCache: Cache lives in render process
    Browser->>CefAppUnmanagedWrapper: OnContextCreated(browserId)
    CefAppUnmanagedWrapper->>IJavaScriptObjectCache: GetCacheValues(browserId)
    IJavaScriptObjectCache-->>CefAppUnmanagedWrapper: collection<JavascriptObject>
    CefAppUnmanagedWrapper->>Browser: bind objects (create V8 bindings)
    Browser->>IJavaScriptObjectCache: InsertOrUpdate(browserId, jsObjects)
    Browser->>IJavaScriptObjectCache: ClearCache(browserId) (on destroy)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

Suggested labels

breaking-change

Poem

"I stitched caches with hops and care,
Per-browser pockets everywhere.
Tests on my paws, flags in my clutch,
No more shared crumbs — a tidy touch.
🐇"

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 31.71% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title clearly and concisely summarizes the primary change, indicating that Javascript objects are now stored on a per-browser basis, which matches the pull request’s main objective of introducing per-browser caching.
Linked Issues Check	✅ Passed	The changes fully implement the objectives of issue #2306 by introducing an IJavaScriptObjectCache abstraction, supplying both legacy and per-browser cache implementations, integrating a command-line flag to select per-browser caching, and replacing process-wide dictionaries with browser-scoped caches in the binding handlers.
Out of Scope Changes Check	✅ Passed	All changes are focused on introducing and wiring up the per-browser caching abstraction for Javascript binding objects and related tests, with no unrelated or out-of-scope modifications present.
Description Check	✅ Passed	The pull request description adheres to the repository’s template by including the Fixes reference, a clear summary of the change, a detailed list of modifications, testing information, types of changes, and a populated checklist, providing sufficient context for reviewers.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch jsb/perbrowsercache

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[amaitland]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (5)

CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs (1)

1-68: Well-implemented per-browser JavaScript object cache

This implementation correctly manages separate JavaScript object caches for each browser instance, addressing the issue with shared render processes. The code handles edge cases appropriately, such as creating new caches when needed and returning empty collections when a cache doesn't exist.

One minor consideration: GetCache will always create a new cache dictionary if one doesn't exist for the specified browser ID. This could potentially create unnecessary empty caches if the caller just wants to check for existence. However, this aligns with the interface contract and is a reasonable implementation decision.

CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.cpp (2)

109-110: ClearCache result is silently ignored – verify intention

_javascriptObjectCache->ClearCache(browser->GetIdentifier());

For the legacy cache implementation this is a no-op by design, while for the per-browser cache it frees memory.
A reader might assume the call always does something, so please:

1. Add a short explanatory comment (// no-op for Legacy cache) to prevent future confusion.
2. Optionally gate the call behind an if (_jsbCachePerBrowser) flag to save an unnecessary virtual dispatch.

---

628-629: Potential duplication of cached objects

InsertOrUpdate is invoked both here and in OnBrowserCreated. If the browser process re-sends the same objects (e.g. after a navigation) the cache might now hold duplicates unless the implementation performs a replace-by-name.

Confirm that:

1. InsertOrUpdate overwrites existing entries that share the same key.
2. Unit tests cover this message path (currently only constructor-time insertion is tested).

CefSharp.Test/JavascriptBinding/LegacyJavaScriptObjectCacheTests.cs (1)

102-116: Test encodes legacy “no-op” behaviour – document rationale

ClearCacheShouldDoNothing intentionally expects the cache to remain populated.
Add a comment explaining that the legacy cache is process-wide and intentionally retains objects to avoid accidental refactors flipping the expectation.

CefSharp.Test/JavascriptBinding/PerBrowserJavaScriptObjectCacheTests.cs (1)

90-105: Missing negative check after overwrite

After overwriting the cache you assert only that the new value is present:

Assert.Equal(2, cachedObjects.First().Value);

To fully validate overwrite semantics, also assert that the previous value (1) no longer exists:

Assert.DoesNotContain(cachedObjects, o => o.Value == 1);

This prevents false positives where both objects remain in the list.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8fe8810 and 83b6747.

📒 Files selected for processing (11)

• CefSharp.BrowserSubprocess.Core/BindObjectAsyncHandler.h (1 hunks)
• CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.cpp (5 hunks)
• CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h (1 hunks)
• CefSharp.BrowserSubprocess.Core/RegisterBoundObjectHandler.h (1 hunks)
• CefSharp.BrowserSubprocess.Core/SubProcess.h (1 hunks)
• CefSharp.Test/JavascriptBinding/LegacyJavaScriptObjectCacheTests.cs (1 hunks)
• CefSharp.Test/JavascriptBinding/PerBrowserJavaScriptObjectCacheTests.cs (1 hunks)
• CefSharp/Internals/CefSharpArguments.cs (1 hunks)
• CefSharp/Internals/IJavaScriptObjectCache.cs (1 hunks)
• CefSharp/Internals/LegacyJavaScriptObjectCache.cs (1 hunks)
• CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs (1 hunks)

🧰 Additional context used

🧬 Code Graph Analysis (7)

CefSharp/Internals/CefSharpArguments.cs (1)

CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs (1)

• PerBrowserJavaScriptObjectCache (14-68)

CefSharp.BrowserSubprocess.Core/RegisterBoundObjectHandler.h (3)

CefSharp/Internals/IJavaScriptObjectCache.cs (1)

• IDictionary (25-25)

CefSharp/Internals/LegacyJavaScriptObjectCache.cs (1)

• IDictionary (40-43)

CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs (2)

• IDictionary (48-53)
• IDictionary (55-67)

CefSharp/Internals/IJavaScriptObjectCache.cs (2)

CefSharp/Internals/LegacyJavaScriptObjectCache.cs (2)

• ClearCache (19-22)
• InsertOrUpdate (25-31)

CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs (2)

• ClearCache (20-23)
• InsertOrUpdate (26-34)

CefSharp.BrowserSubprocess.Core/SubProcess.h (1)

CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs (1)

• PerBrowserJavaScriptObjectCache (14-68)

CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs (2)

CefSharp/Internals/IJavaScriptObjectCache.cs (4)

• ClearCache (18-18)
• InsertOrUpdate (40-40)
• ICollection (33-33)
• IDictionary (25-25)

CefSharp/Internals/LegacyJavaScriptObjectCache.cs (4)

• ClearCache (19-22)
• InsertOrUpdate (25-31)
• ICollection (34-37)
• IDictionary (40-43)

CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h (2)

CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs (1)

• PerBrowserJavaScriptObjectCache (14-68)

CefSharp/Internals/LegacyJavaScriptObjectCache.cs (1)

• LegacyJavaScriptObjectCache (13-44)

CefSharp/Internals/LegacyJavaScriptObjectCache.cs (2)

CefSharp/Internals/IJavaScriptObjectCache.cs (4)

• ClearCache (18-18)
• InsertOrUpdate (40-40)
• ICollection (33-33)
• IDictionary (25-25)

CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs (5)

• ClearCache (20-23)
• InsertOrUpdate (26-34)
• ICollection (37-45)
• IDictionary (48-53)
• IDictionary (55-67)

🔇 Additional comments (11)

CefSharp/Internals/CefSharpArguments.cs (1)

13-13: LGTM: Command-line argument for per-browser JavaScript object caching added.

The new constant follows the existing naming conventions and pattern in the class, providing a clear toggle to enable per-browser JavaScript object caching.

CefSharp.BrowserSubprocess.Core/SubProcess.h (2)

36-36: LGTM: Command-line flag parser for per-browser JavaScript object caching.

The implementation correctly uses the CommandLineArgsParser::HasArgument method to check for the new command-line argument, consistent with how other arguments are parsed in this class.

---

39-39: LGTM: Updated constructor call with new parameter.

The CefAppUnmanagedWrapper constructor is now called with the new jsbCachePerBrowser parameter, allowing the appropriate cache implementation to be selected at runtime.

CefSharp.BrowserSubprocess.Core/RegisterBoundObjectHandler.h (1)

30-30: LGTM: Updated to use interface instead of concrete implementation.

Good change to use the more abstract IDictionary interface instead of the concrete Dictionary class. This modification supports the new cache abstraction by allowing different cache implementations to be injected, promoting loose coupling and better maintainability.

Also applies to: 33-33

CefSharp.BrowserSubprocess.Core/BindObjectAsyncHandler.h (1)

28-28: LGTM: Updated to use interface instead of concrete implementation.

Good change to use the more abstract IDictionary interface instead of the concrete Dictionary class. This enables the class to work with different cache implementations (either legacy process-wide cache or per-browser cache) through dependency injection.

Also applies to: 32-32

CefSharp/Internals/IJavaScriptObjectCache.cs (1)

1-42: Well-designed interface for JavaScript object caching

This interface provides a clean and well-documented contract for managing JavaScript binding objects in the render process, with support for both per-process (legacy) and per-browser caching strategies. The method signatures are clear, focused, and appropriate for the operations being performed.

CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h (2)

38-38: LGTM: Good abstraction for JavaScript object caching

The replacement of the concrete dictionary with an interface type provides better abstraction and flexibility.

---

45-64: LGTM: Proper initialization based on caching strategy

The constructor now correctly initializes the appropriate cache implementation based on the jsbCachePerBrowser flag. This allows for easy switching between caching strategies without modifying the rest of the code.

CefSharp/Internals/LegacyJavaScriptObjectCache.cs (1)

1-44: LGTM: Legacy cache implementation maintains backward compatibility

This implementation correctly maintains the legacy behavior of caching JavaScript objects per process rather than per browser. The ClearCache method is appropriately implemented as a no-op, and the other methods ignore the browser ID parameter as expected.

CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.cpp (1)

133-139: Gracefully handle an empty or null value collection

GetCacheValues is assumed to return a non-null collection. If the implementation ever changes to return nullptr when the browserId is not present, the subsequent ->Count dereference will crash.

-auto values = _javascriptObjectCache->GetCacheValues(browser->GetIdentifier());
-if (values->Count > 0 && rootObject != nullptr)
+auto values = _javascriptObjectCache->GetCacheValues(browser->GetIdentifier());
+if (values != nullptr && values->Count > 0 && rootObject != nullptr)

Adding the extra guard costs nothing and improves robustness.

CefSharp.Test/JavascriptBinding/LegacyJavaScriptObjectCacheTests.cs (1)

15-26: Equality semantics might mask duplicate objects

Assert.Contains(javascriptObjects[0], cachedValues);

Contains relies on reference equality unless JavascriptObject overrides Equals/GetHashCode.
If equality is not overridden, a scenario where two distinct JavascriptObject instances share the same Name but both remain in the cache would not be detected by these tests.

Please either:

1. Override equality on JavascriptObject to be value-based (preferred), or
2. Assert against the cache dictionary keys instead of values.

This will make the test guard the contract more strictly.

[AppVeyorBot]

✅ Build CefSharp 136.1.40-CI5239 completed (commit 344ec7fb5f by @amaitland)

[AppVeyorBot]

✅ Build CefSharp 136.1.40-CI5258 completed (commit df9b49b25a by @amaitland)

[AppVeyorBot]

✅ Build CefSharp 137.0.100-CI5283 completed (commit 55b8f61430 by @amaitland)

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs (1)

44-45: Return a shared empty collection instead of allocating.

When no cache exists we create a new List<JavascriptObject> each call. Returning Array.Empty<JavascriptObject>() avoids repeated allocations while still satisfying the contract.

Apply this diff:

-            return new List<JavascriptObject>();
+            return Array.Empty<JavascriptObject>();

CefSharp.Test/JavascriptBinding/PerBrowserJavaScriptObjectCacheTests.cs (1)

26-40: Test name could be more precise.

The test name suggests it verifies both add and update behavior, but it only validates insertion of new objects. While update behavior is properly tested elsewhere (lines 87-105), consider renaming to InsertOrUpdateAddsObjectsToCache for clarity.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 687ef7c and e064cee.

📒 Files selected for processing (11)

• CefSharp.BrowserSubprocess.Core/BindObjectAsyncHandler.h (1 hunks)
• CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.cpp (5 hunks)
• CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h (1 hunks)
• CefSharp.BrowserSubprocess.Core/RegisterBoundObjectHandler.h (1 hunks)
• CefSharp.BrowserSubprocess.Core/SubProcess.h (1 hunks)
• CefSharp.Test/JavascriptBinding/LegacyJavaScriptObjectCacheTests.cs (1 hunks)
• CefSharp.Test/JavascriptBinding/PerBrowserJavaScriptObjectCacheTests.cs (1 hunks)
• CefSharp/Internals/CefSharpArguments.cs (1 hunks)
• CefSharp/Internals/IJavaScriptObjectCache.cs (1 hunks)
• CefSharp/Internals/LegacyJavaScriptObjectCache.cs (1 hunks)
• CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (5)

• CefSharp/Internals/LegacyJavaScriptObjectCache.cs
• CefSharp.BrowserSubprocess.Core/BindObjectAsyncHandler.h
• CefSharp/Internals/CefSharpArguments.cs
• CefSharp.BrowserSubprocess.Core/SubProcess.h
• CefSharp.BrowserSubprocess.Core/RegisterBoundObjectHandler.h

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: amaitland
PR: cefsharp/CefSharp#4475
File: CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.cpp:147-154
Timestamp: 2025-05-10T23:58:24.668Z
Learning: The `GetCache` method of the `IJavaScriptObjectCache` implementation should never return null in its current implementation. Both the legacy and per-browser cache implementations guarantee a non-null return value, with the per-browser implementation returning an empty dictionary when no cache exists for a given browser ID.

📚 Learning: 2025-05-10T23:58:24.668Z

Learnt from: amaitland
PR: cefsharp/CefSharp#4475
File: CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.cpp:147-154
Timestamp: 2025-05-10T23:58:24.668Z
Learning: The `GetCache` method of the `IJavaScriptObjectCache` implementation should never return null in its current implementation. Both the legacy and per-browser cache implementations guarantee a non-null return value, with the per-browser implementation returning an empty dictionary when no cache exists for a given browser ID.

Applied to files:

• CefSharp/Internals/IJavaScriptObjectCache.cs
• CefSharp.Test/JavascriptBinding/LegacyJavaScriptObjectCacheTests.cs
• CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs
• CefSharp.Test/JavascriptBinding/PerBrowserJavaScriptObjectCacheTests.cs
• CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.cpp

🧬 Code graph analysis (5)

CefSharp/Internals/IJavaScriptObjectCache.cs (3)

CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h (1)

• CefSharp (17-98)

CefSharp/Internals/LegacyJavaScriptObjectCache.cs (2)

• ClearCache (19-22)
• InsertOrUpdate (25-31)

CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs (2)

• ClearCache (20-23)
• InsertOrUpdate (26-34)

CefSharp.Test/JavascriptBinding/LegacyJavaScriptObjectCacheTests.cs (4)

CefSharp.Test/JavascriptBinding/PerBrowserJavaScriptObjectCacheTests.cs (8)

• Fact (12-24)
• Fact (26-40)
• Fact (42-50)
• Fact (52-66)
• Fact (68-85)
• Fact (87-105)
• Fact (107-115)
• Fact (117-126)

CefSharp/Internals/LegacyJavaScriptObjectCache.cs (3)

• LegacyJavaScriptObjectCache (13-44)
• InsertOrUpdate (25-31)
• ClearCache (19-22)

CefSharp/Internals/IJavaScriptObjectCache.cs (2)

• InsertOrUpdate (40-40)
• ClearCache (18-18)

CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs (2)

• InsertOrUpdate (26-34)
• ClearCache (20-23)

CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h (3)

CefSharp.BrowserSubprocess.Core/CefBrowserWrapper.h (1)

• CefBrowserWrapper (27-75)

CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs (1)

• PerBrowserJavaScriptObjectCache (14-68)

CefSharp/Internals/LegacyJavaScriptObjectCache.cs (1)

• LegacyJavaScriptObjectCache (13-44)

CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs (3)

CefSharp.BrowserSubprocess.Core/CefAppUnmanagedWrapper.h (1)

• CefSharp (17-98)

CefSharp/Internals/IJavaScriptObjectCache.cs (4)

• ClearCache (18-18)
• InsertOrUpdate (40-40)
• ICollection (33-33)
• IDictionary (25-25)

CefSharp/Internals/LegacyJavaScriptObjectCache.cs (4)

• ClearCache (19-22)
• InsertOrUpdate (25-31)
• ICollection (34-37)
• IDictionary (40-43)

CefSharp.Test/JavascriptBinding/PerBrowserJavaScriptObjectCacheTests.cs (3)

CefSharp/Internals/PerBrowserJavaScriptObjectCache.cs (3)

• PerBrowserJavaScriptObjectCache (14-68)
• InsertOrUpdate (26-34)
• ClearCache (20-23)

CefSharp/Internals/IJavaScriptObjectCache.cs (2)

• InsertOrUpdate (40-40)
• ClearCache (18-18)

CefSharp/Internals/LegacyJavaScriptObjectCache.cs (2)

• InsertOrUpdate (25-31)
• ClearCache (19-22)

🔇 Additional comments (7)

CefSharp.Test/JavascriptBinding/PerBrowserJavaScriptObjectCacheTests.cs (7)

12-24: LGTM!

The test correctly verifies that ClearCache removes the cache for a specific browser ID.

---

42-50: LGTM!

Good edge case coverage for non-existent cache scenarios.

---

52-66: LGTM!

The test properly verifies GetCache returns the correct dictionary with expected content.

---

68-85: Excellent test for cache isolation!

This test properly validates that the per-browser cache maintains isolation between different browser instances, which is a critical requirement for this feature.

---

87-105: LGTM!

The test correctly verifies that InsertOrUpdate overwrites existing cache entries. The use of .First().Value at line 104 is safe due to the Assert.Single at line 103 ensuring exactly one element exists.

---

107-115: LGTM!

Good edge case coverage. The test correctly verifies that GetCache returns an empty dictionary (not null) for non-existent browser IDs, which aligns with the implementation guarantee.

Based on learnings.

---

117-126: LGTM!

Good defensive test ensuring empty object lists are handled gracefully.

[AppVeyorBot]

❌ Build CefSharp 140.1.140-CI5360 failed (commit 31c7b3a6c8 by @amaitland)