Actionable comments posted: 1

🧹 Nitpick comments (1)

CefSharp.Test/Javascript/EvaluateScriptAsyncTests.cs (1)

68-83: Consider reducing the loop sizes to make the test more practical.

While the test correctly validates OOM cancellation behavior, the nested loops with 10M x 10M iterations creating 100M character strings could cause test instability or excessive runtime. Consider smaller values that still reliably trigger OOM.

-                let array1 = [];
-                for (let i = 0; i < 10000000; i++) {
-                    let array2 = [];
-                    for (let j = 0; j < 10000000; j++) {
-                        array2.push('a'.repeat(100000000));
-                    }
-                    array1.push(array2);
-                }
+                let array1 = [];
+                for (let i = 0; i < 1000; i++) {
+                    let array2 = [];
+                    for (let j = 0; j < 1000; j++) {
+                        array2.push('a'.repeat(10000000));
+                    }
+                    array1.push(array2);
+                }

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between cde57b6 and 7adac4c.

📒 Files selected for processing (7)

• CefSharp.Core.Runtime/Internals/CefFrameWrapper.cpp (1 hunks)
• CefSharp.Core.Runtime/Internals/ClientAdapter.cpp (3 hunks)
• CefSharp.Core.Runtime/Internals/ClientAdapter.h (2 hunks)
• CefSharp.Core.Runtime/Internals/JavascriptCallbackProxy.cpp (1 hunks)
• CefSharp.Test/Javascript/EvaluateScriptAsyncTests.cs (3 hunks)
• CefSharp/Internals/PendingTaskRepository.cs (2 hunks)
• CefSharp/Internals/TaskExtensions.cs (1 hunks)

🧠 Learnings (1) CefSharp.Core.Runtime/Internals/CefFrameWrapper.cpp (1)

Learnt from: amaitland
PR: #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.

🧬 Code Graph Analysis (3) CefSharp.Core.Runtime/Internals/JavascriptCallbackProxy.cpp (3)

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

• CefBrowserWrapper (27-75)

CefSharp.Core.Runtime/Internals/StringUtils.h (1)

• ToNative (74-84)

CefSharp.Core.Runtime/Internals/Serialization/Primitives.cpp (2)

• SetInt64 (41-49)
• SetInt64 (41-41)

CefSharp.Core.Runtime/Internals/ClientAdapter.cpp (2)

CefSharp.Core.Runtime/Internals/CefRequestContextHandlerAdapter.cpp (1)

• ToClr (52-52)

CefSharp.Core.Runtime/Internals/Serialization/Primitives.cpp (2)

• GetInt64 (52-60)
• GetInt64 (52-52)

CefSharp.Core.Runtime/Internals/CefFrameWrapper.cpp (3)

CefSharp.BrowserSubprocess.Core/Wrapper/Browser.h (1)

• Identifier (109-112)

CefSharp.Core.Runtime/Internals/CefBrowserWrapper.h (1)

• Identifier (117-120)

CefSharp.Core.Runtime/Request.h (1)

• Identifier (69-69)

🧰 Additional context used 🧠 Learnings (1) CefSharp.Core.Runtime/Internals/CefFrameWrapper.cpp (1)

Learnt from: amaitland
PR: #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.

🧬 Code Graph Analysis (3) CefSharp.Core.Runtime/Internals/JavascriptCallbackProxy.cpp (3)

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

• CefBrowserWrapper (27-75)

CefSharp.Core.Runtime/Internals/StringUtils.h (1)

• ToNative (74-84)

CefSharp.Core.Runtime/Internals/Serialization/Primitives.cpp (2)

• SetInt64 (41-49)
• SetInt64 (41-41)

CefSharp.Core.Runtime/Internals/ClientAdapter.cpp (2)

CefSharp.Core.Runtime/Internals/CefRequestContextHandlerAdapter.cpp (1)

• ToClr (52-52)

CefSharp.Core.Runtime/Internals/Serialization/Primitives.cpp (2)

• GetInt64 (52-60)
• GetInt64 (52-52)

CefSharp.Core.Runtime/Internals/CefFrameWrapper.cpp (3)

