Drop [UsePointersEvenForNonNullableObjectArguments] from SpeechSynthesis
authoryouenn.fablet@crf.canon.fr <youenn.fablet@crf.canon.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 26 Apr 2016 13:20:38 +0000 (13:20 +0000)
committeryouenn.fablet@crf.canon.fr <youenn.fablet@crf.canon.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 26 Apr 2016 13:20:38 +0000 (13:20 +0000)
https://bugs.webkit.org/show_bug.cgi?id=156901

Reviewed by Darin Adler.

Source/WebCore:

SpeechSynthesis.speak will now throw in case of bad parameters instead of silently failing.
Started refactoring to use more references where possible.

Covered by updated test.

* Modules/speech/SpeechSynthesis.cpp:
(WebCore::SpeechSynthesis::startSpeakingImmediately): Refactored to get a reference.
(WebCore::SpeechSynthesis::speak):
(WebCore::SpeechSynthesis::fireEvent):
(WebCore::SpeechSynthesis::handleSpeakingCompleted): Removing first item in utteranceQueue unconditionally,
since that would crash in Debug mode otherwise.
(WebCore::SpeechSynthesis::boundaryEventOccurred):
(WebCore::SpeechSynthesis::didStartSpeaking):
(WebCore::SpeechSynthesis::didPauseSpeaking):
(WebCore::SpeechSynthesis::didResumeSpeaking):
(WebCore::SpeechSynthesis::didFinishSpeaking):
(WebCore::SpeechSynthesis::speakingErrorOccurred):
* Modules/speech/SpeechSynthesis.h:
* Modules/speech/SpeechSynthesis.idl:

LayoutTests:

Updated test to handle speak throwing behavior in case of bad parameters.

* fast/speechsynthesis/speech-synthesis-crash-on-bad-utterance-expected.txt:
* fast/speechsynthesis/speech-synthesis-crash-on-bad-utterance.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@200080 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/speechsynthesis/speech-synthesis-crash-on-bad-utterance-expected.txt
LayoutTests/fast/speechsynthesis/speech-synthesis-crash-on-bad-utterance.html
Source/WebCore/ChangeLog
Source/WebCore/Modules/speech/SpeechSynthesis.cpp
Source/WebCore/Modules/speech/SpeechSynthesis.h
Source/WebCore/Modules/speech/SpeechSynthesis.idl

index 1b49a3e..6294270 100644 (file)
@@ -1,5 +1,17 @@
 2016-04-26  Youenn Fablet  <youenn.fablet@crf.canon.fr>
 
+        Drop [UsePointersEvenForNonNullableObjectArguments] from SpeechSynthesis
+        https://bugs.webkit.org/show_bug.cgi?id=156901
+
+        Reviewed by Darin Adler.
+
+        Updated test to handle speak throwing behavior in case of bad parameters.
+
+        * fast/speechsynthesis/speech-synthesis-crash-on-bad-utterance-expected.txt:
+        * fast/speechsynthesis/speech-synthesis-crash-on-bad-utterance.html:
+
+2016-04-26  Youenn Fablet  <youenn.fablet@crf.canon.fr>
+
         Drop [UsePointersEvenForNonNullableObjectArguments] from EventTarget
         https://bugs.webkit.org/show_bug.cgi?id=156977
 
index 99537bf..536fa8e 100644 (file)
@@ -3,6 +3,7 @@ This tests that passing in the wrong type of data won't crash speech synthesis c
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
+PASS speechSynthesis.speak('Hello World') threw exception TypeError: Type error.
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 27d4df2..9d54094 100644 (file)
@@ -12,7 +12,7 @@
     description("This tests that passing in the wrong type of data won't crash speech synthesis code");
 
      // Don't crash. Speak is supposed to take an utterance, not a string.
-     speechSynthesis.speak('Hello World');
+     shouldThrow("speechSynthesis.speak('Hello World')");
 
      // Don't crash. An utterance voice is supposed to take a voice object, not a string.
      var x = new SpeechSynthesisUtterance('Hello World');
