2011-01-12 Satish Sampath <satish@chromium.org>
authorsatish@chromium.org <satish@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Jan 2011 20:39:09 +0000 (20:39 +0000)
committersatish@chromium.org <satish@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Jan 2011 20:39:09 +0000 (20:39 +0000)
        Reviewed by Dimitri Glazkov.

        Fix a crash when accessing speech input from script.
        https://bugs.webkit.org/show_bug.cgi?id=52325

        Added a layout test to check enabling/disabling speech input from script.

        * fast/speech/speech-input-scripting-expected.txt: Added.
        * fast/speech/speech-input-scripting.html: Added.
2011-01-12  Satish Sampath  <satish@chromium.org>

        Reviewed by Dimitri Glazkov.

        Fix a crash when accessing speech input from script.
        https://bugs.webkit.org/show_bug.cgi?id=52325

        Test: fast/speech/speech-input-scripting.html

        * html/HTMLInputElement.cpp:
        (WebCore::HTMLInputElement::parseMappedAttribute): Recreate renderer when speech input is enabled/disabled.
        * rendering/RenderTextControlSingleLine.cpp: Remove unused code.
        * rendering/RenderTextControlSingleLine.h:
        * rendering/TextControlInnerElements.cpp: Take self references before firing events and check for renderer validity after.
        (WebCore::InputFieldSpeechButtonElement::defaultEventHandler):
        (WebCore::InputFieldSpeechButtonElement::setRecognitionResult):
        (WebCore::InputFieldSpeechButtonElement::detach):

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

LayoutTests/ChangeLog
LayoutTests/fast/speech/speech-input-scripting-expected.txt [new file with mode: 0644]
LayoutTests/fast/speech/speech-input-scripting.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLInputElement.cpp
Source/WebCore/platform/mock/SpeechInputClientMock.cpp
Source/WebCore/rendering/RenderTextControlSingleLine.cpp
Source/WebCore/rendering/RenderTextControlSingleLine.h
Source/WebCore/rendering/TextControlInnerElements.cpp

index 8b507c2..a1a2dc4 100644 (file)
@@ -1,3 +1,15 @@
+2011-01-12  Satish Sampath  <satish@chromium.org>
+
+        Reviewed by Dimitri Glazkov.
+
+        Fix a crash when accessing speech input from script.
+        https://bugs.webkit.org/show_bug.cgi?id=52325
+
+        Added a layout test to check enabling/disabling speech input from script.
+
+        * fast/speech/speech-input-scripting-expected.txt: Added.
+        * fast/speech/speech-input-scripting.html: Added.
+
 2011-01-14  Abhishek Arya  <inferno@chromium.org>
 
         Reviewed by David Hyatt.
