Skip to content

Commit cf5f836

Browse files
campersauamaitland
campersau
authored and
amaitland
committed
Core - Cancel pending tasks when browser crashed or v8 context gets released (#5145)
* Core - Cancel pending tasks when browser crashed or v8 context gets released * Use TrySetCanceled to cancel pending tasks * Adjust test to use chrome://crash for simulating a crash * Add tests for javascript callbacks * Rename tests and add tests for calling Execute multiple times * Move FramePendingTaskRepository to its own file * Rename test to ShouldCancelAfterV8ContextChange * Fix compile error on .NET Framework * adjust ShouldTimeoutIfNavigationOccurs test which now throws a TaskCanceledException on navigation
1 parent c1f3353 commit cf5f836

File tree

10 files changed

+287
-51
lines changed

10 files changed

+287
-51
lines changed

‎CefSharp.Core.Runtime/Internals/CefFrameWrapper.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -237,15 +237,15 @@ Task<JavascriptResponse^>^ CefFrameWrapper::EvaluateScriptAsync(String^ script,
237237
//If we're unable to get the underlying browser/browserhost then return null
238238
if (!browser.get() || !host.get())
239239
{
240-
return nullptr;
240+
return Task::FromException<JavascriptResponse^>(gcnew InvalidOperationException("Browser host not available"));
241241
}
242242

243243
auto client = static_cast<ClientAdapter*>(host->GetClient().get());
244244

245245
auto pendingTaskRepository = client->GetPendingTaskRepository();
246246

247247
//create a new taskcompletionsource
248-
auto idAndComplectionSource = pendingTaskRepository->CreatePendingTask(timeout);
248+
auto idAndComplectionSource = pendingTaskRepository->CreatePendingTask(Identifier, timeout);
249249

250250
if (useImmediatelyInvokedFuncExpression)
251251
{

‎CefSharp.Core.Runtime/Internals/ClientAdapter.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -701,6 +701,8 @@ namespace CefSharp
701701

702702
void ClientAdapter::OnRenderProcessTerminated(CefRefPtr<CefBrowser> browser, TerminationStatus status, int errorCode, const CefString& errorString)
703703
{
704+
_pendingTaskRepository->CancelPendingTasks();
705+
704706
auto handler = _browserControl->RequestHandler;
705707

706708
if (handler != nullptr)
@@ -1382,9 +1384,13 @@ namespace CefSharp
13821384
//we get here, only continue if we have a valid frame reference
13831385
if (frame.get() && frame->IsValid())
13841386
{
1387+
auto frameId = StringUtils::ToClr(frame->GetIdentifier());
1388+
1389+
_pendingTaskRepository->CancelPendingTasks(frameId);
1390+
13851391
if (frame->IsMain())
13861392
{
1387-
_browserControl->SetCanExecuteJavascriptOnMainFrame(StringUtils::ToClr(frame->GetIdentifier()), false);
1393+
_browserControl->SetCanExecuteJavascriptOnMainFrame(frameId, false);
13881394
}
13891395

13901396
auto handler = _browserControl->RenderProcessMessageHandler;
@@ -1475,14 +1481,16 @@ namespace CefSharp
14751481
return true;
14761482
}
14771483

1484+
auto frameId = StringUtils::ToClr(frame->GetIdentifier());
1485+
14781486
auto callbackFactory = browserAdapter->JavascriptCallbackFactory;
14791487

14801488
auto success = argList->GetBool(0);
14811489
auto callbackId = GetInt64(argList, 1);
14821490

14831491
auto pendingTask = name == kEvaluateJavascriptResponse ?
1484-
_pendingTaskRepository->RemovePendingTask(callbackId) :
1485-
_pendingTaskRepository->RemoveJavascriptCallbackPendingTask(callbackId);
1492+
_pendingTaskRepository->RemovePendingTask(frameId, callbackId) :
1493+
_pendingTaskRepository->RemoveJavascriptCallbackPendingTask(frameId, callbackId);
14861494

14871495
if (pendingTask != nullptr)
14881496
{

‎CefSharp.Core.Runtime/Internals/ClientAdapter.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,7 @@ namespace CefSharp
7070

7171
CloseAllPopups(true);
7272

73-
//this will dispose the repository and cancel all pending tasks
74-
delete _pendingTaskRepository;
73+
_pendingTaskRepository->CancelPendingTasks();
7574

7675
_browser = nullptr;
7776
_browserControl = nullptr;
@@ -80,6 +79,7 @@ namespace CefSharp
8079
_tooltip = nullptr;
8180
_browserAdapter = nullptr;
8281
_popupBrowsers = nullptr;
82+
_pendingTaskRepository = nullptr;
8383
}
8484

8585
HWND GetBrowserHwnd() { return _browserHwnd; }

‎CefSharp.Core.Runtime/Internals/JavascriptCallbackProxy.cpp

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -31,26 +31,27 @@ namespace CefSharp
3131
}
3232

3333
auto browserWrapper = static_cast<CefBrowserWrapper^>(browser);
34-
auto javascriptNameConverter = GetJavascriptNameConverter();
35-
36-
auto doneCallback = _pendingTasks->CreateJavascriptCallbackPendingTask(_callback->Id, timeout);
37-
38-
auto callbackMessage = CefProcessMessage::Create(kJavascriptCallbackRequest);
39-
auto argList = callbackMessage->GetArgumentList();
40-
SetInt64(argList, 0, doneCallback.Key);
41-
SetInt64(argList, 1, _callback->Id);
42-
auto paramList = CefListValue::Create();
43-
for (int i = 0; i < parameters->Length; i++)
44-
{
45-
auto param = parameters[i];
46-
SerializeV8Object(paramList, i, param, javascriptNameConverter);
47-
}
48-
argList->SetList(2, paramList);
4934

5035
auto frame = browserWrapper->Browser->GetFrameByIdentifier(StringUtils::ToNative(_callback->FrameId));
5136

5237
if (frame.get() && frame->IsValid())
5338
{
39+
auto javascriptNameConverter = GetJavascriptNameConverter();
40+
41+
auto doneCallback = _pendingTasks->CreateJavascriptCallbackPendingTask(_callback->FrameId, _callback->Id, timeout);
42+
43+
auto callbackMessage = CefProcessMessage::Create(kJavascriptCallbackRequest);
44+
auto argList = callbackMessage->GetArgumentList();
45+
SetInt64(argList, 0, doneCallback.Key);
46+
SetInt64(argList, 1, _callback->Id);
47+
auto paramList = CefListValue::Create();
48+
for (int i = 0; i < parameters->Length; i++)
49+
{
50+
auto param = parameters[i];
51+
SerializeV8Object(paramList, i, param, javascriptNameConverter);
52+
}
53+
argList->SetList(2, paramList);
54+
5455
frame->SendProcessMessage(CefProcessId::PID_RENDERER, callbackMessage);
5556

5657
return doneCallback.Value->Task;

‎CefSharp.Test/Javascript/EvaluateScriptAsyncTests.cs

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.Text;
99
using System.Threading.Tasks;
1010
using Bogus;
11+
using CefSharp.Example;
1112
using Xunit;
1213
using Xunit.Abstractions;
1314
using Xunit.Repeat;
@@ -26,6 +27,52 @@ public EvaluateScriptAsyncTests(ITestOutputHelper output, CefSharpFixture collec
2627
this.collectionFixture = collectionFixture;
2728
}
2829

30+
[Fact]
31+
public async Task ShouldCancelAfterV8ContextChange()
32+
{
33+
Task evaluateCancelAfterDisposeTask;
34+
using (var browser = new CefSharp.OffScreen.ChromiumWebBrowser(automaticallyCreateBrowser: false))
35+
{
36+
await browser.CreateBrowserAsync();
37+
38+
// no V8 context
39+
var withoutV8ContextException = await Assert.ThrowsAsync<Exception>(() => browser.EvaluateScriptAsync("1+1"));
40+
Assert.StartsWith("Unable to execute javascript at this time", withoutV8ContextException.Message);
41+
42+
Task<JavascriptResponse> evaluateWithoutV8ContextTask;
43+
using (var frame = browser.GetMainFrame())
44+
{
45+
evaluateWithoutV8ContextTask = frame.EvaluateScriptAsync("1+2");
46+
}
47+
48+
// V8 context
49+
await browser.LoadUrlAsync(CefExample.HelloWorldUrl);
50+
var evaluateWithoutV8ContextResponse = await evaluateWithoutV8ContextTask;
51+
Assert.True(evaluateWithoutV8ContextResponse.Success);
52+
Assert.Equal(3, evaluateWithoutV8ContextResponse.Result);
53+
54+
var evaluateCancelAfterV8ContextChangeTask = browser.EvaluateScriptAsync("new Promise(resolve => setTimeout(resolve, 1000))");
55+
56+
// change V8 context
57+
await browser.LoadUrlAsync(CefExample.HelloWorldUrl);
58+
59+
await Assert.ThrowsAsync<TaskCanceledException>(() => evaluateCancelAfterV8ContextChangeTask);
60+
61+
evaluateCancelAfterDisposeTask = browser.EvaluateScriptAsync("new Promise(resolve => setTimeout(resolve, 1000))");
62+
}
63+
await Assert.ThrowsAsync<TaskCanceledException>(() => evaluateCancelAfterDisposeTask);
64+
}
65+
66+
[Fact]
67+
public async Task ShouldCancelOnCrash()
68+
{
69+
AssertInitialLoadComplete();
70+
71+
var task = Browser.EvaluateScriptAsync("new Promise(resolve => setTimeout(resolve, 1000))");
72+
await Browser.LoadUrlAsync("chrome://crash");
73+
await Assert.ThrowsAsync<TaskCanceledException>(() => task);
74+
}
75+
2976
[Theory]
3077
[InlineData(double.MaxValue, "Number.MAX_VALUE")]
3178
[InlineData(double.MaxValue / 2, "Number.MAX_VALUE / 2")]
@@ -264,7 +311,7 @@ public async Task CanEvaluateScriptAsyncReturnArrayBuffer(int iteration)
264311

265312
var randomizer = new Randomizer();
266313

267-
var expected = randomizer.Utf16String(minLength: iteration, maxLength:iteration);
314+
var expected = randomizer.Utf16String(minLength: iteration, maxLength: iteration);
268315
var expectedBytes = Encoding.UTF8.GetBytes(expected);
269316

270317
var javascriptResponse = await Browser.EvaluateScriptAsync($"new TextEncoder().encode('{expected}').buffer");

‎CefSharp.Test/Javascript/JavascriptCallbackTests.cs

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Dynamic;
77
using System.Globalization;
88
using System.Threading.Tasks;
9+
using CefSharp.Example;
910
using Xunit;
1011
using Xunit.Abstractions;
1112

@@ -23,6 +24,74 @@ public JavascriptCallbackTests(ITestOutputHelper output, CefSharpFixture collect
2324
this.collectionFixture = collectionFixture;
2425
}
2526

27+
[Fact]
28+
public async Task ShouldCancelAfterV8ContextChange()
29+
{
30+
IJavascriptCallback callbackExecuteCancelAfterDisposeCallback;
31+
Task callbackExecuteCancelAfterDisposeTask;
32+
using (var browser = new CefSharp.OffScreen.ChromiumWebBrowser(automaticallyCreateBrowser: false))
33+
{
34+
await browser.CreateBrowserAsync();
35+
36+
// no V8 context
37+
var withoutV8ContextException = await Assert.ThrowsAsync<Exception>(() => browser.EvaluateScriptAsync("(function() { return 1+1; })"));
38+
Assert.StartsWith("Unable to execute javascript at this time", withoutV8ContextException.Message);
39+
40+
Task<JavascriptResponse> callbackExecuteWithoutV8ContextTask;
41+
using (var frame = browser.GetMainFrame())
42+
{
43+
callbackExecuteWithoutV8ContextTask = frame.EvaluateScriptAsync("(function() { return 1+2; })");
44+
}
45+
46+
// V8 context
47+
await browser.LoadUrlAsync(CefExample.HelloWorldUrl);
48+
49+
var callbackExecuteWithoutV8ContextResponse = await callbackExecuteWithoutV8ContextTask;
50+
Assert.True(callbackExecuteWithoutV8ContextResponse.Success);
51+
var callbackExecuteWithoutV8ContextCallback = (IJavascriptCallback)callbackExecuteWithoutV8ContextResponse.Result;
52+
var callbackExecuteWithoutV8ContextExecuteResponse = await callbackExecuteWithoutV8ContextCallback.ExecuteAsync();
53+
Assert.True(callbackExecuteWithoutV8ContextExecuteResponse.Success);
54+
Assert.Equal(3, callbackExecuteWithoutV8ContextExecuteResponse.Result);
55+
56+
var callbackExecuteCancelAfterV8ContextResponse = await browser.EvaluateScriptAsync("(function() { return new Promise(resolve => setTimeout(resolve, 1000)); })");
57+
Assert.True(callbackExecuteCancelAfterV8ContextResponse.Success);
58+
var callbackExecuteCancelAfterV8ContextCallback = (IJavascriptCallback)callbackExecuteCancelAfterV8ContextResponse.Result;
59+
var callbackExecuteCancelAfterV8ContextTask = callbackExecuteCancelAfterV8ContextCallback.ExecuteAsync();
60+
61+
// change V8 context
62+
await browser.LoadUrlAsync(CefExample.HelloWorldUrl);
63+
64+
await Assert.ThrowsAsync<TaskCanceledException>(() => callbackExecuteCancelAfterV8ContextTask);
65+
var callbackExecuteCancelAfterV8ContextResult = await callbackExecuteCancelAfterV8ContextCallback.ExecuteAsync();
66+
Assert.False(callbackExecuteCancelAfterV8ContextResult.Success);
67+
Assert.StartsWith("Unable to find JavascriptCallback with Id " + callbackExecuteCancelAfterV8ContextCallback.Id, callbackExecuteCancelAfterV8ContextResult.Message);
68+
69+
var callbackExecuteCancelAfterDisposeResponse = await browser.EvaluateScriptAsync("(function() { return new Promise(resolve => setTimeout(resolve, 1000)); })");
70+
Assert.True(callbackExecuteCancelAfterDisposeResponse.Success);
71+
callbackExecuteCancelAfterDisposeCallback = (IJavascriptCallback)callbackExecuteCancelAfterDisposeResponse.Result;
72+
callbackExecuteCancelAfterDisposeTask = callbackExecuteCancelAfterDisposeCallback.ExecuteAsync();
73+
}
74+
Assert.False(callbackExecuteCancelAfterDisposeCallback.CanExecute);
75+
await Assert.ThrowsAsync<TaskCanceledException>(() => callbackExecuteCancelAfterDisposeTask);
76+
await Assert.ThrowsAsync<InvalidOperationException>(() => callbackExecuteCancelAfterDisposeCallback.ExecuteAsync());
77+
}
78+
79+
[Fact]
80+
public async Task ShouldCancelOnCrash()
81+
{
82+
AssertInitialLoadComplete();
83+
84+
var javascriptResponse = await Browser.EvaluateScriptAsync("(function() { return new Promise(resolve => setTimeout(resolve, 1000)); })");
85+
Assert.True(javascriptResponse.Success);
86+
87+
var callback = (IJavascriptCallback)javascriptResponse.Result;
88+
89+
var task = callback.ExecuteAsync();
90+
91+
await Browser.LoadUrlAsync("chrome://crash");
92+
await Assert.ThrowsAsync<TaskCanceledException>(() => task);
93+
}
94+
2695
[Theory]
2796
[InlineData("(function() { return Promise.resolve(53)})", 53)]
2897
[InlineData("(function() { return Promise.resolve('53')})", "53")]
@@ -233,5 +302,23 @@ public async Task ShouldWorkWithExpandoObject()
233302

234303
output.WriteLine("Expected {0} : Actual {1}", expectedDateTime, actualDateTime);
235304
}
305+
306+
[Fact]
307+
public async Task ShouldWorkWhenExecutedMultipleTimes()
308+
{
309+
AssertInitialLoadComplete();
310+
311+
var javascriptResponse = await Browser.EvaluateScriptAsync("(function() { return 42; })");
312+
Assert.True(javascriptResponse.Success);
313+
314+
var callback = (IJavascriptCallback)javascriptResponse.Result;
315+
316+
for (var i = 0; i < 3; i++)
317+
{
318+
var callbackResponse = await callback.ExecuteAsync();
319+
Assert.True(callbackResponse.Success);
320+
Assert.Equal(42, callbackResponse.Result);
321+
}
322+
}
236323
}
237324
}

‎CefSharp.Test/Selector/WaitForSelectorAsyncTests.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -155,9 +155,9 @@ public async Task CanTimeout()
155155
}
156156