index cef3b58..d4cf28f 100644 (file)
@@ -1,5 +1,32 @@
 2016-04-26  Youenn Fablet  <youenn.fablet@crf.canon.fr>
 
+        Drop [UsePointersEvenForNonNullableObjectArguments] from SpeechSynthesis
+        https://bugs.webkit.org/show_bug.cgi?id=156901
+
+        Reviewed by Darin Adler.
+
+        SpeechSynthesis.speak will now throw in case of bad parameters instead of silently failing.
+        Started refactoring to use more references where possible.
+
+        Covered by updated test.
+
+        * Modules/speech/SpeechSynthesis.cpp:
+        (WebCore::SpeechSynthesis::startSpeakingImmediately): Refactored to get a reference.
+        (WebCore::SpeechSynthesis::speak):
+        (WebCore::SpeechSynthesis::fireEvent):
+        (WebCore::SpeechSynthesis::handleSpeakingCompleted): Removing first item in utteranceQueue unconditionally,
+        since that would crash in Debug mode otherwise.
+        (WebCore::SpeechSynthesis::boundaryEventOccurred):
+        (WebCore::SpeechSynthesis::didStartSpeaking):
+        (WebCore::SpeechSynthesis::didPauseSpeaking):
+        (WebCore::SpeechSynthesis::didResumeSpeaking):
+        (WebCore::SpeechSynthesis::didFinishSpeaking):
+        (WebCore::SpeechSynthesis::speakingErrorOccurred):
+        * Modules/speech/SpeechSynthesis.h:
+        * Modules/speech/SpeechSynthesis.idl:
+
+2016-04-26  Youenn Fablet  <youenn.fablet@crf.canon.fr>
+
         Drop [UsePointersEvenForNonNullableObjectArguments] from WebKitNamedFlow
         https://bugs.webkit.org/show_bug.cgi?id=156979
 
index 7f17503..745c9b9 100644 (file)
@@ -100,29 +100,26 @@ bool SpeechSynthesis::paused() const
     return m_isPaused;
 }
 
-void SpeechSynthesis::startSpeakingImmediately(SpeechSynthesisUtterance* utterance)
+void SpeechSynthesis::startSpeakingImmediately(SpeechSynthesisUtterance& utterance)
 {
     ASSERT(!m_currentSpeechUtterance);
-    utterance->setStartTime(monotonicallyIncreasingTime());
-    m_currentSpeechUtterance = utterance;
+    utterance.setStartTime(monotonicallyIncreasingTime());
+    m_currentSpeechUtterance = &utterance;
     m_isPaused = false;
     
     // Zero lengthed strings should immediately notify that the event is complete.
-    if (utterance->text().isEmpty()) {
+    if (utterance.text().isEmpty()) {
         handleSpeakingCompleted(utterance, false);
         return;
     }
     
     if (!m_platformSpeechSynthesizer)
         m_platformSpeechSynthesizer = std::make_unique<PlatformSpeechSynthesizer>(this);
-    m_platformSpeechSynthesizer->speak(utterance->platformUtterance());
+    m_platformSpeechSynthesizer->speak(utterance.platformUtterance());
 }
 
-void SpeechSynthesis::speak(SpeechSynthesisUtterance* utterance)
+void SpeechSynthesis::speak(SpeechSynthesisUtterance& utterance)
 {
-    if (!utterance)
-        return;
     // Like Audio, we should require that the user interact to start a speech synthesis session.
 #if PLATFORM(IOS)
     if (ScriptController::processingUserGesture())
@@ -135,7 +132,7 @@ void SpeechSynthesis::speak(SpeechSynthesisUtterance* utterance)
     
     // If the queue was empty, speak this immediately and add it to the queue.
     if (m_utteranceQueue.size() == 1)
-        startSpeakingImmediately(utterance);
+        startSpeakingImmediately(m_utteranceQueue.first());
 }
 
 void SpeechSynthesis::cancel()
@@ -164,30 +161,27 @@ void SpeechSynthesis::resume()
         m_platformSpeechSynthesizer->resume();
 }
 
