Bug 118847 – Crash in WebCore::createMarkup()

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Antoine Quint]

We're getting reports of a crash in WebCore::createMarkup(). This is due to code introduced in http://trac.webkit.org/changeset/144995

Index: Source/WebCore/editing/markup.cpp
===================================================================
--- Source/WebCore/editing/markup.cpp	(revision 144994)
+++ Source/WebCore/editing/markup.cpp	(revision 144995)
...
@@ -522,7 +523,7 @@
 
     Node* checkAncestor = specialCommonAncestor ? specialCommonAncestor : commonAncestor;
     if (checkAncestor->renderer()) {
-        Node* newSpecialCommonAncestor = highestEnclosingNodeOfType(firstPositionInNode(checkAncestor), &isElementPresentational);
+        Node* newSpecialCommonAncestor = highestEnclosingNodeOfType(firstPositionInNode(checkAncestor), &isElementPresentational, CanCrossEditingBoundary, checkAncestor->renderer()->containingBlock()->node());
         if (newSpecialCommonAncestor)
             specialCommonAncestor = newSpecialCommonAncestor;
     }

The containingBlock() call in the last argument of the new method signature can potentially be null and thus calling node() on it may result in a crash.

[Radar WebKit Bug Importer]

<rdar://problem/14480588>

[Antoine Quint]

Created attachment 206985 [details]
Patch

[Antoine Quint]

Created attachment 206986 [details]
Patch

[Build Bot]

Comment on attachment 206986 [details]
Patch

Attachment 206986 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1106584

New failing tests:
media/video-zoom.html

[Build Bot]

Created attachment 206991 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.3

[Antoine Quint]

I don't think the crash failure in this media test is related to this patch.

[Sam Weinig]

Can we add a new test case for this?

[Ryosuke Niwa]

Comment on attachment 206986 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=206986&action=review

I can't come up with a test case for this.

> Source/WebCore/ChangeLog:9
> +        Reviewed by NOBODY (OOPS!).

This line should appear before the long description but after the bug URL.

[Antoine Quint]

(In reply to comment #8)
> (From update of attachment 206986 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=206986&action=review
> 
> I can't come up with a test case for this.

Yeah, me neither (should have put that upfront in the comments).

> > Source/WebCore/ChangeLog:9
> > +        Reviewed by NOBODY (OOPS!).
> 
> This line should appear before the long description but after the bug URL.

Will clean up as I land.

[Antoine Quint]

http://trac.webkit.org/changeset/152858