Attachment 233686 Details for Bug 89401 – patch

patch

patch (text/plain), 6.90 KB, created by chris fleizach on 2014-06-23 23:06:57 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: chris fleizach

Created: 2014-06-23 23:06:57 PDT

Size: 6.90 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 170332)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,22 @@
>+2014-06-23  Chris Fleizach  <cfleizach@apple.com>
>+
>+        AX: Crash at com.apple.WebCore: WebCore::AlternativeTextController::removeDictationAlternativesForMarker + 43
>+        https://bugs.webkit.org/show_bug.cgi?id=134226
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Unchecked access of details pointer in the Document marker led to the crash.
>+        It seems erroneous to allow creation of a DocumentMarker without details when one is expected, because it
>+        carries the dictation context (not just the description).
>+        Also, add in ASSERTs to catch scenarios where a nullptr might be passed in.
>+
>+        Test: platform/mac/editing/input/crash-for-empty-text-alternative.html
>+
>+        * dom/DocumentMarker.cpp:
>+        (WebCore::DocumentMarker::DocumentMarker):
>+        * editing/AlternativeTextController.cpp:
>+        (WebCore::AlternativeTextController::removeDictationAlternativesForMarker):
>+
> 2014-06-23  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r170323.
>Index: Source/WebCore/editing/AlternativeTextController.cpp
>===================================================================
>--- Source/WebCore/editing/AlternativeTextController.cpp	(revision 170160)
>+++ Source/WebCore/editing/AlternativeTextController.cpp	(working copy)
>@@ -702,9 +702,11 @@
> void AlternativeTextController::removeDictationAlternativesForMarker(const DocumentMarker* marker)
> {
> #if USE(DICTATION_ALTERNATIVES)
>-    DictationMarkerDetails* details = static_cast<DictationMarkerDetails*>(marker->details());
>-    if (AlternativeTextClient* client = alternativeTextClient())
>-        client->removeDictationAlternatives(details->dictationContext());
>+    ASSERT(marker->details());
>+    if (DictationMarkerDetails* details = static_cast<DictationMarkerDetails*>(marker->details())) {
>+        if (AlternativeTextClient* client = alternativeTextClient())
>+            client->removeDictationAlternatives(details->dictationContext());
>+    }
> #else
>     UNUSED_PARAM(marker);
> #endif
>Index: Source/WebCore/dom/DocumentMarker.cpp
>===================================================================
>--- Source/WebCore/dom/DocumentMarker.cpp	(revision 170160)
>+++ Source/WebCore/dom/DocumentMarker.cpp	(working copy)
>@@ -111,8 +111,9 @@
>     : m_type(type)
>     , m_startOffset(startOffset)
>     , m_endOffset(endOffset)
>-    , m_details(description.isEmpty() ? 0 : DocumentMarkerDescription::create(description))
>+    , m_details(DocumentMarkerDescription::create(description))
> {
>+    ASSERT(m_details);
> }
> 
> DocumentMarker::DocumentMarker(unsigned startOffset, unsigned endOffset, bool activeMatch)
>@@ -121,6 +122,7 @@
>     , m_endOffset(endOffset)
>     , m_details(DocumentMarkerTextMatch::instanceFor(activeMatch))
> {
>+    ASSERT(m_details);
> }
> 
> DocumentMarker::DocumentMarker(MarkerType type, unsigned startOffset, unsigned endOffset, PassRefPtr<DocumentMarkerDetails> details)
>@@ -129,6 +131,7 @@
>     , m_endOffset(endOffset)
>     , m_details(details)
> {
>+    ASSERT(m_details);
> }
> 
> void DocumentMarker::shiftOffsets(int delta)
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 170332)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,15 @@
>+2014-06-23  Chris Fleizach  <cfleizach@apple.com>
>+
>+        AX: Crash at com.apple.WebCore: WebCore::AlternativeTextController::removeDictationAlternativesForMarker + 43
>+        https://bugs.webkit.org/show_bug.cgi?id=134226
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * platform/mac-wk2/TestExpectations
>+            Skip test on wk2 because dictation related tests are not yet supported (89401)
>+        * platform/mac/editing/input/crash-for-empty-text-alternative-expected.txt: Added.
>+        * platform/mac/editing/input/crash-for-empty-text-alternative.html: Added.
>+
> 2014-06-19  Jeffrey Pfau  <jpfau@apple.com>
> 
>         Database process crashes when multiple transactions attempt to run at once
>Index: LayoutTests/platform/mac/editing/input/crash-for-empty-text-alternative-expected.txt
>===================================================================
>--- LayoutTests/platform/mac/editing/input/crash-for-empty-text-alternative-expected.txt	(revision 0)
>+++ LayoutTests/platform/mac/editing/input/crash-for-empty-text-alternative-expected.txt	(working copy)
>@@ -0,0 +1,6 @@
>+Ensure that a empty text alternative does not lead to a crash
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+tisatest
>+TEST COMPLETE
>Index: LayoutTests/platform/mac/editing/input/crash-for-empty-text-alternative.html
>===================================================================
>--- LayoutTests/platform/mac/editing/input/crash-for-empty-text-alternative.html	(revision 0)
>+++ LayoutTests/platform/mac/editing/input/crash-for-empty-text-alternative.html	(working copy)
>@@ -0,0 +1,47 @@
>+<html>
>+<head>
>+<script src="../../../../editing/editing.js" language="JavaScript" type="text/JavaScript" ></script>
>+<script src="../../../../resources/js-test-pre.js"></script>
>+
>+<style>
>+body { margin: 0; padding: 0 }
>+</style>
>+</head>
>+<body>
>+<pre id="description"></pre>
>+<div id="test" contenteditable>thisisatest</div>
>+<p id="console"></p>
>+<script type="text/javascript">
>+    description("Ensure that a empty text alternative does not lead to a crash");
>+
>+    
>+    if (window.testRunner) {
>+        
>+        try {
>+
>+            document.getElementById("test").focus();
>+
>+            moveSelectionForwardByCharacterCommand();
>+            moveSelectionForwardByCharacterCommand();
>+
>+            var dictatedString = textInputController.makeDictatedString(" ", " ", 0, 1);
>+            textInputController.insertText(dictatedString);
>+
>+            moveSelectionForwardByCharacterCommand();
>+            moveSelectionForwardByCharacterCommand();
>+
>+            for ( var k = 0; k < 4; k++) {
>+                textInputController.doCommand("deleteBackward:");
>+            }
>+
>+            document.write("TEST COMPLETE");
>+
>+        } catch (ex) {
>+            document.write("Exception: " + ex.description);
>+        }
>+    } else {
>+        document.write("(cannot run interactively)");
>+    }
>+</script>
>+</body>
>+</html>
>Index: LayoutTests/platform/mac-wk2/TestExpectations
>===================================================================
>--- LayoutTests/platform/mac-wk2/TestExpectations	(revision 170160)
>+++ LayoutTests/platform/mac-wk2/TestExpectations	(working copy)
>@@ -184,6 +184,7 @@
> # Missing functionality of textInputController
> # https://bugs.webkit.org/show_bug.cgi?id=89401
> platform/mac/editing/input/insert-dictated-text.html
>+platform/mac/editing/input/crash-for-empty-text-alternative.html
> platform/mac/editing/input/edit-dictated-text-with-alternative.html
> platform/mac/editing/attributed-string/
>

Actions: View

Attachments on bug 89401: 233686