diff --git a/LayoutTests/fast/speech/speech-input-scripting-expected.txt b/LayoutTests/fast/speech/speech-input-scripting-expected.txt
new file mode 100644 (file)
index 0000000..428d359
--- /dev/null
@@ -0,0 +1,13 @@
+Tests for enabling and disabling speech input via script.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS document.getElementById("speechInput").value is "Pictures of the moon"
+PASS document.getElementById("speechInput").value is "Pictures of the moon"
+PASS document.getElementById("speechInput").value is "Pictures of the moon"
+PASS document.getElementById("speechInput").value is "Pictures of the moon"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+   
diff --git a/LayoutTests/fast/speech/speech-input-scripting.html b/LayoutTests/fast/speech/speech-input-scripting.html
new file mode 100644 (file)
index 0000000..dd887a1
--- /dev/null
@@ -0,0 +1,79 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="../js/resources/js-test-style.css">
+<script src="../js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script type="text/javascript">
+description('Tests for enabling and disabling speech input via script.');
+
+var speechInputCreatedByScript = false;
+
+function onWebkitSpeechChange() {
+    shouldBeEqualToString('document.getElementById("speechInput").value', 'Pictures of the moon');
+
+    // Disable speech input now, just to verify this doesn't result in any crashes.
+    var input = document.getElementById('speechInput');
+    input.removeAttribute('x-webkit-speech');
+
+    // If the test has only been done with a type='text' field, repeat the same with a
+    // type='search' field since that takes a slightly different code path.
+    if (input.type != 'search') {
+        input.type = 'search';
+        testSetAttributeAndClick();
+        return;
+    }
+
+    // If the test has only been done with a statically declared element, now repeat the same
+    // with a dynamically created/inserted element.
+    if (!speechInputCreatedByScript) {
+        document.body.removeChild(input);
+        input = document.createElement('input');
+        input.id = 'speechInput';
+        document.body.appendChild(input);
+        speechInputCreatedByScript = true;
+        testSetAttributeAndClick();
+        return;
+    }
+
+    finishJSTest();
+}
+
+function testSetAttributeAndClick() {
+    // Enable speech input and test that clicking the speech button fills in mock speech-recognized text.
+    var input = document.getElementById('speechInput');
+    input.setAttribute('x-webkit-speech', '');
+    input.addEventListener('webkitspeechchange', onWebkitSpeechChange);
+
+    var x = input.offsetLeft + input.offsetWidth - 8;
+    var y = input.offsetTop + input.offsetHeight / 2;
+    eventSender.mouseMoveTo(x, y);
+    eventSender.mouseDown();
+    eventSender.mouseUp();
+}
+
+function run() {
+    if (!window.layoutTestController || !window.eventSender)
+        return;
+
+    layoutTestController.addMockSpeechInputResult('Pictures of the moon', 1.0, '');
+
+    // Try disabling speech with an input tag which has the attribute set in HTML and
+    // verify that doesn't result in any crashes.
+    document.getElementById('inputWithAttribute').removeAttribute('x-webkit-speech');
+
+    testSetAttributeAndClick();
+}
+
+window.onload = run;
+window.jsTestIsAsync = true;
+window.successfullyParsed = true;
+</script>
+<script src="../js/resources/js-test-post.js"></script>
+<input id='speechInput'>
+<input id='inputWithAttribute' x-webkit-speech>
+</body>
+</html>
index dbe5b0a..440990e 100644 (file)
@@ -1,3 +1,21 @@
+2011-01-12  Satish Sampath  <satish@chromium.org>
+
+        Reviewed by Dimitri Glazkov.
+
+        Fix a crash when accessing speech input from script.
+        https://bugs.webkit.org/show_bug.cgi?id=52325
+
+        Test: fast/speech/speech-input-scripting.html
+
+        * html/HTMLInputElement.cpp:
+        (WebCore::HTMLInputElement::parseMappedAttribute): Recreate renderer when speech input is enabled/disabled.
+        * rendering/RenderTextControlSingleLine.cpp: Remove unused code.
+        * rendering/RenderTextControlSingleLine.h:
+        * rendering/TextControlInnerElements.cpp: Take self references before firing events and check for renderer validity after.
+        (WebCore::InputFieldSpeechButtonElement::defaultEventHandler):
+        (WebCore::InputFieldSpeechButtonElement::setRecognitionResult):
+        (WebCore::InputFieldSpeechButtonElement::detach):
+
 2011-01-14  Abhishek Arya  <inferno@chromium.org>
 
         Reviewed by David Hyatt.
index 7d2a3c5..a1e7d8b 100644 (file)
@@ -631,9 +631,14 @@ void HTMLInputElement::parseMappedAttribute(Attribute* attr)
 #endif
 #if ENABLE(INPUT_SPEECH)
     else if (attr->name() == webkitspeechAttr) {
-      if (renderer())
-          toRenderTextControlSingleLine(renderer())->speechAttributeChanged();
-      setNeedsStyleRecalc();
+        if (renderer()) {
+            // This renderer and its children have quite different layouts and styles depending on
+            // whether the speech button is visible or not. So we reset the whole thing and recreate
+            // to get the right styles and layout.
+            detach();
+            attach();
+        }
+        setNeedsStyleRecalc();
     } else if (attr->name() == onwebkitspeechchangeAttr)
         setAttributeEventListener(eventNames().webkitspeechchangeEvent, createAttributeEventListener(this, attr));
 #endif
index 16f2825..16a7c76 100644 (file)
@@ -72,8 +72,8 @@ void SpeechInputClientMock::stopRecording(int requestId)
 
 void SpeechInputClientMock::cancelRecognition(int requestId)
 {
-    ASSERT(requestId == m_requestId);
     if (m_timer.isActive()) {
+        ASSERT(requestId == m_requestId);
         m_timer.stop();
         m_recording = false;
         m_listener->didCompleteRecognition(m_requestId);
@@ -107,15 +107,20 @@ void SpeechInputClientMock::timerFired(WebCore::Timer<SpeechInputClientMock>*)
     } else {
         bool noResultsFound = false;
 
+        // We take a copy of the requestId here so that if scripts destroyed the input element
+        // inside one of the callbacks below, we'll still know what this session's requestId was.
+        int requestId = m_requestId;
+        m_requestId = 0;
+
         // Empty language case must be handled separately to avoid problems with HashMap and empty keys.
         if (m_language.isEmpty()) {
             if (!m_resultsForEmptyLanguage.isEmpty())
-                m_listener->setRecognitionResult(m_requestId, m_resultsForEmptyLanguage);
+                m_listener->setRecognitionResult(requestId, m_resultsForEmptyLanguage);
             else
                 noResultsFound = true;
         } else {
             if (m_recognitionResults.contains(m_language))
-                m_listener->setRecognitionResult(m_requestId, m_recognitionResults.get(m_language));
+                m_listener->setRecognitionResult(requestId, m_recognitionResults.get(m_language));
             else
                 noResultsFound = true;
         }
@@ -128,11 +133,10 @@ void SpeechInputClientMock::timerFired(WebCore::Timer<SpeechInputClientMock>*)
             error.append("'");
             SpeechInputResultArray results;
             results.append(SpeechInputResult::create(error, 1.0));
-            m_listener->setRecognitionResult(m_requestId, results);
+            m_listener->setRecognitionResult(requestId, results);
         }
 
-        m_listener->didCompleteRecognition(m_requestId);
-        m_requestId = 0;
+        m_listener->didCompleteRecognition(requestId);
     }
 }
 