-void SpeechSynthesis::fireEvent(const AtomicString& type, SpeechSynthesisUtterance* utterance, unsigned long charIndex, const String& name)
+void SpeechSynthesis::fireEvent(const AtomicString& type, SpeechSynthesisUtterance& utterance, unsigned long charIndex, const String& name)
 {
-    utterance->dispatchEvent(SpeechSynthesisEvent::create(type, charIndex, (monotonicallyIncreasingTime() - utterance->startTime()), name));
+    utterance.dispatchEvent(SpeechSynthesisEvent::create(type, charIndex, (monotonicallyIncreasingTime() - utterance.startTime()), name));
 }
     
-void SpeechSynthesis::handleSpeakingCompleted(SpeechSynthesisUtterance* utterance, bool errorOccurred)
+void SpeechSynthesis::handleSpeakingCompleted(SpeechSynthesisUtterance& utterance, bool errorOccurred)
 {
-    ASSERT(utterance);
     ASSERT(m_currentSpeechUtterance);
-    RefPtr<SpeechSynthesisUtterance> protect(utterance);
+    Ref<SpeechSynthesisUtterance> protect(utterance);
     
     m_currentSpeechUtterance = nullptr;
 
     fireEvent(errorOccurred ? eventNames().errorEvent : eventNames().endEvent, utterance, 0, String());
 
     if (m_utteranceQueue.size()) {
-        RefPtr<SpeechSynthesisUtterance> firstUtterance = m_utteranceQueue.first();
-        ASSERT(firstUtterance == utterance);
-        if (firstUtterance == utterance)
-            m_utteranceQueue.removeFirst();
-        
+        Ref<SpeechSynthesisUtterance> firstUtterance = m_utteranceQueue.takeFirst();
+        ASSERT(&utterance == firstUtterance.ptr());
+
         // Start the next job if there is one pending.
         if (!m_utteranceQueue.isEmpty())
-            startSpeakingImmediately(m_utteranceQueue.first().get());
+            startSpeakingImmediately(m_utteranceQueue.first());
     }
 }
     