CefSharp.BrowserSubprocess.Core/Wrapper/Browser.h (1)

• Identifier (109-112)

CefSharp.Core.Runtime/Internals/CefBrowserWrapper.h (1)

• Identifier (117-120)

CefSharp.Core.Runtime/Request.h (1)

• Identifier (69-69)

🔇 Additional comments (20)

CefSharp.Core.Runtime/Internals/ClientAdapter.h (2)

73-73: Good change to cancel tasks before cleanup.

The shift from direct deletion to explicit task cancellation aligns with the PR objective of properly canceling pending JavaScript tasks when the V8 context is released or the browser crashes.

---

82-82: Appropriate nullification after task cancellation.

Setting the pointer to nullptr after canceling tasks is consistent with the new lifecycle management approach.

CefSharp/Internals/TaskExtensions.cs (1)

23-23: Good simplification of cancellation logic.

Relying directly on TrySetCanceled() return value is cleaner and more robust than the previous status check, as the method already handles the completed task scenario internally.

CefSharp.Core.Runtime/Internals/CefFrameWrapper.cpp (2)

240-240: Improved error handling with faulted Task.

Returning a faulted Task instead of nullptr provides better error propagation and consistent API behavior when the browser host is unavailable.

---

248-248: Frame-scoped task management implementation.

Adding the Identifier parameter enables frame-scoped task management, which is essential for the PR objective of canceling pending tasks when V8 contexts are released.

CefSharp.Core.Runtime/Internals/JavascriptCallbackProxy.cpp (2)

30-30: Consistent error handling pattern.

Returning a faulted Task instead of throwing an exception maintains consistency with the error handling pattern established in CefFrameWrapper::EvaluateScriptAsync.

---

39-54: Frame-scoped task management and improved logic flow.

Moving the callback task creation and message construction inside the frame validity check ensures tasks are only created for valid frames, and the addition of _callback->FrameId parameter enables proper frame-scoped task tracking.

CefSharp.Core.Runtime/Internals/ClientAdapter.cpp (4)

704-704: Essential cleanup on render process termination.

Canceling all pending tasks when the render process terminates ensures proper cleanup and prevents dangling tasks, which is a core objective of this PR.

---

387-389: Frame-scoped task cancellation on context release.

Extracting the frame ID and canceling frame-specific pending tasks when the V8 context is released is exactly what the PR aims to achieve - proper cleanup when contexts become invalid.

---

393-393: Consistent use of extracted frame ID.

Using the pre-extracted frameId variable maintains consistency and avoids redundant calls to StringUtils::ToClr(frame->GetIdentifier()).

---

484-484: Frame-aware task removal operations.

Adding the frameId parameter to both RemovePendingTask and RemoveJavascriptCallbackPendingTask calls enables proper frame-scoped task management, ensuring tasks are tracked and removed at the correct granularity.

Also applies to: 492-493

CefSharp.Test/Javascript/EvaluateScriptAsyncTests.cs (3)

11-11: LGTM!

The using statement is appropriately added to support the new test methods.

---

30-66: Well-structured test for V8 context lifecycle!

The test comprehensively validates the frame-scoped task cancellation behavior across different V8 context states, which aligns perfectly with the PR objectives.

---

323-323: Good formatting consistency fix.

CefSharp/Internals/PendingTaskRepository.cs (6)

22-23: Good refactoring to support frame-scoped task management.

The new structure properly encapsulates tasks by frame ID, which aligns with the PR objective of canceling tasks when V8 context is released.

---

33-60: Correctly implements frame-scoped task creation.

The method properly handles both creating new frame repositories and adding tasks to existing ones, with appropriate framework-specific implementations.

---

69-94: Consistent implementation for callback task creation.

The method correctly maintains the separation between regular and callback tasks while implementing frame-scoped management.

---

107-120: Proper implementation of frame-scoped task removal.

---

147-158: Key implementation for frame-specific task cancellation.

This method enables the core PR objective of canceling all pending tasks when a V8 context is released or frame is destroyed.

---

160-170: Comprehensive global task cancellation.

Properly handles cancellation of all pending tasks across all frames, which is essential for browser disposal scenarios.