index c815be1..c850321 100644 (file)
@@ -667,21 +667,6 @@ void RenderTextControlSingleLine::createSubtreeIfNeeded()
     }
 }
 
-#if ENABLE(INPUT_SPEECH)
-void RenderTextControlSingleLine::speechAttributeChanged()
-{
-    // The inner text element of this renderer has different styles depending on whether the
-    // speech button is visible or not. So when the speech attribute changes, we reset the
-    // whole thing and recreate to get the right styles and layout.
-    if (m_speechButton)
-        m_speechButton->detach();
-    setChildrenInline(true);
-    RenderStyle* parentStyle = m_innerBlock ? m_innerBlock->renderer()->style() : style();
-    setStyle(createInnerTextStyle(parentStyle));
-    updateFromElement();
-}
-#endif
-
 void RenderTextControlSingleLine::updateFromElement()
 {
     createSubtreeIfNeeded();
index 51509c0..16ce1e4 100644 (file)
@@ -59,10 +59,6 @@ public:
     // Decoration width outside of the text field.
     int decorationWidthRight() const;
 
-#if ENABLE(INPUT_SPEECH)
-    void speechAttributeChanged();
-#endif
-
 private:
     int preferredDecorationWidthRight() const;
     virtual bool hasControlClip() const;
index d6fc7aa..5b8712d 100644 (file)
@@ -383,7 +383,7 @@ inline InputFieldSpeechButtonElement::InputFieldSpeechButtonElement(HTMLElement*
 InputFieldSpeechButtonElement::~InputFieldSpeechButtonElement()
 {
     SpeechInput* speech = speechInput();
-    if (speech)  { // Could be null when page is unloading.
+    if (speech && m_listenerId)  { // Could be null when page is unloading.
         if (m_state != Idle)
             speech->cancelRecognition(m_listenerId);
         speech->unregisterListener(m_listenerId);
@@ -416,6 +416,7 @@ void InputFieldSpeechButtonElement::defaultEventHandler(Event* event)
         // remove the input element from DOM. To make sure it remains valid until we finish our work
         // here, we take a temporary reference.
         RefPtr<HTMLInputElement> holdRef(input);
+        RefPtr<InputFieldSpeechButtonElement> holdRefButton(this);
         input->focus();
         input->select();
         event->setDefaultHandled();
@@ -482,9 +483,14 @@ void InputFieldSpeechButtonElement::setRecognitionResult(int, const SpeechInputR
     // remove the input element from DOM. To make sure it remains valid until we finish our work
     // here, we take a temporary reference.
     RefPtr<HTMLInputElement> holdRef(input);
+    RefPtr<InputFieldSpeechButtonElement> holdRefButton(this);
     input->setValue(results.isEmpty() ? "" : results[0]->utterance());
     input->dispatchEvent(SpeechInputEvent::create(eventNames().webkitspeechchangeEvent, results));
-    renderer()->repaint();
+
+    // Check before accessing the renderer as the above event could have potentially turned off
+    // speech in the input element, hence removing this button and renderer from the hierarchy.
+    if (renderer())
+        renderer()->repaint();
 }
 
 void InputFieldSpeechButtonElement::detach()
@@ -494,8 +500,12 @@ void InputFieldSpeechButtonElement::detach()
             frame->eventHandler()->setCapturingMouseEventsNode(0);
     }
 
-    if (m_state != Idle)
-      speechInput()->cancelRecognition(m_listenerId);
+    if (m_listenerId) {
+        if (m_state != Idle)
+            speechInput()->cancelRecognition(m_listenerId);
+        speechInput()->unregisterListener(m_listenerId);
+        m_listenerId = 0;
+    }
 
     TextControlInnerElement::detach();
 }