Commit ade9db92 authored by Simon Zünd's avatar Simon Zünd Committed by Commit Bot

[devtools] Fix crash in LogHandler when multiple sessions are attached

The {OnQuicTransportHandshakeFailed} method uses {DispatchToAgents}
to send a console message to attached LogHandlers. There can
be potentially multiple LogHandlers, so passing a std::unique_ptr is
not possible. The first LogHandler receives a valid std::unique_ptr
but subsequent will only receive an empty one, causing a crash later
down the line.

This CL fixes the crash by passing a raw pointer and cloning the
log message before handing ownership over to the protocol.

R=sigurds@chromium.org

Bug: chromium:1069744, chromium:1084418
Change-Id: I6e1c2481b304481fb2b6c24e905f00562973af7f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2207182
Commit-Queue: Simon Zünd <szuend@chromium.org>
Reviewed-by: default avatarSigurd Schneider <sigurds@chromium.org>
Reviewed-by: default avatarYutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#770054}
parent d01b8c14
......@@ -766,7 +766,7 @@ void OnQuicTransportHandshakeFailed(
.SetText(text)
.SetTimestamp(base::Time::Now().ToDoubleT() * 1000.0)
.Build();
DispatchToAgents(ftn, &protocol::LogHandler::EntryAdded, std::move(entry));
DispatchToAgents(ftn, &protocol::LogHandler::EntryAdded, entry.get());
}
} // namespace devtools_instrumentation
......
......@@ -32,11 +32,11 @@ DispatchResponse LogHandler::Enable() {
return Response::FallThrough();
}
void LogHandler::EntryAdded(std::unique_ptr<Log::LogEntry> entry) {
void LogHandler::EntryAdded(Log::LogEntry* entry) {
if (!enabled_) {
return;
}
frontend_->EntryAdded(std::move(entry));
frontend_->EntryAdded(entry->clone());
}
} // namespace protocol
......
......@@ -31,7 +31,7 @@ class LogHandler final : public DevToolsDomainHandler, public Log::Backend {
DispatchResponse Disable() override;
DispatchResponse Enable() override;
void EntryAdded(std::unique_ptr<Log::LogEntry> entry);
void EntryAdded(Log::LogEntry* entry);
private:
std::unique_ptr<Log::Frontend> frontend_;
......
Check that multiple attached sessions don't crash the Log domain.
Log in session 1 enabled
Log in session 2 enabled
Trigger browser originating log message by instantiating QuicTransport.
Log.onEntryAdded: log message received
Log.onEntryAdded: log message received
(async function(testRunner) {
const {session, dp, page} = await testRunner.startBlank(
`Check that multiple attached sessions don't crash the Log domain.`);
const url = 'quic-transport://localhost';
await dp.Log.enable();
testRunner.log('Log in session 1 enabled');
const dp2 = (await page.createSession()).protocol;
await dp2.Log.enable();
testRunner.log('Log in session 2 enabled');
let onEntryAddedCallCount = 0;
const onEntryAddedHandler = event => {
testRunner.log(`Log.onEntryAdded: log message received`);
onEntryAddedCallCount++;
if (onEntryAddedCallCount == 2) {
testRunner.completeTest();
}
}
dp.Log.onEntryAdded(onEntryAddedHandler);
dp2.Log.onEntryAdded(onEntryAddedHandler);
// Taken from 'quic-transport-handshake-failure.js', failure to establish a connection
// causes a error message to be logged to the DevTools console.
testRunner.log('Trigger browser originating log message by instantiating QuicTransport.');
session.evaluate(`new QuicTransport('${url}');`);
})
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment