Skip to content

Commit

Permalink
Merge r220916 - Factor render tree mutation code from RenderListItem …
Browse files Browse the repository at this point in the history
…to RenderTreeUpdater

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

Reviewed by Andreas Kling.

We already stopped doing layout time mutations. We can now move the code out too.

* WebCore.xcodeproj/project.pbxproj:
* rendering/RenderListItem.cpp:
(WebCore::isHTMLListElement):
(WebCore::getParentOfFirstLineBox): Deleted.
(WebCore::firstNonMarkerChild): Deleted.
(WebCore::RenderListItem::updateMarkerRenderer): Deleted.

    Moved to RenderTreeUpdater::ListItem.

* rendering/RenderListItem.h:
* rendering/RenderListMarker.cpp:
(WebCore::RenderListMarker::willBeDestroyed):
* rendering/TextAutoSizing.cpp:
(WebCore::TextAutoSizingValue::adjustTextNodeSizes):
* style/RenderTreeUpdater.cpp:
(WebCore::RenderTreeUpdater::pushParent):
(WebCore::RenderTreeUpdater::popParent):
(WebCore::RenderTreeUpdater::updateBeforeDescendants):
(WebCore::RenderTreeUpdater::updateAfterDescendants):

    Factor pre/post update into functions.

(WebCore::RenderTreeUpdater::updateBeforeOrAfterPseudoElement):
* style/RenderTreeUpdater.h:
* style/RenderTreeUpdaterListItem.cpp: Added.

    Mutation functions move here.

(WebCore::getParentOfFirstLineBox):
(WebCore::firstNonMarkerChild):
(WebCore::RenderTreeUpdater::ListItem::updateMarker):
* style/RenderTreeUpdaterListItem.h: Added.
  • Loading branch information
anttijk authored and carlosgcampos committed Aug 28, 2017
1 parent 0a06eca commit 43a3335
Show file tree
Hide file tree
Showing 10 changed files with 241 additions and 106 deletions.
1 change: 1 addition & 0 deletions Source/WebCore/CMakeLists.txt
Expand Up @@ -2796,6 +2796,7 @@ set(WebCore_SOURCES
style/RenderTreePosition.cpp
style/RenderTreeUpdater.cpp
style/RenderTreeUpdaterFirstLetter.cpp
style/RenderTreeUpdaterListItem.cpp
style/StyleChange.cpp
style/StyleFontSizeFunctions.cpp
style/StyleInvalidator.cpp
Expand Down
42 changes: 42 additions & 0 deletions Source/WebCore/ChangeLog
@@ -1,3 +1,45 @@
2017-08-18 Antti Koivisto <antti@apple.com>

Factor render tree mutation code from RenderListItem to RenderTreeUpdater
https://bugs.webkit.org/show_bug.cgi?id=175718

Reviewed by Andreas Kling.

We already stopped doing layout time mutations. We can now move the code out too.

* WebCore.xcodeproj/project.pbxproj:
* rendering/RenderListItem.cpp:
(WebCore::isHTMLListElement):
(WebCore::getParentOfFirstLineBox): Deleted.
(WebCore::firstNonMarkerChild): Deleted.
(WebCore::RenderListItem::updateMarkerRenderer): Deleted.

Moved to RenderTreeUpdater::ListItem.

* rendering/RenderListItem.h:
* rendering/RenderListMarker.cpp:
(WebCore::RenderListMarker::willBeDestroyed):
* rendering/TextAutoSizing.cpp:
(WebCore::TextAutoSizingValue::adjustTextNodeSizes):
* style/RenderTreeUpdater.cpp:
(WebCore::RenderTreeUpdater::pushParent):
(WebCore::RenderTreeUpdater::popParent):
(WebCore::RenderTreeUpdater::updateBeforeDescendants):
(WebCore::RenderTreeUpdater::updateAfterDescendants):

Factor pre/post update into functions.

(WebCore::RenderTreeUpdater::updateBeforeOrAfterPseudoElement):
* style/RenderTreeUpdater.h:
* style/RenderTreeUpdaterListItem.cpp: Added.

Mutation functions move here.

(WebCore::getParentOfFirstLineBox):
(WebCore::firstNonMarkerChild):
(WebCore::RenderTreeUpdater::ListItem::updateMarker):
* style/RenderTreeUpdaterListItem.h: Added.

2017-08-18 Ms2ger <Ms2ger@gmail.com>

[GTK] Show controls if a video element isn't allowed to play inline.
Expand Down
86 changes: 1 addition & 85 deletions Source/WebCore/rendering/RenderListItem.cpp
Expand Up @@ -34,9 +34,6 @@
#include "RenderChildIterator.h"
#include "RenderInline.h"
#include "RenderListMarker.h"
#include "RenderMultiColumnFlowThread.h"
#include "RenderRuby.h"
#include "RenderTable.h"
#include "RenderView.h"
#include "StyleInheritedData.h"
#if !ASSERT_DISABLED
Expand Down Expand Up @@ -110,7 +107,7 @@ void RenderListItem::willBeRemovedFromTree()
updateListMarkerNumbers();
}

