<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[169599] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/169599">169599</a></dd>
<dt>Author</dt> <dd>benjamin@webkit.org</dd>
<dt>Date</dt> <dd>2014-06-04 15:08:44 -0700 (Wed, 04 Jun 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>Regression(<a href="http://trac.webkit.org/projects/webkit/changeset/169547">r169547</a>): Crash in WebCore::styleForFirstLetter() while loading http://thenextweb.com/apple/2014/02/21/apple-confirms-acquired-testflight-creator-burstly/
https://bugs.webkit.org/show_bug.cgi?id=133500
Reviewed by Antti Koivisto.
Source/WebCore:
Unlike the other tree marking optimization, adding HasPseudoStyle() on an element style
during style resolution must be conservative and not set the flag unless every other filter
matched first.
After <a href="http://trac.webkit.org/projects/webkit/changeset/169547">r169547</a>, the SelectorChecker was setting flags on HasPseudoStyle() during the pseudo
element matching. If that passed but the remaining filter did not match, the element style
could have the flag "FIRST_LETTER" while not being able to resolve a style for "FIRST_LETTER".
When the layout code was creating the renderer, it would find the FIRST_LETTER flag on the main style
and try to get the specific style for FIRST_LETTER. Since no rule would resolve for FIRST_LETTER,
the style would be null and the code would be crashing when accessing its parameters.
This patch fixes the issue by moving the style marking for pseudo element at the end of the matching.
Test: fast/css/first-letter-on-non-matching-selectors.html
* css/SelectorChecker.cpp:
(WebCore::SelectorChecker::match):
(WebCore::SelectorChecker::matchRecursively):
* css/SelectorChecker.h:
(WebCore::SelectorChecker::match): Deleted.
LayoutTests:
* fast/css/first-letter-on-non-matching-selectors-expected.txt: Added.
* fast/css/first-letter-on-non-matching-selectors.html: Added.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorecssSelectorCheckercpp">trunk/Source/WebCore/css/SelectorChecker.cpp</a></li>
<li><a href="#trunkSourceWebCorecssSelectorCheckerh">trunk/Source/WebCore/css/SelectorChecker.h</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastcssfirstletteronnonmatchingselectorsexpectedtxt">trunk/LayoutTests/fast/css/first-letter-on-non-matching-selectors-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastcssfirstletteronnonmatchingselectorshtml">trunk/LayoutTests/fast/css/first-letter-on-non-matching-selectors.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (169598 => 169599)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2014-06-04 21:54:46 UTC (rev 169598)
+++ trunk/LayoutTests/ChangeLog 2014-06-04 22:08:44 UTC (rev 169599)
</span><span class="lines">@@ -1,3 +1,13 @@
</span><ins>+2014-06-04 Benjamin Poulain <benjamin@webkit.org>
+
+ Regression(r169547): Crash in WebCore::styleForFirstLetter() while loading http://thenextweb.com/apple/2014/02/21/apple-confirms-acquired-testflight-creator-burstly/
+ https://bugs.webkit.org/show_bug.cgi?id=133500
+
+ Reviewed by Antti Koivisto.
+
+ * fast/css/first-letter-on-non-matching-selectors-expected.txt: Added.
+ * fast/css/first-letter-on-non-matching-selectors.html: Added.
+
</ins><span class="cx"> 2014-06-04 Oliver Hunt <oliver@apple.com>
</span><span class="cx">
</span><span class="cx"> ArrayIterator should not be exposed in Safari 8
</span></span></pre></div>
<a id="trunkLayoutTestsfastcssfirstletteronnonmatchingselectorsexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css/first-letter-on-non-matching-selectors-expected.txt (0 => 169599)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css/first-letter-on-non-matching-selectors-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/css/first-letter-on-non-matching-selectors-expected.txt 2014-06-04 22:08:44 UTC (rev 169599)
</span><span class="lines">@@ -0,0 +1,11 @@
</span><ins>+The style resolution has an optimization to detect if a selector has certain pseudo element. The optimization should only happen if the selector completely match with the exception of the pseudo element.
+
+This test pass if all the lines read "PASS".
+
+PASS
+
+PASS
+
+PASS
+
+PASS
</ins></span></pre></div>
<a id="trunkLayoutTestsfastcssfirstletteronnonmatchingselectorshtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css/first-letter-on-non-matching-selectors.html (0 => 169599)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css/first-letter-on-non-matching-selectors.html (rev 0)
+++ trunk/LayoutTests/fast/css/first-letter-on-non-matching-selectors.html 2014-06-04 22:08:44 UTC (rev 169599)
</span><span class="lines">@@ -0,0 +1,39 @@
</span><ins>+<!DOCTYPE html>
+<html>
+<head>
+<title></title>
+<style type="text/css">
+doesnotexist:first-letter { color: red; }
+doesnotexist > #targeta:first-letter { color: red; }
+doesnotexist #targetb:first-letter { color: red; }
+doesnotexist + #targetc:first-letter { color: red; }
+doesnotexist ~ #targetd:first-letter { color: red; }
+</style>
+
+<script type="text/javascript">
+if (window.testRunner)
+ testRunner.dumpAsText();
+
+function test()
+{
+ document.body.offsetTop;
+ document.getElementById("targeta").innerHTML = "PASS";
+ document.getElementById("targetb").firstChild.data = "PASS";
+ document.getElementById("targetc").replaceChild(document.createTextNode("PASS"), document.getElementById("targetc").firstChild);
+ document.getElementById("targetd").textContent = "PASS";
+}
+</script>
+
+</head>
+<body onload="test()">
+<p>
+The style resolution has an optimization to detect if a selector has certain pseudo element. The optimization should only happen if the selector completely match with the exception of the pseudo element.
+</p>
+<p>This test pass if all the lines read "PASS".</p>
+<hr>
+<p id="targeta">didn&rsquo;t run</p>
+<p id="targetb">didn&rsquo;t run</p>
+<p id="targetc">didn&rsquo;t run</p>
+<p id="targetd">didn&rsquo;t run</p>
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (169598 => 169599)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2014-06-04 21:54:46 UTC (rev 169598)
+++ trunk/Source/WebCore/ChangeLog 2014-06-04 22:08:44 UTC (rev 169599)
</span><span class="lines">@@ -1,3 +1,32 @@
</span><ins>+2014-06-04 Benjamin Poulain <benjamin@webkit.org>
+
+ Regression(r169547): Crash in WebCore::styleForFirstLetter() while loading http://thenextweb.com/apple/2014/02/21/apple-confirms-acquired-testflight-creator-burstly/
+ https://bugs.webkit.org/show_bug.cgi?id=133500
+
+ Reviewed by Antti Koivisto.
+
+ Unlike the other tree marking optimization, adding HasPseudoStyle() on an element style
+ during style resolution must be conservative and not set the flag unless every other filter
+ matched first.
+
+ After r169547, the SelectorChecker was setting flags on HasPseudoStyle() during the pseudo
+ element matching. If that passed but the remaining filter did not match, the element style
+ could have the flag "FIRST_LETTER" while not being able to resolve a style for "FIRST_LETTER".
+
+ When the layout code was creating the renderer, it would find the FIRST_LETTER flag on the main style
+ and try to get the specific style for FIRST_LETTER. Since no rule would resolve for FIRST_LETTER,
+ the style would be null and the code would be crashing when accessing its parameters.
+
+ This patch fixes the issue by moving the style marking for pseudo element at the end of the matching.
+
+ Test: fast/css/first-letter-on-non-matching-selectors.html
+
+ * css/SelectorChecker.cpp:
+ (WebCore::SelectorChecker::match):
+ (WebCore::SelectorChecker::matchRecursively):
+ * css/SelectorChecker.h:
+ (WebCore::SelectorChecker::match): Deleted.
+
</ins><span class="cx"> 2014-06-04 Eric Carlson <eric.carlson@apple.com>
</span><span class="cx">
</span><span class="cx"> [iOS] allow UA to specify network interface
</span></span></pre></div>
<a id="trunkSourceWebCorecssSelectorCheckercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/css/SelectorChecker.cpp (169598 => 169599)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/css/SelectorChecker.cpp 2014-06-04 21:54:46 UTC (rev 169598)
+++ trunk/Source/WebCore/css/SelectorChecker.cpp 2014-06-04 22:08:44 UTC (rev 169599)
</span><span class="lines">@@ -139,6 +139,23 @@
</span><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+bool SelectorChecker::match(const SelectorCheckingContext& context) const
+{
+ PseudoId pseudoId = NOPSEUDO;
+ if (matchRecursively(context, pseudoId) != SelectorMatches)
+ return false;
+ if (context.pseudoId != NOPSEUDO && context.pseudoId != pseudoId)
+ return false;
+ if (context.pseudoId == NOPSEUDO && pseudoId != NOPSEUDO) {
+ if (m_mode == ResolvingStyle && pseudoId < FIRST_INTERNAL_PSEUDOID)
+ context.elementStyle->setHasPseudoStyle(pseudoId);
+
+ // For SharingRules testing, any match is good enough, we don't care what is matched.
+ return m_mode == SharingRules || m_mode == StyleInvalidation;
+ }
+ return true;
+}
+
</ins><span class="cx"> // Recursive check of selectors and combinators
</span><span class="cx"> // It can return 4 different values:
</span><span class="cx"> // * SelectorMatches - the selector matches the element e
</span><span class="lines">@@ -172,11 +189,6 @@
</span><span class="cx">
</span><span class="cx"> if (pseudoId == FIRST_LETTER)
</span><span class="cx"> context.element->document().styleSheetCollection().setUsesFirstLetterRules(true);
</span><del>-
- if (context.pseudoId == NOPSEUDO) {
- if (m_mode == ResolvingStyle && pseudoId < FIRST_INTERNAL_PSEUDOID)
- context.elementStyle->setHasPseudoStyle(pseudoId);
- }
</del><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebCorecssSelectorCheckerh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/css/SelectorChecker.h (169598 => 169599)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/css/SelectorChecker.h 2014-06-04 21:54:46 UTC (rev 169598)
+++ trunk/Source/WebCore/css/SelectorChecker.h 2014-06-04 22:08:44 UTC (rev 169599)
</span><span class="lines">@@ -82,19 +82,7 @@
</span><span class="cx"> bool hasSelectionPseudo;
</span><span class="cx"> };
</span><span class="cx">
</span><del>- bool match(const SelectorCheckingContext& context) const
- {
- PseudoId pseudoId = NOPSEUDO;
- if (matchRecursively(context, pseudoId) != SelectorMatches)
- return false;
- if (context.pseudoId != NOPSEUDO && context.pseudoId != pseudoId)
- return false;
- if (context.pseudoId == NOPSEUDO && pseudoId != NOPSEUDO) {
- // For SharingRules testing, any match is good enough, we don't care what is matched.
- return m_mode == SharingRules || m_mode == StyleInvalidation;
- }
- return true;
- }
</del><ins>+ bool match(const SelectorCheckingContext&) const;
</ins><span class="cx">
</span><span class="cx"> static bool tagMatches(const Element*, const QualifiedName&);
</span><span class="cx"> static bool isCommonPseudoClassSelector(const CSSSelector*);
</span></span></pre>
</div>
</div>
</body>
</html>