@@ -196,12 +190,15 @@ void SpeechSynthesis::boundaryEventOccurred(PassRefPtr<PlatformSpeechSynthesisUt
     static NeverDestroyed<const String> wordBoundaryString(ASCIILiteral("word"));
     static NeverDestroyed<const String> sentenceBoundaryString(ASCIILiteral("sentence"));
 
+    ASSERT(utterance);
+    ASSERT(utterance->client());
+
     switch (boundary) {
     case SpeechWordBoundary:
-        fireEvent(eventNames().boundaryEvent, static_cast<SpeechSynthesisUtterance*>(utterance->client()), charIndex, wordBoundaryString);
+        fireEvent(eventNames().boundaryEvent, static_cast<SpeechSynthesisUtterance&>(*utterance->client()), charIndex, wordBoundaryString);
         break;
     case SpeechSentenceBoundary:
-        fireEvent(eventNames().boundaryEvent, static_cast<SpeechSynthesisUtterance*>(utterance->client()), charIndex, sentenceBoundaryString);
+        fireEvent(eventNames().boundaryEvent, static_cast<SpeechSynthesisUtterance&>(*utterance->client()), charIndex, sentenceBoundaryString);
         break;
     default:
         ASSERT_NOT_REACHED();
@@ -211,33 +208,33 @@ void SpeechSynthesis::boundaryEventOccurred(PassRefPtr<PlatformSpeechSynthesisUt
 void SpeechSynthesis::didStartSpeaking(PassRefPtr<PlatformSpeechSynthesisUtterance> utterance)
 {
     if (utterance->client())
-        fireEvent(eventNames().startEvent, static_cast<SpeechSynthesisUtterance*>(utterance->client()), 0, String());
+        fireEvent(eventNames().startEvent, static_cast<SpeechSynthesisUtterance&>(*utterance->client()), 0, String());
 }
     
 void SpeechSynthesis::didPauseSpeaking(PassRefPtr<PlatformSpeechSynthesisUtterance> utterance)
 {
     m_isPaused = true;
     if (utterance->client())
-        fireEvent(eventNames().pauseEvent, static_cast<SpeechSynthesisUtterance*>(utterance->client()), 0, String());
+        fireEvent(eventNames().pauseEvent, static_cast<SpeechSynthesisUtterance&>(*utterance->client()), 0, String());
 }
 
 void SpeechSynthesis::didResumeSpeaking(PassRefPtr<PlatformSpeechSynthesisUtterance> utterance)
 {
     m_isPaused = false;
     if (utterance->client())
-        fireEvent(eventNames().resumeEvent, static_cast<SpeechSynthesisUtterance*>(utterance->client()), 0, String());
+        fireEvent(eventNames().resumeEvent, static_cast<SpeechSynthesisUtterance&>(*utterance->client()), 0, String());
 }
 
 void SpeechSynthesis::didFinishSpeaking(PassRefPtr<PlatformSpeechSynthesisUtterance> utterance)
 {
     if (utterance->client())
-        handleSpeakingCompleted(static_cast<SpeechSynthesisUtterance*>(utterance->client()), false);
+        handleSpeakingCompleted(static_cast<SpeechSynthesisUtterance&>(*utterance->client()), false);
 }
     
 void SpeechSynthesis::speakingErrorOccurred(PassRefPtr<PlatformSpeechSynthesisUtterance> utterance)
 {
     if (utterance->client())
-        handleSpeakingCompleted(static_cast<SpeechSynthesisUtterance*>(utterance->client()), true);
+        handleSpeakingCompleted(static_cast<SpeechSynthesisUtterance&>(*utterance->client()), true);
 }
 
 } // namespace WebCore
index 29485ed..689eb43 100644 (file)
@@ -51,7 +51,7 @@ public:
     bool speaking() const;
     bool paused() const;
     
-    void speak(SpeechSynthesisUtterance*);
+    void speak(SpeechSynthesisUtterance&);
     void cancel();
     void pause();
     void resume();
@@ -73,9 +73,9 @@ private:
     void speakingErrorOccurred(PassRefPtr<PlatformSpeechSynthesisUtterance>) override;
     void boundaryEventOccurred(PassRefPtr<PlatformSpeechSynthesisUtterance>, SpeechBoundary, unsigned charIndex) override;
 
-    void startSpeakingImmediately(SpeechSynthesisUtterance*);
-    void handleSpeakingCompleted(SpeechSynthesisUtterance*, bool errorOccurred);
-    void fireEvent(const AtomicString& type, SpeechSynthesisUtterance*, unsigned long charIndex, const String& name);
+    void startSpeakingImmediately(SpeechSynthesisUtterance&);
+    void handleSpeakingCompleted(SpeechSynthesisUtterance&, bool errorOccurred);
+    void fireEvent(const AtomicString& type, SpeechSynthesisUtterance&, unsigned long charIndex, const String& name);
     
 #if PLATFORM(IOS)
     // Restrictions to change default behaviors.
@@ -91,7 +91,7 @@ private:
     std::unique_ptr<PlatformSpeechSynthesizer> m_platformSpeechSynthesizer;
     Vector<RefPtr<SpeechSynthesisVoice>> m_voiceList;
     SpeechSynthesisUtterance* m_currentSpeechUtterance;
-    Deque<RefPtr<SpeechSynthesisUtterance>> m_utteranceQueue;
+    Deque<Ref<SpeechSynthesisUtterance>> m_utteranceQueue;
     bool m_isPaused;
 #if PLATFORM(IOS)
     BehaviorRestrictions m_restrictions;
index 1f648d7..025003e 100644 (file)
@@ -24,9 +24,8 @@
  */
  
 [
-    NoInterfaceObject,
     Conditional=SPEECH_SYNTHESIS,
-    UsePointersEvenForNonNullableObjectArguments,
+    NoInterfaceObject,
 ] interface SpeechSynthesis  {
     readonly attribute boolean pending;
     readonly attribute boolean speaking;