Crash in CompositeEditCommand::ensureComposition

https://bugs.webkit.org/show_bug.cgi?id=76207

Reviewed by Chang Shu.

Source/WebCore:

The crash was caused by TypingCommand not kept alive when new editing commands are executed
during adding more typings to the open last typing command since m_lastEditCommand is replaced
by the new command. Fixed the bug by keeping them alive a little longer with RefPtr.

Test: editing/execCommand/editing-command-while-executing-typing-command-crash.html

* editing/FrameSelection.cpp:
(WebCore::shouldStopBlinkingDueToTypingCommand):
(WebCore::FrameSelection::updateAppearance):
* editing/TypingCommand.cpp:
(WebCore::TypingCommand::deleteSelection):
(WebCore::TypingCommand::deleteKeyPressed):
(WebCore::TypingCommand::forwardDeleteKeyPressed):
(WebCore::TypingCommand::insertText):
(WebCore::TypingCommand::insertLineBreak):
(WebCore::TypingCommand::insertParagraphSeparatorInQuotedContent):
(WebCore::TypingCommand::insertParagraphSeparator):
(WebCore::TypingCommand::lastTypingCommandIfStillOpenForTyping):
(WebCore::TypingCommand::closeTyping):
* editing/TypingCommand.h:

LayoutTests:

Add a regression test.

* editing/execCommand/editing-command-while-executing-typing-command-crash-expected.txt: Added.
* editing/execCommand/editing-command-while-executing-typing-command-crash.html: Added.

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 70abcdecb0096b3e4dd45d679c5246e52166824d..8ff4b55ecd2670e2cb09d418456b2a47bff24a62 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2012-01-13  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Crash in CompositeEditCommand::ensureComposition
+        https://bugs.webkit.org/show_bug.cgi?id=76207
+
+        Reviewed by Chang Shu.
+
+        Add a regression test.
+
+        * editing/execCommand/editing-command-while-executing-typing-command-crash-expected.txt: Added.
+        * editing/execCommand/editing-command-while-executing-typing-command-crash.html: Added.
+
 2012-01-19  Eric Carlson  <eric.carlson@apple.com>
 
         https://bugs.webkit.org/show_bug.cgi?id=75192

diff --git a/LayoutTests/editing/execCommand/editing-command-while-executing-typing-command-crash-expected.txt b/LayoutTests/editing/execCommand/editing-command-while-executing-typing-command-crash-expected.txt
new file mode 100644 (file)
index 0000000..dcb9e34
--- /dev/null
+++ b/LayoutTests/editing/execCommand/editing-command-while-executing-typing-command-crash-expected.txt
@@ -0,0 +1,2 @@
+This tests executing an editing command while executing a typing command.
+PASS

diff --git a/LayoutTests/editing/execCommand/editing-command-while-executing-typing-command-crash.html b/LayoutTests/editing/execCommand/editing-command-while-executing-typing-command-crash.html
new file mode 100644 (file)
index 0000000..ada1db0
--- /dev/null
+++ b/LayoutTests/editing/execCommand/editing-command-while-executing-typing-command-crash.html
@@ -0,0 +1,18 @@
+<script>
+window.onload = function() {
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+
+    document.execCommand("selectall", false);
+    document.designMode="on";
+    document.execCommand("insertparagraph", false);
+    document.execCommand("InsertText", false);
+
+    document.firstChild.appendChild(document.createElement('body'));
+    document.body.innerText = "This tests executing an editing command while executing a typing command.\nPASS";
+};
+
+document.addEventListener("DOMNodeRemovedFromDocument",
+    function() { document.execCommand("JustifyNone", false); },true);
+
+</script>

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 6b650657cf78f72f2c5fb6cc9f89a217768ba1e7..e2e0aea5a6d66335c93b469ff2186c3a6cc0de8c 100755 (executable)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,31 @@
+2012-01-13  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Crash in CompositeEditCommand::ensureComposition
+        https://bugs.webkit.org/show_bug.cgi?id=76207
+
+        Reviewed by Chang Shu.
+
+        The crash was caused by TypingCommand not kept alive when new editing commands are executed
+        during adding more typings to the open last typing command since m_lastEditCommand is replaced
+        by the new command. Fixed the bug by keeping them alive a little longer with RefPtr.
+
+        Test: editing/execCommand/editing-command-while-executing-typing-command-crash.html
+
+        * editing/FrameSelection.cpp:
+        (WebCore::shouldStopBlinkingDueToTypingCommand):
+        (WebCore::FrameSelection::updateAppearance):
+        * editing/TypingCommand.cpp:
+        (WebCore::TypingCommand::deleteSelection):
+        (WebCore::TypingCommand::deleteKeyPressed):
+        (WebCore::TypingCommand::forwardDeleteKeyPressed):
+        (WebCore::TypingCommand::insertText):
+        (WebCore::TypingCommand::insertLineBreak):
+        (WebCore::TypingCommand::insertParagraphSeparatorInQuotedContent):
+        (WebCore::TypingCommand::insertParagraphSeparator):
+        (WebCore::TypingCommand::lastTypingCommandIfStillOpenForTyping):
+        (WebCore::TypingCommand::closeTyping):
+        * editing/TypingCommand.h:
+
 2012-01-19  Andreas Kling  <awesomekling@apple.com>
 
         Unreviewed debug build fix.