Actionable comments posted: 0

🧹 Nitpick comments (2)

CefSharp.Test/Javascript/JavascriptCallbackTests.cs (2)

27-71: Comprehensive test coverage for V8 context lifecycle management.

This test effectively validates the key scenarios outlined in the PR objectives:

• Script evaluation failure before V8 context creation
• Successful callback execution after context availability
• Task cancellation on context changes and browser disposal

The test logic is sound and the resource management with using statements is appropriate.

Consider these improvements for test robustness:

1. Timing dependency: The 1000ms timeout in promises could cause flakiness in CI environments. Consider using a shorter timeout or mocking time-sensitive operations.

2. Exception validation: Line 36 only checks the message prefix. Consider validating the specific exception type for better test precision:

-var withoutV8ContextException = await Assert.ThrowsAsync<Exception>(() => browser.EvaluateScriptAsync("(function() { return 1+1; })"));
+var withoutV8ContextException = await Assert.ThrowsAsync<InvalidOperationException>(() => browser.EvaluateScriptAsync("(function() { return 1+1; })"));

---

73-87: Excellent focused test for crash scenario validation.

This test cleanly validates the critical crash scenario outlined in the PR objectives. The use of chrome://crash is the standard approach for simulating browser crashes in Chromium-based testing, and the assertion logic correctly expects TaskCanceledException.

The test structure is well-organized and focused on a single concern.

Minor consideration for test reliability:

Similar to the previous test, the 1000ms timeout could potentially cause timing issues in slower CI environments, though it's likely sufficient for crash testing scenarios.

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 1389aff and f1567eb.

📒 Files selected for processing (2)

• CefSharp.Test/Javascript/EvaluateScriptAsyncTests.cs (3 hunks)
• CefSharp.Test/Javascript/JavascriptCallbackTests.cs (2 hunks)

✅ Files skipped from review due to trivial changes (1)

• CefSharp.Test/Javascript/EvaluateScriptAsyncTests.cs

🔇 Additional comments (1)

CefSharp.Test/Javascript/JavascriptCallbackTests.cs (1)

9-9: LGTM: Clean namespace addition for test support.

The using statement is appropriately added to support the CefExample.HelloWorldUrl usage in the new test methods.

Actionable comments posted: 0

🧹 Nitpick comments (1)

CefSharp.Test/Javascript/JavascriptCallbackTests.cs (1)

27-77: Comprehensive test coverage for V8 context lifecycle scenarios.

This test method effectively validates the core functionality described in the PR objectives - canceling pending tasks when V8 context is released or browser is disposed. The test structure is well-organized and covers multiple scenarios:

1. ✅ Script evaluation failure before V8 context creation
2. ✅ Successful callback execution after context creation
3. ✅ Task cancellation on V8 context change (reload)
4. ✅ Task cancellation on browser disposal

The async/await patterns and exception assertions are correctly implemented.

However, consider these minor improvements:

-                var callbackExecuteWithoutV8ContextException = await Assert.ThrowsAsync<Exception>(() => browser.EvaluateScriptAsync("(function() { return 1+1; })"));
+                var withoutContextException = await Assert.ThrowsAsync<Exception>(() => browser.EvaluateScriptAsync("(function() { return 1+1; })"));
-                Assert.StartsWith("Unable to execute javascript at this time", withoutV8ContextException.Message);
+                Assert.StartsWith("Unable to execute javascript at this time", withoutContextException.Message);

The variable names could be more concise while maintaining 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 f1567eb and 432cc2e.

📒 Files selected for processing (3)

• CefSharp.Core.Runtime/Internals/JavascriptCallbackProxy.cpp (1 hunks)
• CefSharp.Test/Javascript/EvaluateScriptAsyncTests.cs (3 hunks)
• CefSharp.Test/Javascript/JavascriptCallbackTests.cs (3 hunks)

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

• CefSharp.Core.Runtime/Internals/JavascriptCallbackProxy.cpp
• CefSharp.Test/Javascript/EvaluateScriptAsyncTests.cs

🔇 Additional comments (3)

CefSharp.Test/Javascript/JavascriptCallbackTests.cs (3)