157157
[Fact]
158-
public async Task ShouldTimeoutIfNavigationOccurs()
158+
public async Task ShouldCancelIfNavigationOccurs()
159159
{
160-
const string expected = "The operation has timed out.";
160+
const string expected = "A task was canceled.";
161161
const string url = CefExample.HelloWorldUrl;
162162

163163
using (var browser = new ChromiumWebBrowser(CefExample.DefaultUrl, useLegacyRenderHandler: false))
@@ -166,7 +166,7 @@ public async Task ShouldTimeoutIfNavigationOccurs()
166166

167167
Assert.True(response.Success);
168168

169-
var exception = await Assert.ThrowsAnyAsync<TimeoutException>(async () =>
169+
var exception = await Assert.ThrowsAnyAsync<TaskCanceledException>(async () =>
170170
{
171171
var navigationTask = browser.WaitForSelectorAsync("non-existant");
172172
var evaluateTask = browser.EvaluateScriptAsync($"setTimeout(() => window.location.href = '{url}', 100);");
@@ -175,7 +175,6 @@ public async Task ShouldTimeoutIfNavigationOccurs()
175175
});
176176

177177
Assert.Contains(expected, exception.Message);
178-
Assert.Contains(url, browser.GetMainFrame().Url);
179178

180179
output.WriteLine("Exception {0}", exception.Message);
181180
}

‎CefSharp/Internals/FramePendingTaskRepository.cs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Copyright © 2015 The CefSharp Authors. All rights reserved.
2+
//
3+
// Use of this source code is governed by a BSD-style license that can be found in the LICENSE file.
4+
5+
using System;
6+
using System.Collections.Concurrent;
7+
using System.Threading.Tasks;
8+
9+
namespace CefSharp.Internals
10+
{
11+
internal sealed class FramePendingTaskRepository<TResult> : IDisposable
12+
{
13+
public ConcurrentDictionary<long, TaskCompletionSource<TResult>> PendingTasks { get; } =
14+
new ConcurrentDictionary<long, TaskCompletionSource<TResult>>();
15+
public ConcurrentDictionary<long, TaskCompletionSource<TResult>> CallbackPendingTasks { get; } =
16+
new ConcurrentDictionary<long, TaskCompletionSource<TResult>>();
17+
18+
public void Dispose()
19+
{
20+
foreach (var tcs in PendingTasks.Values)
21+
{
22+
tcs.TrySetCanceled();
23+
}
24+
PendingTasks.Clear();
25+
26+
foreach (var tcs in CallbackPendingTasks.Values)
27+
{
28+
tcs.TrySetCanceled();
29+
}
30+
CallbackPendingTasks.Clear();
31+
}
32+
}
33+
}

0 commit comments

Comments
 (0)