Skip to content

Commit

Permalink
Merge r220795 - Move first-letter renderer mutation code out of Rende…
Browse files Browse the repository at this point in the history
…rBlock and into RenderTreeUpdater

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

Reviewed by Andreas Kling.

Render tree should not mutate itself. We already fixed this for first-letter, supporting code
can now move to RenderTreeUpdater too.

* CMakeLists.txt:
* WebCore.xcodeproj/project.pbxproj:
* rendering/RenderBlock.cpp:
(WebCore::styleForFirstLetter): Deleted.
(WebCore::isPunctuationForFirstLetter): Deleted.
(WebCore::shouldSkipForFirstLetter): Deleted.
(WebCore::RenderBlock::updateFirstLetterStyle): Deleted.
(WebCore::RenderBlock::createFirstLetterRenderer): Deleted.
(WebCore::RenderBlock::updateFirstLetter): Deleted.
* rendering/RenderBlock.h:
* rendering/RenderRubyRun.cpp:
(WebCore::RenderRubyRun::updateFirstLetter): Deleted.
* rendering/RenderRubyRun.h:
* rendering/RenderTable.cpp:
(WebCore::RenderTable::updateFirstLetter): Deleted.
* rendering/RenderTable.h:

    Virtual overrides just disabled first letter for some RenderBlock subclasses. This is now achieved via
    supportsFirstLetter test in the first letter updater.

* rendering/TextAutoSizing.cpp:
(WebCore::TextAutoSizingValue::adjustTextNodeSizes):
* rendering/svg/RenderSVGText.cpp:
(WebCore::RenderSVGText::updateFirstLetter): Deleted.
* rendering/svg/RenderSVGText.h:
* style/RenderTreeUpdater.cpp:
(WebCore::RenderTreeUpdater::popParent):
* style/RenderTreeUpdater.h:
* style/RenderTreeUpdaterFirstLetter.cpp: Added.
(WebCore::styleForFirstLetter):
(WebCore::isPunctuationForFirstLetter):
(WebCore::shouldSkipForFirstLetter):
(WebCore::updateFirstLetterStyle):
(WebCore::createFirstLetterRenderer):
(WebCore::supportsFirstLetter):
(WebCore::RenderTreeUpdater::FirstLetter::update):
* style/RenderTreeUpdaterFirstLetter.h: Added.
  • Loading branch information
anttijk authored and carlosgcampos committed Aug 17, 2017
1 parent f09890e commit 98455aa
Show file tree
Hide file tree
Showing 15 changed files with 348 additions and 225 deletions.
1 change: 1 addition & 0 deletions Source/WebCore/CMakeLists.txt
Expand Up @@ -2795,6 +2795,7 @@ set(WebCore_SOURCES
style/InlineTextBoxStyle.cpp
style/RenderTreePosition.cpp
style/RenderTreeUpdater.cpp
style/RenderTreeUpdaterFirstLetter.cpp
style/StyleChange.cpp
style/StyleFontSizeFunctions.cpp
style/StyleInvalidator.cpp
Expand Down
48 changes: 48 additions & 0 deletions Source/WebCore/ChangeLog
@@ -1,3 +1,51 @@
2017-08-16 Antti Koivisto <antti@apple.com>

Move first-letter renderer mutation code out of RenderBlock and into RenderTreeUpdater
https://bugs.webkit.org/show_bug.cgi?id=175627

Reviewed by Andreas Kling.

Render tree should not mutate itself. We already fixed this for first-letter, supporting code
can now move to RenderTreeUpdater too.

* CMakeLists.txt:
* WebCore.xcodeproj/project.pbxproj:
* rendering/RenderBlock.cpp:
(WebCore::styleForFirstLetter): Deleted.
(WebCore::isPunctuationForFirstLetter): Deleted.
(WebCore::shouldSkipForFirstLetter): Deleted.
(WebCore::RenderBlock::updateFirstLetterStyle): Deleted.
(WebCore::RenderBlock::createFirstLetterRenderer): Deleted.
(WebCore::RenderBlock::updateFirstLetter): Deleted.
* rendering/RenderBlock.h:
* rendering/RenderRubyRun.cpp:
(WebCore::RenderRubyRun::updateFirstLetter): Deleted.
* rendering/RenderRubyRun.h:
* rendering/RenderTable.cpp:
(WebCore::RenderTable::updateFirstLetter): Deleted.
* rendering/RenderTable.h:

Virtual overrides just disabled first letter for some RenderBlock subclasses. This is now achieved via
supportsFirstLetter test in the first letter updater.

* rendering/TextAutoSizing.cpp:
(WebCore::TextAutoSizingValue::adjustTextNodeSizes):
* rendering/svg/RenderSVGText.cpp:
(WebCore::RenderSVGText::updateFirstLetter): Deleted.
* rendering/svg/RenderSVGText.h:
* style/RenderTreeUpdater.cpp:
(WebCore::RenderTreeUpdater::popParent):
* style/RenderTreeUpdater.h:
* style/RenderTreeUpdaterFirstLetter.cpp: Added.
(WebCore::styleForFirstLetter):
(WebCore::isPunctuationForFirstLetter):
(WebCore::shouldSkipForFirstLetter):
(WebCore::updateFirstLetterStyle):
(WebCore::createFirstLetterRenderer):
(WebCore::supportsFirstLetter):
(WebCore::RenderTreeUpdater::FirstLetter::update):
* style/RenderTreeUpdaterFirstLetter.h: Added.

2017-08-15 Darin Adler <darin@apple.com>

REGRESSION(r220052): http/tests/appcache/deferred-events-delete-while-raising-timer.html is crashing.
Expand Down
201 changes: 0 additions & 201 deletions Source/WebCore/rendering/RenderBlock.cpp
Expand Up @@ -3085,70 +3085,6 @@ RenderBlock* RenderBlock::firstLineBlock() const
return firstLineBlock;
}