9-9: LGTM: Import addition is appropriate.

The CefSharp.Example import is correctly added and used in the test methods to access CefExample.HelloWorldUrl.

---

79-93: Excellent test for browser crash scenario.

This test method directly validates one of the core PR objectives - ensuring pending tasks are canceled when the browser crashes. The implementation is clean and effective:

• Uses chrome://crash to simulate browser crash condition
• Properly tests that delayed promise callbacks are canceled
• Correctly expects TaskCanceledException

The test aligns perfectly with the PR's goal of handling task cancellation during browser crashes.

---

306-322: Good test coverage for repeated callback execution.

This test method ensures that the refactored frame-scoped task management system maintains compatibility with existing callback behavior. The test validates that:

• Callbacks can be executed multiple times successfully
• Results remain consistent across executions
• No issues arise from the new task repository structure

The simple but effective test design provides confidence in the backwards compatibility of the changes.

Overall looks excellent! Thanks

Couple of minor things to discuss inline.

Actionable comments posted: 2

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 774abb1 and 7415ff5.

📒 Files selected for processing (4)

• CefSharp.Test/Javascript/EvaluateScriptAsyncTests.cs (3 hunks)
• CefSharp.Test/Javascript/JavascriptCallbackTests.cs (3 hunks)
• CefSharp/Internals/FramePendingTaskRepository.cs (1 hunks)
• CefSharp/Internals/PendingTaskRepository.cs (2 hunks)

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

• CefSharp.Test/Javascript/JavascriptCallbackTests.cs
• CefSharp.Test/Javascript/EvaluateScriptAsyncTests.cs

🔇 Additional comments (7)

CefSharp/Internals/FramePendingTaskRepository.cs (2)

11-16: Well-designed frame-scoped task repository.

The class design follows good practices with appropriate access modifiers, thread-safe collections, and immutable property references. The generic type parameter enables reuse across different result types.

---

18-31: Correct disposal pattern with safe task cancellation.

The Dispose method properly uses TrySetCanceled() to avoid exceptions if tasks are already completed, and clears the dictionaries after cancellation. This implementation aligns with best practices for disposing TaskCompletionSource collections.

CefSharp/Internals/PendingTaskRepository.cs (5)

22-23: Effective refactoring to frame-scoped task management.

The change from separate task dictionaries to a single dictionary of frame repositories enables targeted task cancellation per frame while maintaining thread safety and proper encapsulation.

---

107-120: Correct implementation of frame-scoped task removal.

The method properly handles the two-level lookup (frame then task) with appropriate null checking and maintains thread safety throughout the operation.

---

132-145: Consistent implementation with proper frame-scoped lookup.

The method correctly mirrors the RemovePendingTask implementation pattern, operating on the CallbackPendingTasks dictionary with proper error handling.

---

147-158: Excellent implementation of frame-scoped task cancellation.

This new method enables the core functionality of canceling tasks per frame. The use of TryRemove ensures atomic removal and the subsequent Dispose() call properly cancels all frame-scoped tasks.

---

163-170: Proper implementation of global task cancellation.

The method correctly disposes all frame repositories and clears the main dictionary, providing a clean way to cancel all pending tasks across all frames.

Actionable comments posted: 1

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 7415ff5 and 7265b22.

📒 Files selected for processing (1)

• CefSharp/Internals/PendingTaskRepository.cs (2 hunks)

🔇 Additional comments (7)

CefSharp/Internals/PendingTaskRepository.cs (7)

22-23: LGTM! Clean refactoring to frame-scoped architecture.

The change from separate dictionaries to a single frame-keyed dictionary aligns well with the PR objective of managing tasks per frame/V8Context for proper cancellation.

---

65-95: Consistent implementation with CreatePendingTask.

The JavaScript callback variant maintains the same pattern and structure as the regular pending task creation, which is good for consistency.

---

107-120: Proper frame-scoped task removal with safe fallback.

The implementation correctly handles the case where the frame doesn't exist and provides safe fallback by returning null. The nested TryGetValue/TryRemove pattern is appropriate for concurrent access.

---

132-145: Consistent with RemovePendingTask implementation.

