Skip to content

GitLab

T
tangled
Commit 33232afc authored by dmazzoni@google.com's avatar dmazzoni@google.com
Browse files
Options

Fix use-after-free of m_currentSpeechUtterance. 

SpeechSynthesis.cpp incorrectly assumed that calling
m_platformSpeechSynthesizer->cancel() would immediately call
didFinishSpeaking or speakingErrorOccurred, which would null out
m_currentSpeechUtterance. This assumption was true in WebKit/Mac, but
Chromium's platform implementation is asynchronous, so that call may
come later.

Fix the issue and simplify the logic by getting rid of the raw pointer
to the current utterance altogether. Now the RefPtr at the front of the
utterance queue is the current utterance, and the platform implementation
is allowed to fire events on utterances that are no longer in the queue.

BUG=344881
R=abarth@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=168092

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=168169

Review URL: https://codereview.chromium.org/180553004

git-svn-id: svn://svn.chromium.org/blink/trunk@168171 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 4dece178
Hide whitespace changes
Inline Side-by-side
Showing with 83 additions and 32 deletions
third_party/WebKit/LayoutTests/fast/speechsynthesis/speech-synthesis-cancel-twice-expected.txt 0 → 100644
View file @ 33232afc
This tests that cancelling an utterance a second time after garbage collection doesn't crash under ASAN.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS successfullyParsed is true
TEST COMPLETE
third_party/WebKit/LayoutTests/fast/speechsynthesis/speech-synthesis-cancel-twice.html 0 → 100644
View file @ 33232afc
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
<html>
<head>
<script src="../../resources/js-test.js"></script>
</head>
<body id="body">
<div id="console"></div>
<script>
description("This tests that cancelling an utterance a second time after garbage collection doesn't crash under ASAN.");
if (window.internals)
window.internals.enableMockSpeechSynthesizer(document);
speechSynthesis.speak(new SpeechSynthesisUtterance("Hello"));
speechSynthesis.cancel();
window.gc();
speechSynthesis.cancel();
</script>
</body>
</html>
third_party/WebKit/Source/modules/speech/SpeechSynthesis.cpp
View file @ 33232afc
......@@ -42,7 +42,6 @@ PassRefPtrWillBeRawPtr<SpeechSynthesis> SpeechSynthesis::create(ExecutionContext
SpeechSynthesis::SpeechSynthesis(ExecutionContext* context)
: ContextLifecycleObserver(context)
, m_platformSpeechSynthesizer(PlatformSpeechSynthesizer::create(this))
, m_currentSpeechUtterance(nullptr)
, m_isPaused(false)
{
ScriptWrappable::init(this);
......@@ -83,7 +82,7 @@ bool SpeechSynthesis::speaking() const
{
// If we have a current speech utterance, then that means we're assumed to be in a speaking state.
// This state is independent of whether the utterance happens to be paused.
return m_currentSpeechUtterance;
return currentSpeechUtterance();
}
bool SpeechSynthesis::pending() const
......@@ -98,11 +97,12 @@ bool SpeechSynthesis::paused() const
return m_isPaused;
}
void SpeechSynthesis::startSpeakingImmediately(SpeechSynthesisUtterance* utterance)
void SpeechSynthesis::startSpeakingImmediately()
{
ASSERT(!m_currentSpeechUtterance);
SpeechSynthesisUtterance* utterance = currentSpeechUtterance();
ASSERT(utterance);
utterance->setStartTime(monotonicallyIncreasingTime());
m_currentSpeechUtterance = utterance;
m_isPaused = false;
m_platformSpeechSynthesizer->speak(utterance->platformUtterance());
}
......@@ -116,22 +116,18 @@ void SpeechSynthesis::speak(SpeechSynthesisUtterance* utterance, ExceptionState&
m_utteranceQueue.append(utterance);
// If the queue was empty, speak this immediately and add it to the queue.
// If the queue was empty, speak this immediately.
if (m_utteranceQueue.size() == 1)
startSpeakingImmediately(utterance);
startSpeakingImmediately();
}
void SpeechSynthesis::cancel()
{
// Remove all the items from the utterance queue.
// Hold on to the current utterance so the platform synthesizer can have a chance to clean up.
RefPtrWillBeMember<SpeechSynthesisUtterance> current = m_currentSpeechUtterance;
// Remove all the items from the utterance queue. The platform
// may still have references to some of these utterances and may
// fire events on them asynchronously.
m_utteranceQueue.clear();
m_platformSpeechSynthesizer->cancel();
current = nullptr;
// The platform should have called back immediately and cleared the current utterance.
ASSERT(!m_currentSpeechUtterance);
}
void SpeechSynthesis::pause()
......@@ -142,7 +138,7 @@ void SpeechSynthesis::pause()
void SpeechSynthesis::resume()
{
if (!m_currentSpeechUtterance)
if (!currentSpeechUtterance())
return;
m_platformSpeechSynthesizer->resume();
}
......@@ -156,21 +152,24 @@ void SpeechSynthesis::fireEvent(const AtomicString& type, SpeechSynthesisUtteran
void SpeechSynthesis::handleSpeakingCompleted(SpeechSynthesisUtterance* utterance, bool errorOccurred)
{
ASSERT(utterance);
ASSERT(m_currentSpeechUtterance);
m_currentSpeechUtterance = nullptr;
fireEvent(errorOccurred ? EventTypeNames::error : EventTypeNames::end, utterance, 0, String());
bool didJustFinishCurrentUtterance = false;
// If the utterance that completed was the one we're currently speaking,
// remove it from the queue and start speaking the next one.
if (utterance == currentSpeechUtterance()) {
m_utteranceQueue.removeFirst();
didJustFinishCurrentUtterance = true;
}
if (m_utteranceQueue.size()) {
RefPtrWillBeMember<SpeechSynthesisUtterance> firstUtterance = m_utteranceQueue.first();
ASSERT(firstUtterance == utterance);
if (firstUtterance == utterance)
m_utteranceQueue.removeFirst();
// Always fire the event, because the platform may have asynchronously
// sent an event on an utterance before it got the message that we
// canceled it, and we should always report to the user what actually
// happened.
fireEvent(errorOccurred ? EventTypeNames::error : EventTypeNames::end, utterance, 0, String());
// Start the next job if there is one pending.
if (!m_utteranceQueue.isEmpty())
startSpeakingImmediately(m_utteranceQueue.first().get());
}
// Start the next utterance if we just finished one and one was pending.
if (didJustFinishCurrentUtterance && !m_utteranceQueue.isEmpty())
startSpeakingImmediately();
}
void SpeechSynthesis::boundaryEventOccurred(PassRefPtr<PlatformSpeechSynthesisUtterance> utterance, SpeechBoundary boundary, unsigned charIndex)
......@@ -222,6 +221,13 @@ void SpeechSynthesis::speakingErrorOccurred(PassRefPtr<PlatformSpeechSynthesisUt
handleSpeakingCompleted(static_cast<SpeechSynthesisUtterance*>(utterance->client()), true);
}
SpeechSynthesisUtterance* SpeechSynthesis::currentSpeechUtterance() const
{
if (!m_utteranceQueue.isEmpty())
return m_utteranceQueue.first().get();
return 0;
}
const AtomicString& SpeechSynthesis::interfaceName() const
{
return EventTargetNames::SpeechSynthesisUtterance;
......@@ -230,7 +236,6 @@ const AtomicString& SpeechSynthesis::interfaceName() const
void SpeechSynthesis::trace(Visitor* visitor)
{
visitor->trace(m_voiceList);
visitor->trace(m_currentSpeechUtterance);
visitor->trace(m_utteranceQueue);
}
......
third_party/WebKit/Source/modules/speech/SpeechSynthesis.h
View file @ 33232afc
......@@ -81,13 +81,15 @@ private:
virtual void speakingErrorOccurred(PassRefPtr<PlatformSpeechSynthesisUtterance>) OVERRIDE;
virtual void boundaryEventOccurred(PassRefPtr<PlatformSpeechSynthesisUtterance>, SpeechBoundary, unsigned charIndex) OVERRIDE;
void startSpeakingImmediately(SpeechSynthesisUtterance*);
void startSpeakingImmediately();
void handleSpeakingCompleted(SpeechSynthesisUtterance*, bool errorOccurred);
void fireEvent(const AtomicString& type, SpeechSynthesisUtterance*, unsigned long charIndex, const String& name);
// Returns the utterance at the front of the queue.
SpeechSynthesisUtterance* currentSpeechUtterance() const;
OwnPtr<PlatformSpeechSynthesizer> m_platformSpeechSynthesizer;
WillBeHeapVector<RefPtrWillBeMember<SpeechSynthesisVoice> > m_voiceList;
RawPtrWillBeMember<SpeechSynthesisUtterance> m_currentSpeechUtterance;
Deque<RefPtrWillBeMember<SpeechSynthesisUtterance> > m_utteranceQueue;
bool m_isPaused;
......
third_party/WebKit/Source/modules/speech/testing/PlatformSpeechSynthesizerMock.cpp
View file @ 33232afc
......@@ -42,12 +42,14 @@ PassOwnPtr<PlatformSpeechSynthesizerMock> PlatformSpeechSynthesizerMock::create(
PlatformSpeechSynthesizerMock::PlatformSpeechSynthesizerMock(PlatformSpeechSynthesizerClient* client)
: PlatformSpeechSynthesizer(client)
, m_speakingFinishedTimer(this, &PlatformSpeechSynthesizerMock::speakingFinished)
, m_speakingErrorOccurredTimer(this, &PlatformSpeechSynthesizerMock::speakingErrorOccurred)
{
}
PlatformSpeechSynthesizerMock::~PlatformSpeechSynthesizerMock()
{
m_speakingFinishedTimer.stop();
m_speakingErrorOccurredTimer.stop();
}
void PlatformSpeechSynthesizerMock::speakingFinished(Timer<PlatformSpeechSynthesizerMock>*)
......@@ -57,6 +59,13 @@ void PlatformSpeechSynthesizerMock::speakingFinished(Timer<PlatformSpeechSynthes
m_utterance = nullptr;
}
void PlatformSpeechSynthesizerMock::speakingErrorOccurred(Timer<PlatformSpeechSynthesizerMock>*)
{
ASSERT(m_utterance.get());
client()->speakingErrorOccurred(m_utterance);
m_utterance = nullptr;
}
void PlatformSpeechSynthesizerMock::initializeVoiceList()
{
m_voiceList.clear();
......@@ -85,8 +94,7 @@ void PlatformSpeechSynthesizerMock::cancel()
return;
m_speakingFinishedTimer.stop();
client()->speakingErrorOccurred(m_utterance);
m_utterance = nullptr;
m_speakingErrorOccurredTimer.startOneShot(.1);
}
void PlatformSpeechSynthesizerMock::pause()
......
third_party/WebKit/Source/modules/speech/testing/PlatformSpeechSynthesizerMock.h
View file @ 33232afc
......@@ -46,8 +46,10 @@ private:
explicit PlatformSpeechSynthesizerMock(PlatformSpeechSynthesizerClient*);
virtual void initializeVoiceList() OVERRIDE;
void speakingFinished(Timer<PlatformSpeechSynthesizerMock>*);
void speakingErrorOccurred(Timer<PlatformSpeechSynthesizerMock>*);
Timer<PlatformSpeechSynthesizerMock> m_speakingFinishedTimer;
Timer<PlatformSpeechSynthesizerMock> m_speakingErrorOccurredTimer;
RefPtr<PlatformSpeechSynthesisUtterance> m_utterance;
};
......
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