static RenderStyle styleForFirstLetter(const RenderElement& 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.
ASSERT(containerFirstLetterStyle);
auto firstLetterStyle = RenderStyle::clone(containerFirstLetterStyle ? *containerFirstLetterStyle : firstLetterContainer.firstLineStyle());

// If we have an initial letter drop that is >= 1, then we need to force floating to be on.
if (firstLetterStyle.initialLetterDrop() >= 1 && !firstLetterStyle.isFloating())
firstLetterStyle.setFloating(firstLetterStyle.isLeftToRightDirection() ? LeftFloat : RightFloat);

// We have to compute the correct font-size for the first-letter if it has an initial letter height set.
auto* paragraph = firstLetterContainer.isRenderBlockFlow() ? &firstLetterContainer : firstLetterContainer.containingBlock();
if (firstLetterStyle.initialLetterHeight() >= 1 && firstLetterStyle.fontMetrics().hasCapHeight() && paragraph->style().fontMetrics().hasCapHeight()) {
// FIXME: For ideographic baselines, we want to go from line edge to line edge. This is equivalent to (N-1)*line-height + the font height.
// We don't yet support ideographic baselines.
// For an N-line first-letter and for alphabetic baselines, the cap-height of the first letter needs to equal (N-1)*line-height of paragraph lines + cap-height of the paragraph
// Mathematically we can't rely on font-size, since font().height() doesn't necessarily match. For reliability, the best approach is simply to
// compare the final measured cap-heights of the two fonts in order to get to the closest possible value.
firstLetterStyle.setLineBoxContain(LineBoxContainInitialLetter);
int lineHeight = paragraph->style().computedLineHeight();

// Set the font to be one line too big and then ratchet back to get to a precise fit. We can't just set the desired font size based off font height metrics
// because many fonts bake ascent into the font metrics. Therefore we have to look at actual measured cap height values in order to know when we have a good fit.
auto newFontDescription = firstLetterStyle.fontDescription();
float capRatio = firstLetterStyle.fontMetrics().floatCapHeight() / firstLetterStyle.computedFontPixelSize();
float startingFontSize = ((firstLetterStyle.initialLetterHeight() - 1) * lineHeight + paragraph->style().fontMetrics().capHeight()) / capRatio;
newFontDescription.setSpecifiedSize(startingFontSize);
newFontDescription.setComputedSize(startingFontSize);
firstLetterStyle.setFontDescription(newFontDescription);
firstLetterStyle.fontCascade().update(firstLetterStyle.fontCascade().fontSelector());

int desiredCapHeight = (firstLetterStyle.initialLetterHeight() - 1) * lineHeight + paragraph->style().fontMetrics().capHeight();
int actualCapHeight = firstLetterStyle.fontMetrics().capHeight();
while (actualCapHeight > desiredCapHeight) {
auto newFontDescription = firstLetterStyle.fontDescription();
newFontDescription.setSpecifiedSize(newFontDescription.specifiedSize() - 1);
newFontDescription.setComputedSize(newFontDescription.computedSize() -1);
firstLetterStyle.setFontDescription(newFontDescription);
firstLetterStyle.fontCascade().update(firstLetterStyle.fontCascade().fontSelector());
actualCapHeight = firstLetterStyle.fontMetrics().capHeight();
}
}

// Force inline display (except for floating first-letters).
firstLetterStyle.setDisplay(firstLetterStyle.isFloating() ? BLOCK : INLINE);
// CSS2 says first-letter can't be positioned.
firstLetterStyle.setPosition(StaticPosition);
return firstLetterStyle;
}