The callback task removal follows the same safe pattern as the regular task removal, maintaining consistency across the API.

---

147-158: Frame-specific cancellation implements core PR requirement.

This method directly addresses the PR objective of canceling pending tasks when V8Context is released. The TryRemove followed by Dispose ensures proper cleanup.

---

160-170: FramePendingTaskRepository.Dispose safely uses TrySetCanceled
I’ve verified in CefSharp/Internals/FramePendingTaskRepository.cs that the Dispose() implementation iterates over both PendingTasks.Values and CallbackPendingTasks.Values, calling tcs.TrySetCanceled() each time before clearing the dictionaries. There’s no use of SetCanceled(), so cancellations won’t throw. All good to go.

---

40-52: Conditional compilation and generic type usage verified

All generic class definitions and instantiations are consistent:

• FramePendingTaskRepository defined at CefSharp/Internals/FramePendingTaskRepository.cs:11
• Instantiated with <TResult> in CefSharp/Internals/PendingTaskRepository.cs at lines 42, 49, 77, and 84

No non-generic usages detected. Approving these changes.

Actionable comments posted: 0

♻️ Duplicate comments (1)

CefSharp.Core.Runtime/Internals/CefFrameWrapper.cpp (1)

237-242: Return a faulted Task instead of null — good improvement; update the stale comment

Returning Task::FromException with a descriptive InvalidOperationException is preferable to nullptr and aligns with earlier review feedback about avoiding NREs.

Please update the comment to reflect the new behavior:

-    //If we're unable to get the underlying browser/browserhost then return null
+    // If we're unable to get the underlying browser/browserhost then return a faulted Task

🧹 Nitpick comments (5)

CefSharp/Internals/TaskExtensions.cs (1)

20-31: Race-safe cancellation via TrySetCanceled — LGTM; consider disposing the timer on early completion

Switching to TrySetCanceled() and invoking the cancelled callback only when cancellation succeeds removes redundant checks and avoids invoking the callback after completion. Looks good.

Optionally, dispose the timer if the task completes before the timeout to free resources sooner (current code disposes on timeout only):

 var timer = new Timer(state =>
 {
     ((Timer)state).Dispose();
     if (taskCompletionSource.TrySetCanceled())
     {
         if (cancelled != null)
         {
             cancelled();
         }
     }
 });
 timer.Change(timeout, Timeout.InfiniteTimeSpan);
+
+// Ensure timer is released if the task completes before the timeout
+taskCompletionSource.Task.ContinueWith(_ => timer.Dispose(), TaskScheduler.Default);
 
 return taskCompletionSource;

CefSharp.Core.Runtime/Internals/CefFrameWrapper.cpp (1)

247-265: Typo: rename idAndComplectionSource to idAndCompletionSource (local rename only)

Purely a readability/typo fix within the method:

-    auto idAndComplectionSource = pendingTaskRepository->CreatePendingTask(Identifier, timeout);
+    auto idAndCompletionSource = pendingTaskRepository->CreatePendingTask(Identifier, timeout);

-    if (useImmediatelyInvokedFuncExpression)
+    if (useImmediatelyInvokedFuncExpression)
     {
-        script = "(function() { let cefSharpInternalCallbackId = " + idAndComplectionSource.Key + "; " + script + " })();";
+        script = "(function() { let cefSharpInternalCallbackId = " + idAndCompletionSource.Key + "; " + script + " })();";
     }

     auto message = CefProcessMessage::Create(kEvaluateJavascriptRequest);
     auto argList = message->GetArgumentList();
-    SetInt64(argList, 0, idAndComplectionSource.Key);
+    SetInt64(argList, 0, idAndCompletionSource.Key);
     argList->SetString(1, StringUtils::ToNative(script));
     argList->SetString(2, StringUtils::ToNative(scriptUrl));
     argList->SetInt(3, startLine);

     _frame->SendProcessMessage(CefProcessId::PID_RENDERER, message);

-    return idAndComplectionSource.Value->Task;
+    return idAndCompletionSource.Value->Task;

CefSharp.Core.Runtime/Internals/ClientAdapter.cpp (2)

702-706: Cancel pending tasks on render-process termination — good; add a defensive null-check