static inline bool isHTMLListElement(const Node& node)
bool isHTMLListElement(const Node& node)
{
return is<HTMLUListElement>(node) || is<HTMLOListElement>(node);
}
Expand Down Expand Up @@ -226,35 +223,6 @@ void RenderListItem::updateValueNow() const
m_isValueUpToDate = true;
}

static RenderBlock* getParentOfFirstLineBox(RenderBlock& current, RenderObject& marker)
{
bool inQuirksMode = current.document().inQuirksMode();
for (auto& child : childrenOfType<RenderObject>(current)) {
if (&child == &marker)
continue;

if (child.isInline() && (!is<RenderInline>(child) || current.generatesLineBoxesForInlineChild(&child)))
return &current;

if (child.isFloating() || child.isOutOfFlowPositioned())
continue;

if (!is<RenderBlock>(child) || is<RenderTable>(child) || is<RenderRubyAsBlock>(child))
break;

if (is<RenderBox>(child) && downcast<RenderBox>(child).isWritingModeRoot())
break;

if (is<RenderListItem>(current) && inQuirksMode && child.node() && isHTMLListElement(*child.node()))
break;

if (RenderBlock* lineBox = getParentOfFirstLineBox(downcast<RenderBlock>(child), marker))
return lineBox;
}

return nullptr;
}

void RenderListItem::updateValue()
{
if (!m_hasExplicitValue) {
Expand All @@ -264,58 +232,6 @@ void RenderListItem::updateValue()
}
}

static RenderObject* firstNonMarkerChild(RenderBlock& parent)
{
RenderObject* child = parent.firstChild();
while (is<RenderListMarker>(child))
child = child->nextSibling();
return child;
}

void RenderListItem::updateMarkerRenderer()
{
ASSERT_WITH_SECURITY_IMPLICATION(!view().layoutState());

if (style().listStyleType() == NoneListStyle && (!style().listStyleImage() || style().listStyleImage()->errorOccurred())) {
if (m_marker) {
m_marker->destroy();
ASSERT(!m_marker);
}
return;
}

auto newStyle = computeMarkerStyle();
if (m_marker)
m_marker->setStyle(WTFMove(newStyle));
else {
m_marker = createRenderer<RenderListMarker>(*this, WTFMove(newStyle)).leakPtr();
m_marker->initializeStyle();
}

RenderElement* currentParent = m_marker->parent();
RenderBlock* newParent = getParentOfFirstLineBox(*this, *m_marker);
if (!newParent) {
// If the marker is currently contained inside an anonymous box,
// then we are the only item in that anonymous box (since no line box
// parent was found). It's ok to just leave the marker where it is
// in this case.
if (currentParent && currentParent->isAnonymousBlock())
return;
if (multiColumnFlowThread())
newParent = multiColumnFlowThread();
else
newParent = this;
}

if (newParent != currentParent) {
m_marker->removeFromParent();
newParent->addChild(m_marker, firstNonMarkerChild(*newParent));
// If current parent is an anonymous block that has lost all its children, destroy it.
if (currentParent && currentParent->isAnonymousBlock() && !currentParent->firstChild() && !downcast<RenderBlock>(*currentParent).continuation())
currentParent->destroy();
}
}