// CSS 2.1 http://www.w3.org/TR/CSS21/selector.html#first-letter
// "Punctuation (i.e, characters defined in Unicode [UNICODE] in the "open" (Ps), "close" (Pe),
// "initial" (Pi). "final" (Pf) and "other" (Po) punctuation classes), that precedes or follows the first letter should be included"
static inline bool isPunctuationForFirstLetter(UChar c)
{
return U_GET_GC_MASK(c) & (U_GC_PS_MASK | U_GC_PE_MASK | U_GC_PI_MASK | U_GC_PF_MASK | U_GC_PO_MASK);
}

static inline bool shouldSkipForFirstLetter(UChar c)
{
return isSpaceOrNewline(c) || c == noBreakSpace || isPunctuationForFirstLetter(c);
}

static inline RenderBlock* findFirstLetterBlock(RenderBlock* start)
{
RenderBlock* firstLetterBlock = start;
Expand All @@ -3169,117 +3105,6 @@ static inline RenderBlock* findFirstLetterBlock(RenderBlock* start)
return nullptr;
}

void RenderBlock::updateFirstLetterStyle(RenderElement* firstLetterBlock, RenderObject* currentChild)
{
RenderElement* firstLetter = currentChild->parent();
RenderElement* firstLetterContainer = firstLetter->parent();
auto pseudoStyle = styleForFirstLetter(*firstLetterBlock, *firstLetterContainer);
ASSERT(firstLetter->isFloating() || firstLetter->isInline());

if (Style::determineChange(firstLetter->style(), pseudoStyle) == Style::Detach) {
// The first-letter renderer needs to be replaced. Create a new renderer of the right type.
RenderBoxModelObject* newFirstLetter;
if (pseudoStyle.display() == INLINE)
newFirstLetter = new RenderInline(document(), WTFMove(pseudoStyle));
else
newFirstLetter = new RenderBlockFlow(document(), WTFMove(pseudoStyle));
newFirstLetter->initializeStyle();

// Move the first letter into the new renderer.
LayoutStateDisabler layoutStateDisabler(view());
while (RenderObject* child = firstLetter->firstChild()) {
if (is<RenderText>(*child))
downcast<RenderText>(*child).removeAndDestroyTextBoxes();
firstLetter->removeChild(*child);
newFirstLetter->addChild(child, nullptr);
}

RenderObject* nextSibling = firstLetter->nextSibling();
if (RenderTextFragment* remainingText = downcast<RenderBoxModelObject>(*firstLetter).firstLetterRemainingText()) {
ASSERT(remainingText->isAnonymous() || remainingText->textNode()->renderer() == remainingText);
// Replace the old renderer with the new one.
remainingText->setFirstLetter(*newFirstLetter);
newFirstLetter->setFirstLetterRemainingText(remainingText);
}
// To prevent removal of single anonymous block in RenderBlock::removeChild and causing
// |nextSibling| to go stale, we remove the old first letter using removeChildNode first.
firstLetterContainer->removeChildInternal(*firstLetter, NotifyChildren);
firstLetter->destroy();
firstLetter = newFirstLetter;
firstLetterContainer->addChild(firstLetter, nextSibling);
} else
firstLetter->setStyle(WTFMove(pseudoStyle));
}