The eager cancellation here is appropriate and should reduce dangling awaiters when the renderer dies. For robustness (during late shutdown/disposal windows), guard against a null repository:

-            _pendingTaskRepository->CancelPendingTasks();
+            if (_pendingTaskRepository != nullptr)
+            {
+                _pendingTaskRepository->CancelPendingTasks();
+            }

---

1484-1494: Guard against a potentially invalid frame before accessing GetIdentifier()

While CEF typically provides a valid frame for these messages, races during teardown can leave frame invalid. Adding a sanity check avoids dereferencing an invalid frame in those rare cases; cancelled tasks would already have been handled by earlier context-release/crash logic.

-                auto frameId = StringUtils::ToClr(frame->GetIdentifier());
+                if (!(frame.get() && frame->IsValid()))
+                {
+                    handled = true; // likely already cancelled
+                    return handled;
+                }
+                auto frameId = StringUtils::ToClr(frame->GetIdentifier());

CefSharp.Test/Javascript/EvaluateScriptAsyncTests.cs (1)

30-64: New test validates cancellation on V8 context change — LGTM; reduce flakiness with a unique reload URL

This is a solid coverage addition. To minimize race conditions where the promise resolves before the reload causes a context change, consider navigating to a unique URL (query param) to guarantee a fresh V8 context:

-                // V8 context
-                await browser.LoadUrlAsync(CefExample.HelloWorldUrl);
+                // V8 context
+                await browser.LoadUrlAsync(CefExample.HelloWorldUrl);

                 var evaluateWithoutV8ContextResponse = await evaluateWithoutV8ContextTask;
                 Assert.True(evaluateWithoutV8ContextResponse.Success);
                 Assert.Equal(3, evaluateWithoutV8ContextResponse.Result);

                 var evaluateCancelAfterV8ContextChangeTask = browser.EvaluateScriptAsync("new Promise(resolve => setTimeout(resolve, 1000))");

-                // change V8 context
-                await browser.LoadUrlAsync(CefExample.HelloWorldUrl);
+                // change V8 context (unique URL to force reload)
+                var reloadUrl = CefExample.HelloWorldUrl + "?r=" + Guid.NewGuid().ToString("N");
+                await browser.LoadUrlAsync(reloadUrl);

📜 Review details

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

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 7265b22 and 1bf7e15.

📒 Files selected for processing (9)

• CefSharp.Core.Runtime/Internals/CefFrameWrapper.cpp (1 hunks)
• CefSharp.Core.Runtime/Internals/ClientAdapter.cpp (3 hunks)
• CefSharp.Core.Runtime/Internals/ClientAdapter.h (2 hunks)
• CefSharp.Core.Runtime/Internals/JavascriptCallbackProxy.cpp (1 hunks)
• CefSharp.Test/Javascript/EvaluateScriptAsyncTests.cs (3 hunks)
• CefSharp.Test/Javascript/JavascriptCallbackTests.cs (3 hunks)
• CefSharp/Internals/FramePendingTaskRepository.cs (1 hunks)
• CefSharp/Internals/PendingTaskRepository.cs (2 hunks)
• CefSharp/Internals/TaskExtensions.cs (1 hunks)

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

• CefSharp/Internals/FramePendingTaskRepository.cs
• CefSharp.Core.Runtime/Internals/ClientAdapter.h
• CefSharp.Test/Javascript/JavascriptCallbackTests.cs
• CefSharp.Core.Runtime/Internals/JavascriptCallbackProxy.cpp
• CefSharp/Internals/PendingTaskRepository.cs

🧰 Additional context used 🧬 Code Graph Analysis (3) CefSharp.Core.Runtime/Internals/ClientAdapter.cpp (2)

CefSharp.Core.Runtime/Internals/CefRequestContextHandlerAdapter.cpp (1)

• ToClr (52-52)

CefSharp.Core.Runtime/Internals/Serialization/Primitives.cpp (2)

• GetInt64 (52-60)
• GetInt64 (52-52)

CefSharp.Test/Javascript/EvaluateScriptAsyncTests.cs (3)

