Bug 118847 - Crash in WebCore::createMarkup()
Summary: Crash in WebCore::createMarkup()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on: 111228
Blocks:
  Show dependency treegraph
 
Reported: 2013-07-18 06:07 PDT by Antoine Quint
Modified: 2013-07-18 11:57 PDT (History)
6 users (show)

See Also:


Attachments
Patch (1.59 KB, patch)
2013-07-18 06:10 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch (1.63 KB, patch)
2013-07-18 06:21 PDT, Antoine Quint
rniwa: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion (965.02 KB, application/zip)
2013-07-18 08:11 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2013-07-18 06:07:51 PDT
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.
Comment 1 Radar WebKit Bug Importer 2013-07-18 06:08:06 PDT
<rdar://problem/14480588>
Comment 2 Antoine Quint 2013-07-18 06:10:14 PDT
Created attachment 206985 [details]
Patch
Comment 3 Antoine Quint 2013-07-18 06:21:23 PDT
Created attachment 206986 [details]
Patch
Comment 4 Build Bot 2013-07-18 08:11:34 PDT
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
Comment 5 Build Bot 2013-07-18 08:11:36 PDT
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
Comment 6 Antoine Quint 2013-07-18 09:14:16 PDT
I don't think the crash failure in this media test is related to this patch.
Comment 7 Sam Weinig 2013-07-18 10:39:53 PDT
Can we add a new test case for this?
Comment 8 Ryosuke Niwa 2013-07-18 11:19:35 PDT
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.
Comment 9 Antoine Quint 2013-07-18 11:53:31 PDT
(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.
Comment 10 Antoine Quint 2013-07-18 11:57:59 PDT
http://trac.webkit.org/changeset/152858