diff --git a/Source/WebCore/editing/FrameSelection.cpp b/Source/WebCore/editing/FrameSelection.cpp
index d1267eaedfe2f8d0ffcdedef96b0761fda524aff..3f2e45f9f26aed74cf7335a461abdcac84fdfb9c 100644 (file)
--- a/Source/WebCore/editing/FrameSelection.cpp
+++ b/Source/WebCore/editing/FrameSelection.cpp
@@ -1654,6 +1654,11 @@ bool FrameSelection::isFocusedAndActive() const
     return m_focused && m_frame->page() && m_frame->page()->focusController()->isActive();
 }
 
+inline static bool shouldStopBlinkingDueToTypingCommand(Frame* frame)
+{
+    return frame->editor()->lastEditCommand() && frame->editor()->lastEditCommand()->shouldStopCaretBlinking();
+}
+
 void FrameSelection::updateAppearance()
 {
 #if ENABLE(TEXT_CARET)
@@ -1661,13 +1666,10 @@ void FrameSelection::updateAppearance()
 
     bool caretBrowsing = m_frame->settings() && m_frame->settings()->caretBrowsingEnabled();
     bool shouldBlink = caretIsVisible() && isCaret() && (isContentEditable() || caretBrowsing);
-    
-    CompositeEditCommand* lastEditCommand = m_frame ? m_frame->editor()->lastEditCommand() : 0;
-    bool shouldStopBlinkingDueToTypingCommand = lastEditCommand && lastEditCommand->shouldStopCaretBlinking();
 
     // If the caret moved, stop the blink timer so we can restart with a
     // black caret in the new location.
-    if (caretRectChanged || !shouldBlink || shouldStopBlinkingDueToTypingCommand)
+    if (caretRectChanged || !shouldBlink || shouldStopBlinkingDueToTypingCommand(m_frame))
         m_caretBlinkTimer.stop();
 
     // Start blinking with a black caret. Be sure not to restart if we're

diff --git a/Source/WebCore/editing/TypingCommand.cpp b/Source/WebCore/editing/TypingCommand.cpp
index 1ee003327cde69007fd9f4b9350f9e6bd160f555..4101ecfd70f78d625474062222d95fdc82509361 100644 (file)
--- a/Source/WebCore/editing/TypingCommand.cpp
+++ b/Source/WebCore/editing/TypingCommand.cpp
@@ -87,7 +87,7 @@ void TypingCommand::deleteSelection(Document* document, Options options)
     if (!frame->selection()->isRange())
         return;
 
-    if (TypingCommand* lastTypingCommand = lastTypingCommandIfStillOpenForTyping(frame)) {
+    if (RefPtr<TypingCommand> lastTypingCommand = lastTypingCommandIfStillOpenForTyping(frame)) {
         lastTypingCommand->setShouldPreventSpellChecking(options & PreventSpellChecking);
         lastTypingCommand->deleteSelection(options & SmartDelete);
         return;
@@ -100,8 +100,8 @@ void TypingCommand::deleteKeyPressed(Document *document, Options options, TextGr
 {
     ASSERT(document);
     if (granularity == CharacterGranularity) {
-        if (TypingCommand* lastTypingCommand = lastTypingCommandIfStillOpenForTyping(document->frame())) {
-            updateSelectionIfDifferentFromCurrentSelection(lastTypingCommand, document->frame());
+        if (RefPtr<TypingCommand> lastTypingCommand = lastTypingCommandIfStillOpenForTyping(document->frame())) {
+            updateSelectionIfDifferentFromCurrentSelection(lastTypingCommand.get(), document->frame());
             lastTypingCommand->setShouldPreventSpellChecking(options & PreventSpellChecking);
             lastTypingCommand->deleteKeyPressed(granularity, options & KillRing);
             return;
@@ -117,8 +117,8 @@ void TypingCommand::forwardDeleteKeyPressed(Document *document, Options options,
     ASSERT(document);
     Frame* frame = document->frame();
     if (granularity == CharacterGranularity) {
-        if (TypingCommand* lastTypingCommand = lastTypingCommandIfStillOpenForTyping(frame)) {
-            updateSelectionIfDifferentFromCurrentSelection(lastTypingCommand, frame);
+        if (RefPtr<TypingCommand> lastTypingCommand = lastTypingCommandIfStillOpenForTyping(frame)) {
+            updateSelectionIfDifferentFromCurrentSelection(lastTypingCommand.get(), frame);
             lastTypingCommand->setShouldPreventSpellChecking(options & PreventSpellChecking);
             lastTypingCommand->forwardDeleteKeyPressed(granularity, options & KillRing);
             return;
@@ -176,7 +176,7 @@ void TypingCommand::insertText(Document* document, const String& text, const Vis
     // Set the starting and ending selection appropriately if we are using a selection
     // that is different from the current selection.  In the future, we should change EditCommand
     // to deal with custom selections in a general way that can be used by all of the commands.
-    if (TypingCommand* lastTypingCommand = lastTypingCommandIfStillOpenForTyping(frame.get())) {
+    if (RefPtr<TypingCommand> lastTypingCommand = lastTypingCommandIfStillOpenForTyping(frame.get())) {
         if (lastTypingCommand->endingSelection() != selectionForInsertion) {
             lastTypingCommand->setStartingSelection(selectionForInsertion);
             lastTypingCommand->setEndingSelection(selectionForInsertion);
@@ -204,7 +204,7 @@ void TypingCommand::insertText(Document* document, const String& text, const Vis
 void TypingCommand::insertLineBreak(Document *document, Options options)
 {
     ASSERT(document);
-    if (TypingCommand* lastTypingCommand = lastTypingCommandIfStillOpenForTyping(document->frame())) {
+    if (RefPtr<TypingCommand> lastTypingCommand = lastTypingCommandIfStillOpenForTyping(document->frame())) {
         lastTypingCommand->setShouldRetainAutocorrectionIndicator(options & RetainAutocorrectionIndicator);
         lastTypingCommand->insertLineBreak();
         return;
@@ -216,7 +216,7 @@ void TypingCommand::insertLineBreak(Document *document, Options options)
 void TypingCommand::insertParagraphSeparatorInQuotedContent(Document *document)
 {
     ASSERT(document);
-    if (TypingCommand* lastTypingCommand = lastTypingCommandIfStillOpenForTyping(document->frame())) {
+    if (RefPtr<TypingCommand> lastTypingCommand = lastTypingCommandIfStillOpenForTyping(document->frame())) {
         lastTypingCommand->insertParagraphSeparatorInQuotedContent();
         return;
     }
@@ -227,7 +227,7 @@ void TypingCommand::insertParagraphSeparatorInQuotedContent(Document *document)
 void TypingCommand::insertParagraphSeparator(Document *document, Options options)
 {
     ASSERT(document);
-    if (TypingCommand* lastTypingCommand = lastTypingCommandIfStillOpenForTyping(document->frame())) {
+    if (RefPtr<TypingCommand> lastTypingCommand = lastTypingCommandIfStillOpenForTyping(document->frame())) {
         lastTypingCommand->setShouldRetainAutocorrectionIndicator(options & RetainAutocorrectionIndicator);
         lastTypingCommand->insertParagraphSeparator();
         return;
@@ -236,20 +236,20 @@ void TypingCommand::insertParagraphSeparator(Document *document, Options options
     applyCommand(TypingCommand::create(document, InsertParagraphSeparator, "", options));
 }
 
-TypingCommand* TypingCommand::lastTypingCommandIfStillOpenForTyping(Frame* frame)
+PassRefPtr<TypingCommand> TypingCommand::lastTypingCommandIfStillOpenForTyping(Frame* frame)
 {
     ASSERT(frame);
 
-    CompositeEditCommand* lastEditCommand = frame->editor()->lastEditCommand();
-    if (!lastEditCommand || !lastEditCommand->isTypingCommand() || !static_cast<TypingCommand*>(lastEditCommand)->isOpenForMoreTyping())
+    RefPtr<CompositeEditCommand> lastEditCommand = frame->editor()->lastEditCommand();
+    if (!lastEditCommand || !lastEditCommand->isTypingCommand() || !static_cast<TypingCommand*>(lastEditCommand.get())->isOpenForMoreTyping())
         return 0;
 
-    return static_cast<TypingCommand*>(lastEditCommand);
+    return static_cast<TypingCommand*>(lastEditCommand.get());
 }
 
 void TypingCommand::closeTyping(Frame* frame)
 {
-    if (TypingCommand* lastTypingCommand = lastTypingCommandIfStillOpenForTyping(frame))
+    if (RefPtr<TypingCommand> lastTypingCommand = lastTypingCommandIfStillOpenForTyping(frame))
         lastTypingCommand->closeTyping();
 }
 

diff --git a/Source/WebCore/editing/TypingCommand.h b/Source/WebCore/editing/TypingCommand.h
index cce9cdb61b2689283f67f8a1b4ff655ac0d34b36..1f8c181dfcebb259ce8d666c7cbd6854d644526a 100644 (file)
--- a/Source/WebCore/editing/TypingCommand.h
+++ b/Source/WebCore/editing/TypingCommand.h
@@ -95,7 +95,7 @@ private:
     bool isOpenForMoreTyping() const { return m_openForMoreTyping; }
     void closeTyping() { m_openForMoreTyping = false; }
 
-    static TypingCommand* lastTypingCommandIfStillOpenForTyping(Frame*);
+    static PassRefPtr<TypingCommand> lastTypingCommandIfStillOpenForTyping(Frame*);
 
     virtual void doApply();
     virtual EditAction editingAction() const;