CefSharp.Test/Javascript/JavascriptCallbackTests.cs (5)

• Fact (27-77)
• Fact (79-93)
• Fact (259-279)
• Fact (281-304)
• Fact (306-322)

CefSharp.Example/CefExample.cs (1)

• CefExample (15-274)

CefSharp.Test/BrowserTests.cs (1)

• AssertInitialLoadComplete (17-23)

CefSharp.Core.Runtime/Internals/CefFrameWrapper.cpp (3)

CefSharp.BrowserSubprocess.Core/Wrapper/Browser.h (1)

• Identifier (109-112)

CefSharp.Core.Runtime/Internals/CefBrowserWrapper.h (1)

• Identifier (117-120)

CefSharp.Core.Runtime/Request.h (1)

• Identifier (69-69)

🔇 Additional comments (4)

CefSharp.Core.Runtime/Internals/ClientAdapter.cpp (1)

1387-1394: Per-frame cancellation on V8 context release — LGTM

Extracting frameId once and cancelling only that frame’s pending tasks matches the PR objective and keeps unrelated frames unaffected.

CefSharp.Test/Javascript/EvaluateScriptAsyncTests.cs (3)

11-11: Import for CefExample URLs — LGTM

The using CefSharp.Example; import is required for HelloWorldUrl. Good.

---

66-75: Crash-triggered cancellation coverage — LGTM

Accurately asserts cancellation behavior on chrome://crash (aligned with new CancelPendingTasks on termination).

---

314-315: Minor formatting fix — LGTM

Style consistency improvement is fine.

Actionable comments posted: 0

🧹 Nitpick comments (2)

CefSharp.Test/Selector/WaitForSelectorAsyncTests.cs (2)

160-160: Avoid asserting on the exact exception message

Exception messages are locale/runtime-dependent and brittle in CI. Asserting the exception type already validates the behavior; drop the message assertion.

Apply this diff:

-            const string expected = "A task was canceled.";
@@
-                Assert.Contains(expected, exception.Message);
+                // Exception type validation is sufficient; message can vary by runtime/locale.

Also applies to: 177-177

---

169-175: Prefer OperationCanceledException, fix selector typo, and clarify variable naming

• Using OperationCanceledException increases robustness across runtimes that may throw the base type.
• Typo: “non-existant” → “non-existent”.
• Rename navigationTask to selectorTask for clarity (it’s a selector wait, not a navigation).

Apply this diff:

-                var exception = await Assert.ThrowsAnyAsync<TaskCanceledException>(async () =>
+                var exception = await Assert.ThrowsAnyAsync<OperationCanceledException>(async () =>
                 {
-                    var navigationTask = browser.WaitForSelectorAsync("non-existant");
+                    var selectorTask = browser.WaitForSelectorAsync("non-existent");
                     var evaluateTask = browser.EvaluateScriptAsync($"setTimeout(() => window.location.href = '{url}', 100);");
 
-                    await Task.WhenAll(navigationTask, evaluateTask);
+                    await Task.WhenAll(selectorTask, evaluateTask);
                 });

📜 Review details

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

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 1bf7e15 and b0ba5e1.

📒 Files selected for processing (1)

• CefSharp.Test/Selector/WaitForSelectorAsyncTests.cs (2 hunks)

🧰 Additional context used 🧬 Code Graph Analysis (1) CefSharp.Test/Selector/WaitForSelectorAsyncTests.cs (1)

CefSharp/WebBrowserExtensions.cs (16)

• Task (322-327)
• Task (336-346)
• Task (355-360)
• Task (369-379)
• Task (418-485)
• Task (513-583)
• Task (612-705)
• Task (1196-1204)
• Task (1213-1218)
• Task (1353-1364)
• Task (1377-1382)
• Task (1603-1612)
• Task (1628-1633)
• Task (1653-1658)
• Task (1699-1712)
• Task (1729-1744)

🔇 Additional comments (1)

CefSharp.Test/Selector/WaitForSelectorAsyncTests.cs (1)

158-158: Rename aligns with new cancellation semantics

The test name change to ShouldCancelIfNavigationOccurs matches the PR’s per-frame cancellation behavior and clarifies intent.