void RenderBlock::createFirstLetterRenderer(RenderElement* firstLetterBlock, RenderText* currentTextChild)
{
RenderElement* firstLetterContainer = currentTextChild->parent();
auto pseudoStyle = styleForFirstLetter(*firstLetterBlock, *firstLetterContainer);
RenderBoxModelObject* firstLetter = nullptr;
if (pseudoStyle.display() == INLINE)
firstLetter = new RenderInline(document(), WTFMove(pseudoStyle));
else
firstLetter = new RenderBlockFlow(document(), WTFMove(pseudoStyle));
firstLetter->initializeStyle();
firstLetterContainer->addChild(firstLetter, currentTextChild);

// The original string is going to be either a generated content string or a DOM node's
// string. We want the original string before it got transformed in case first-letter has
// no text-transform or a different text-transform applied to it.
String oldText = currentTextChild->originalText();
ASSERT(!oldText.isNull());

if (!oldText.isEmpty()) {
unsigned length = 0;

// Account for leading spaces and punctuation.
while (length < oldText.length() && shouldSkipForFirstLetter(oldText[length]))
length++;

// Account for first grapheme cluster.
length += numCharactersInGraphemeClusters(StringView(oldText).substring(length), 1);

// Keep looking for whitespace and allowed punctuation, but avoid
// accumulating just whitespace into the :first-letter.
for (unsigned scanLength = length; scanLength < oldText.length(); ++scanLength) {
UChar c = oldText[scanLength];

if (!shouldSkipForFirstLetter(c))
break;

if (isPunctuationForFirstLetter(c))
length = scanLength + 1;
}

// Construct a text fragment for the text after the first letter.
// This text fragment might be empty.
RenderTextFragment* remainingText;
if (currentTextChild->textNode())
remainingText = new RenderTextFragment(*currentTextChild->textNode(), oldText, length, oldText.length() - length);
else
remainingText = new RenderTextFragment(document(), oldText, length, oldText.length() - length);

if (remainingText->textNode())
remainingText->textNode()->setRenderer(remainingText);

firstLetterContainer->addChild(remainingText, currentTextChild);
firstLetterContainer->removeChild(*currentTextChild);
remainingText->setFirstLetter(*firstLetter);
firstLetter->setFirstLetterRemainingText(remainingText);

// construct text fragment for the first letter
RenderTextFragment* letter;
if (remainingText->textNode())
letter = new RenderTextFragment(*remainingText->textNode(), oldText, 0, length);
else
letter = new RenderTextFragment(document(), oldText, 0, length);

firstLetter->addChild(letter);

currentTextChild->destroy();
}
}

void RenderBlock::getFirstLetter(RenderObject*& firstLetter, RenderElement*& firstLetterContainer, RenderObject* skipObject)
{
firstLetter = nullptr;
Expand Down Expand Up @@ -3335,32 +3160,6 @@ void RenderBlock::getFirstLetter(RenderObject*& firstLetter, RenderElement*& fir
firstLetterContainer = nullptr;
}

void RenderBlock::updateFirstLetter()
{
ASSERT_WITH_SECURITY_IMPLICATION(!view().layoutState());

RenderObject* firstLetterObj;
RenderElement* firstLetterContainer;
// FIXME: The first letter might be composed of a variety of code units, and therefore might
// be contained within multiple RenderElements.
getFirstLetter(firstLetterObj, firstLetterContainer);

if (!firstLetterObj || !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);
return;
}

if (!is<RenderText>(*firstLetterObj))
return;

createFirstLetterRenderer(firstLetterContainer, downcast<RenderText>(firstLetterObj));
}

