Modified: trunk/LayoutTests/ChangeLog (225013 => 225014)
--- trunk/LayoutTests/ChangeLog 2017-11-18 06:32:23 UTC (rev 225013)
+++ trunk/LayoutTests/ChangeLog 2017-11-18 11:46:06 UTC (rev 225014)
@@ -1,3 +1,14 @@
+2017-11-18 Antti Koivisto <[email protected]>
+
+ REGRESSION (r220646): ASSERTION FAILED: skipAssert || nextSiblingRenderer(node) == m_nextSibling
+ https://bugs.webkit.org/show_bug.cgi?id=179855
+ <rdar://problem/35464071>
+
+ Reviewed by Zalan Bujtas.
+
+ * fast/css-generated-content/first-letter-update-crash-expected.txt: Added.
+ * fast/css-generated-content/first-letter-update-crash.html: Added.
+
2017-11-17 Megan Gardner <[email protected]>
Rebaseline and/or turn on more iOS selection tests that either already pass, or need minimal visual tweaking
Added: trunk/LayoutTests/fast/css-generated-content/first-letter-update-crash-expected.txt (0 => 225014)
--- trunk/LayoutTests/fast/css-generated-content/first-letter-update-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/css-generated-content/first-letter-update-crash-expected.txt 2017-11-18 11:46:06 UTC (rev 225014)
@@ -0,0 +1 @@
+This test passes if it doesn't assert or crash.
Added: trunk/LayoutTests/fast/css-generated-content/first-letter-update-crash.html (0 => 225014)
--- trunk/LayoutTests/fast/css-generated-content/first-letter-update-crash.html (rev 0)
+++ trunk/LayoutTests/fast/css-generated-content/first-letter-update-crash.html 2017-11-18 11:46:06 UTC (rev 225014)
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<body>
+<input value='foo'>
+This test passes if it doesn't assert or crash.
+</body>
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+
+const head = document.getElementsByTagName("head")[0];
+let style = document.createElement("style");
+style.innerHTML = "::first-letter { color:blue; }";
+head.appendChild(style);
+
+document.body.offsetLeft;
+
+style = document.createElement("style");
+style.innerHTML = "* { position: absolute; }";
+head.appendChild(style);
+</script>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (225013 => 225014)
--- trunk/Source/WebCore/ChangeLog 2017-11-18 06:32:23 UTC (rev 225013)
+++ trunk/Source/WebCore/ChangeLog 2017-11-18 11:46:06 UTC (rev 225014)
@@ -1,3 +1,33 @@
+2017-11-18 Antti Koivisto <[email protected]>
+
+ REGRESSION (r220646): REGRESSION (r220646): RenderTreePosition::computeNextSibling hits assertion with certain first-letter mutations
+ https://bugs.webkit.org/show_bug.cgi?id=179855
+ <rdar://problem/35464071>
+
+ Reviewed by Zalan Bujtas.
+
+ Test: fast/css-generated-content/first-letter-update-crash.html
+
+ * style/RenderTreeUpdaterFirstLetter.cpp:
+ (WebCore::styleForFirstLetter):
+ (WebCore::updateFirstLetterStyle):
+ (WebCore::createFirstLetterRenderer):
+
+ Tighten these to take RenderBlock.
+
+ (WebCore::supportsFirstLetter):
+
+ Test exact conditions where first letter renderer is allowed for fast rejection.
+
+ (WebCore::RenderTreeUpdater::FirstLetter::update):
+
+ If update was called on a block that doesn't support first letter, getFirstLetter could in
+ some cases return an ancestor of the block and we would end up mutating a first letter renderer
+ that wasn't current block's descendant. This violates assumptions of the RenderTreeUpdater
+ and could cause cached render tree position to become invalid.
+
+ Fix by ensuring we are always updating first letter for the current block only.
+
2017-11-17 Chris Dumez <[email protected]>
[Service Workers] Implement "Notify Controller Change" algorithm
Modified: trunk/Source/WebCore/style/RenderTreeUpdaterFirstLetter.cpp (225013 => 225014)
--- trunk/Source/WebCore/style/RenderTreeUpdaterFirstLetter.cpp 2017-11-18 06:32:23 UTC (rev 225013)
+++ trunk/Source/WebCore/style/RenderTreeUpdaterFirstLetter.cpp 2017-11-18 11:46:06 UTC (rev 225014)
@@ -26,6 +26,7 @@
#include "FontCascade.h"
#include "RenderBlock.h"
+#include "RenderButton.h"
#include "RenderInline.h"
#include "RenderRubyRun.h"
#include "RenderSVGText.h"
@@ -35,7 +36,7 @@
namespace WebCore {
-static RenderStyle styleForFirstLetter(const RenderElement& firstLetterBlock, const RenderObject& firstLetterContainer)
+static RenderStyle styleForFirstLetter(const RenderBlock& firstLetterBlock, const RenderObject& firstLetterContainer)
{
auto* containerFirstLetterStyle = firstLetterBlock.getCachedPseudoStyle(FIRST_LETTER, &firstLetterContainer.firstLineStyle());
// FIXME: There appears to be some path where we have a first letter renderer without first letter style.
@@ -99,7 +100,7 @@
return isSpaceOrNewline(c) || c == noBreakSpace || isPunctuationForFirstLetter(c);
}
-static void updateFirstLetterStyle(RenderElement& firstLetterBlock, RenderObject& currentChild)
+static void updateFirstLetterStyle(RenderBlock& firstLetterBlock, RenderObject& currentChild)
{
RenderElement* firstLetter = currentChild.parent();
ASSERT(firstLetter->isFirstLetter());
@@ -139,7 +140,7 @@
firstLetter->setStyle(WTFMove(pseudoStyle));
}
-static void createFirstLetterRenderer(RenderElement& firstLetterBlock, RenderText& currentTextChild)
+static void createFirstLetterRenderer(RenderBlock& firstLetterBlock, RenderText& currentTextChild)
{
RenderElement* firstLetterContainer = currentTextChild.parent();
auto pseudoStyle = styleForFirstLetter(firstLetterBlock, *firstLetterContainer);
@@ -209,42 +210,47 @@
static bool supportsFirstLetter(RenderBlock& block)
{
- if (is<RenderTable>(block))
+ if (is<RenderButton>(block))
+ return true;
+ if (!is<RenderBlockFlow>(block))
return false;
if (is<RenderSVGText>(block))
return false;
if (is<RenderRubyRun>(block))
return false;
- return true;
+ return block.canHaveGeneratedChildren();
}
void RenderTreeUpdater::FirstLetter::update(RenderBlock& block)
{
- ASSERT_WITH_SECURITY_IMPLICATION(!block.view().frameView().layoutContext().layoutState());
-
+ if (!block.style().hasPseudoStyle(FIRST_LETTER))
+ return;
if (!supportsFirstLetter(block))
return;
- RenderObject* firstLetterObj;
+ // FIXME: This should be refactored, firstLetterContainer is not needed.
+ RenderObject* firstLetterRenderer;
RenderElement* firstLetterContainer;
- // FIXME: The first letter might be composed of a variety of code units, and therefore might
- // be contained within multiple RenderElements.
- block.getFirstLetter(firstLetterObj, firstLetterContainer);
+ block.getFirstLetter(firstLetterRenderer, firstLetterContainer);
- if (!firstLetterObj || !firstLetterContainer)
+ if (!firstLetterRenderer)
return;
+ // Other containers are handled when updating their renderers.
+ if (&block != firstLetterContainer)
+ return;
+
// If the child already has style, then it has already been created, so we just want
// to update it.
- if (firstLetterObj->parent()->style().styleType() == FIRST_LETTER) {
- updateFirstLetterStyle(*firstLetterContainer, *firstLetterObj);
+ if (firstLetterRenderer->parent()->style().styleType() == FIRST_LETTER) {
+ updateFirstLetterStyle(block, *firstLetterRenderer);
return;
}
- if (!is<RenderText>(*firstLetterObj))
+ if (!is<RenderText>(firstLetterRenderer))
return;
- createFirstLetterRenderer(*firstLetterContainer, downcast<RenderText>(*firstLetterObj));
+ createFirstLetterRenderer(block, downcast<RenderText>(*firstLetterRenderer));
}
};