void RenderListItem::layout()
{
StackStats::LayoutCheckPoint layoutCheckPoint;
Expand Down
8 changes: 5 additions & 3 deletions Source/WebCore/rendering/RenderListItem.h
Expand Up @@ -54,9 +54,10 @@ class RenderListItem final : public RenderBlockFlow {
static void updateItemValuesForOrderedList(const HTMLOListElement&);
static unsigned itemCountForOrderedList(const HTMLOListElement&);

void didDestroyListMarker() { m_marker = nullptr; }
RenderStyle computeMarkerStyle() const;

void updateMarkerRenderer();
RenderListMarker* markerRenderer() { return m_marker; }
void setMarkerRenderer(RenderListMarker* marker) { m_marker = marker; }

#if !ASSERT_DISABLED
bool inLayout() const { return m_inLayout; }
Expand Down Expand Up @@ -85,7 +86,6 @@ class RenderListItem final : public RenderBlockFlow {
void updateValueNow() const;
void explicitValueChanged();

RenderStyle computeMarkerStyle() const;

int m_explicitValue;
RenderListMarker* m_marker;
Expand All @@ -98,6 +98,8 @@ class RenderListItem final : public RenderBlockFlow {
bool m_notInList : 1;
};

bool isHTMLListElement(const Node&);

} // namespace WebCore

SPECIALIZE_TYPE_TRAITS_RENDER_OBJECT(RenderListItem, isListItem())
2 changes: 1 addition & 1 deletion Source/WebCore/rendering/RenderListMarker.cpp
Expand Up @@ -1133,7 +1133,7 @@ RenderListMarker::~RenderListMarker()

void RenderListMarker::willBeDestroyed()
{
m_listItem.didDestroyListMarker();
m_listItem.setMarkerRenderer(nullptr);
if (m_image)
m_image->removeClient(this);

Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/rendering/TextAutoSizing.cpp
Expand Up @@ -33,11 +33,11 @@
#include "FontCascade.h"
#include "Logging.h"
#include "RenderBlock.h"
#include "RenderListItem.h"
#include "RenderListMarker.h"
#include "RenderText.h"
#include "RenderTextFragment.h"
#include "RenderTreeUpdaterFirstLetter.h"
#include "RenderTreeUpdaterListItem.h"
#include "StyleResolver.h"

namespace WebCore {
Expand Down Expand Up @@ -159,7 +159,7 @@ auto TextAutoSizingValue::adjustTextNodeSizes() -> StillHasNodes
parentRenderer->setStyle(WTFMove(newParentStyle));

if (is<RenderListItem>(*parentRenderer))
downcast<RenderListItem>(*parentRenderer).updateMarkerRenderer();
RenderTreeUpdater::ListItem::updateMarker(downcast<RenderListItem>(*parentRenderer));
}

for (auto& node : m_autoSizedNodes) {
Expand Down
43 changes: 28 additions & 15 deletions Source/WebCore/style/RenderTreeUpdater.cpp
Expand Up @@ -43,6 +43,7 @@
#include "RenderNamedFlowThread.h"
#include "RenderQuote.h"
#include "RenderTreeUpdaterFirstLetter.h"
#include "RenderTreeUpdaterListItem.h"
#include "StyleResolver.h"
#include "StyleTreeResolver.h"
#include <wtf/SystemTracing.h>
Expand Down Expand Up @@ -224,26 +225,15 @@ void RenderTreeUpdater::pushParent(Element& element, Style::Change changeType)
{
m_parentStack.append(Parent(element, changeType));

updateBeforeOrAfterPseudoElement(element, BEFORE);
updateBeforeDescendants(element);
}

void RenderTreeUpdater::popParent()
{
auto& parent = m_parentStack.last();
if (parent.element)
updateAfterDescendants(*parent.element, parent.styleChange);

if (parent.element) {
updateBeforeOrAfterPseudoElement(*parent.element, AFTER);

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

if (parent.element->hasCustomStyleResolveCallbacks() && parent.styleChange == Style::Detach)
parent.element->didAttachRenderers();
}
}
m_parentStack.removeLast();
}

Expand All @@ -255,6 +245,29 @@ void RenderTreeUpdater::popParentsToDepth(unsigned depth)
popParent();
}

void RenderTreeUpdater::updateBeforeDescendants(Element& element)
{
updateBeforeOrAfterPseudoElement(element, BEFORE);
}

void RenderTreeUpdater::updateAfterDescendants(Element& element, Style::Change styleChange)
{
updateBeforeOrAfterPseudoElement(element, AFTER);

auto* renderer = element.renderer();
if (!renderer)
return;

// These functions do render tree mutations that require descendant renderers.
if (is<RenderBlock>(*renderer))
FirstLetter::update(downcast<RenderBlock>(*renderer));
if (is<RenderListItem>(*renderer))
ListItem::updateMarker(downcast<RenderListItem>(*renderer));

if (element.hasCustomStyleResolveCallbacks() && styleChange == Style::Detach)
element.didAttachRenderers();
}

static bool pseudoStyleCacheIsInvalid(RenderElement* renderer, RenderStyle* newStyle)
{
const RenderStyle& currentStyle = renderer->style();
Expand Down Expand Up @@ -571,7 +584,7 @@ void RenderTreeUpdater::updateBeforeOrAfterPseudoElement(Element& current, Pseud
updateQuotesUpTo(&child);
}
if (is<RenderListItem>(*pseudoRenderer))
downcast<RenderListItem>(*pseudoRenderer).updateMarkerRenderer();
ListItem::updateMarker(downcast<RenderListItem>(*pseudoRenderer));
}

void RenderTreeUpdater::tearDownRenderers(Element& root, TeardownType teardownType)
Expand Down
3 changes: 3 additions & 0 deletions Source/WebCore/style/RenderTreeUpdater.h
Expand Up @@ -52,13 +52,16 @@ class RenderTreeUpdater {
static void tearDownRenderer(Text&);

class FirstLetter;
class ListItem;

private:
void updateRenderTree(ContainerNode& root);
void updateTextRenderer(Text&, const Style::TextUpdate*);
void updateElementRenderer(Element&, const Style::ElementUpdate&);
void createRenderer(Element&, RenderStyle&&);
void invalidateWhitespaceOnlyTextSiblingsAfterAttachIfNeeded(Node&);
void updateBeforeDescendants(Element&);
void updateAfterDescendants(Element&, Style::Change);
void updateBeforeOrAfterPseudoElement(Element&, PseudoId);

struct Parent {
Expand Down

0 comments on commit 43a3335

Please sign in to comment.