RenderFlowThread* RenderBlock::cachedFlowThreadContainingBlock() const
{
RenderBlockRareData* rareData = getBlockRareData(*this);
Expand Down
4 changes: 0 additions & 4 deletions Source/WebCore/rendering/RenderBlock.h
Expand Up @@ -263,7 +263,6 @@ class RenderBlock : public RenderBox {
LayoutUnit collapsedMarginBeforeForChild(const RenderBox& child) const;
LayoutUnit collapsedMarginAfterForChild(const RenderBox& child) const;

virtual void updateFirstLetter();
void getFirstLetter(RenderObject*& firstLetter, RenderElement*& firstLetterContainer, RenderObject* skipObject = nullptr);

virtual void scrollbarsChanged(bool /*horizontalScrollbarChanged*/, bool /*verticalScrollbarChanged*/) { }
Expand Down Expand Up @@ -454,9 +453,6 @@ class RenderBlock : public RenderBox {
bool isSelfCollapsingBlock() const override;
virtual bool childrenPreventSelfCollapsing() const;

void createFirstLetterRenderer(RenderElement* firstLetterBlock, RenderText* currentTextChild);
void updateFirstLetterStyle(RenderElement* firstLetterBlock, RenderObject* firstLetterContainer);

Node* nodeForHitTest() const;

// FIXME-BLOCKFLOW: Remove virtualizaion when all callers have moved to RenderBlockFlow
Expand Down
4 changes: 0 additions & 4 deletions Source/WebCore/rendering/RenderRubyRun.cpp
Expand Up @@ -101,10 +101,6 @@ RenderBlock* RenderRubyRun::firstLineBlock() const
return 0;
}

void RenderRubyRun::updateFirstLetter()
{
}

bool RenderRubyRun::isChildAllowed(const RenderObject& child, const RenderStyle&) const
{
return child.isInline() || child.isRubyText();
Expand Down
1 change: 0 additions & 1 deletion Source/WebCore/rendering/RenderRubyRun.h
Expand Up @@ -60,7 +60,6 @@ class RenderRubyRun final : public RenderBlockFlow {
void removeChild(RenderObject&) override;

RenderBlock* firstLineBlock() const override;
void updateFirstLetter() override;

void getOverhang(bool firstLine, RenderObject* startRenderer, RenderObject* endRenderer, float& startOverhang, float& endOverhang) const;

Expand Down
4 changes: 0 additions & 4 deletions Source/WebCore/rendering/RenderTable.cpp
Expand Up @@ -1485,10 +1485,6 @@ RenderBlock* RenderTable::firstLineBlock() const
return nullptr;
}

void RenderTable::updateFirstLetter()
{
}

int RenderTable::baselinePosition(FontBaseline baselineType, bool firstLine, LineDirectionMode direction, LinePositionMode linePositionMode) const
{
return valueOrCompute(firstLineBaseline(), [&] {
Expand Down
1 change: 0 additions & 1 deletion Source/WebCore/rendering/RenderTable.h
Expand Up @@ -302,7 +302,6 @@ class RenderTable : public RenderBlock {
void invalidateCachedColumnOffsets();

RenderBlock* firstLineBlock() const final;
void updateFirstLetter() final;

void updateLogicalWidth() final;

Expand Down
5 changes: 3 additions & 2 deletions Source/WebCore/rendering/TextAutoSizing.cpp
Expand Up @@ -36,6 +36,7 @@
#include "RenderListMarker.h"
#include "RenderText.h"
#include "RenderTextFragment.h"
#include "RenderTreeUpdaterFirstLetter.h"
#include "StyleResolver.h"

namespace WebCore {
Expand Down Expand Up @@ -157,15 +158,15 @@ auto TextAutoSizingValue::adjustTextNodeSizes() -> StillHasNodes
parentRenderer->setStyle(WTFMove(newParentStyle));
}

// FIXME: All render tree mutations should be done via RenderTreeUpdater.
for (auto& node : m_autoSizedNodes) {
auto& textRenderer = *node->renderer();
if (!is<RenderTextFragment>(textRenderer))
continue;
auto* block = downcast<RenderTextFragment>(textRenderer).blockForAccompanyingFirstLetter();
if (!block)
continue;
block->updateFirstLetter();
// FIXME: All render tree mutations should be done by RenderTreeUpdater commit.
RenderTreeUpdater::FirstLetter::update(*block);
}

return stillHasNodes;
Expand Down
6 changes: 0 additions & 6 deletions Source/WebCore/rendering/svg/RenderSVGText.cpp
Expand Up @@ -542,10 +542,4 @@ RenderBlock* RenderSVGText::firstLineBlock() const
return 0;
}

// Fix for <rdar://problem/8048875>. We should not render :first-letter CSS Style
// in a SVG text element context.
void RenderSVGText::updateFirstLetter()
{
}

}
1 change: 0 additions & 1 deletion Source/WebCore/rendering/svg/RenderSVGText.h
Expand Up @@ -91,7 +91,6 @@ class RenderSVGText final : public RenderSVGBlock {
std::unique_ptr<RootInlineBox> createRootInlineBox() override;

RenderBlock* firstLineBlock() const override;
void updateFirstLetter() override;

bool shouldHandleSubtreeMutations() const;

Expand Down
3 changes: 2 additions & 1 deletion Source/WebCore/style/RenderTreeUpdater.cpp
Expand Up @@ -41,6 +41,7 @@
#include "RenderFullScreen.h"
#include "RenderNamedFlowThread.h"
#include "RenderQuote.h"
#include "RenderTreeUpdaterFirstLetter.h"
#include "StyleResolver.h"
#include "StyleTreeResolver.h"
#include <wtf/SystemTracing.h>
Expand Down Expand Up @@ -234,7 +235,7 @@ void RenderTreeUpdater::popParent()

auto* renderer = parent.element->renderer();
if (is<RenderBlock>(renderer))
downcast<RenderBlock>(*renderer).updateFirstLetter();
FirstLetter::update(downcast<RenderBlock>(*renderer));

if (parent.element->hasCustomStyleResolveCallbacks() && parent.styleChange == Style::Detach && renderer)
parent.element->didAttachRenderers();
Expand Down

0 comments on commit 98455aa

Please sign in to comment.