Skip to content
Permalink
Browse files
Unreviewed, rolling out r223500.
https://bugs.webkit.org/show_bug.cgi?id=178408

Introduced a crash in CSSAnimationController::updateAnimations
(Requested by rniwa on #webkit).

Reverted changeset:

"Resolve ::before and ::after pseudo elements during style
resolution"
https://bugs.webkit.org/show_bug.cgi?id=178339
https://trac.webkit.org/changeset/223500

Canonical link: https://commits.webkit.org/194640@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@223579 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
webkit-commit-queue committed Oct 17, 2017
1 parent 28d7864 commit 365528c57f44f6d0edade5d3f853ebda8568eba9
@@ -1,3 +1,18 @@
2017-10-17 Commit Queue <commit-queue@webkit.org>

Unreviewed, rolling out r223500.
https://bugs.webkit.org/show_bug.cgi?id=178408

Introduced a crash in CSSAnimationController::updateAnimations
(Requested by rniwa on #webkit).

Reverted changeset:

"Resolve ::before and ::after pseudo elements during style
resolution"
https://bugs.webkit.org/show_bug.cgi?id=178339
https://trac.webkit.org/changeset/223500

2017-10-17 Myles C. Maxfield <mmaxfield@apple.com>

Delete button doesn't fully delete certain emoji
@@ -28,7 +28,6 @@
#include "config.h"
#include "PseudoElement.h"

#include "CSSAnimationController.h"
#include "ContentData.h"
#include "InspectorInstrumentation.h"
#include "RenderElement.h"
@@ -72,22 +71,10 @@ PseudoElement::~PseudoElement()
ASSERT(!m_hostElement);
}

Ref<PseudoElement> PseudoElement::create(Element& host, PseudoId pseudoId)
{
auto pseudoElement = adoptRef(*new PseudoElement(host, pseudoId));

InspectorInstrumentation::pseudoElementCreated(host.document().page(), pseudoElement.get());

return pseudoElement;
}

void PseudoElement::clearHostElement()
{
InspectorInstrumentation::pseudoElementDestroyed(document().page(), *this);

if (auto* frame = document().frame())
frame->animation().cancelAnimations(*this);

m_hostElement = nullptr;
}

@@ -33,7 +33,10 @@ namespace WebCore {

class PseudoElement final : public Element {
public:
static Ref<PseudoElement> create(Element& host, PseudoId);
static Ref<PseudoElement> create(Element& host, PseudoId pseudoId)
{
return adoptRef(*new PseudoElement(host, pseudoId));
}
virtual ~PseudoElement();

Element* hostElement() const { return m_hostElement; }
@@ -75,9 +75,9 @@ RenderTreeUpdater::Parent::Parent(ContainerNode& root)
{
}

RenderTreeUpdater::Parent::Parent(Element& element, const Style::ElementUpdates* updates)
RenderTreeUpdater::Parent::Parent(Element& element, Style::Change styleChange)
: element(&element)
, updates(updates)
, styleChange(styleChange)
, renderTreePosition(element.renderer() ? std::make_optional(RenderTreePosition(*element.renderer())) : std::nullopt)
{
}
@@ -176,7 +176,7 @@ void RenderTreeUpdater::updateRenderTree(ContainerNode& root)
if (is<Text>(node)) {
auto& text = downcast<Text>(node);
auto* textUpdate = m_styleUpdate->textUpdate(text);
if ((parent().updates && parent().updates->update.change == Style::Detach) || textUpdate || m_invalidatedWhitespaceOnlyTextSiblings.contains(&text))
if (parent().styleChange == Style::Detach || textUpdate || m_invalidatedWhitespaceOnlyTextSiblings.contains(&text))
updateTextRenderer(text, textUpdate);

it.traverseNextSkippingChildren();
@@ -185,25 +185,25 @@ void RenderTreeUpdater::updateRenderTree(ContainerNode& root)

auto& element = downcast<Element>(node);

auto* elementUpdates = m_styleUpdate->elementUpdates(element);
auto* elementUpdate = m_styleUpdate->elementUpdate(element);

// We hop through display: contents elements in findRenderingRoot, so
// there may be other updates down the tree.
if (!elementUpdates && !element.hasDisplayContents()) {
if (!elementUpdate && !element.hasDisplayContents()) {
it.traverseNextSkippingChildren();
continue;
}

if (elementUpdates)
updateElementRenderer(element, elementUpdates->update);
if (elementUpdate)
updateElementRenderer(element, *elementUpdate);

bool mayHaveRenderedDescendants = element.renderer() || (element.hasDisplayContents() && shouldCreateRenderer(element, renderTreePosition().parent()));
if (!mayHaveRenderedDescendants) {
it.traverseNextSkippingChildren();
continue;
}

pushParent(element, elementUpdates);
pushParent(element, elementUpdate ? elementUpdate->change : Style::NoChange);

it.traverseNext();
}
@@ -223,18 +223,18 @@ RenderTreePosition& RenderTreeUpdater::renderTreePosition()
return *m_parentStack.last().renderTreePosition;
}

void RenderTreeUpdater::pushParent(Element& element, const Style::ElementUpdates* updates)
void RenderTreeUpdater::pushParent(Element& element, Style::Change changeType)
{
m_parentStack.append(Parent(element, updates));
m_parentStack.append(Parent(element, changeType));

updateBeforeDescendants(element, updates);
updateBeforeDescendants(element);
}

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

m_parentStack.removeLast();
}
@@ -247,16 +247,14 @@ void RenderTreeUpdater::popParentsToDepth(unsigned depth)
popParent();
}

void RenderTreeUpdater::updateBeforeDescendants(Element& element, const Style::ElementUpdates* updates)
void RenderTreeUpdater::updateBeforeDescendants(Element& element)
{
if (updates)
generatedContent().updatePseudoElement(element, updates->beforePseudoElementUpdate, BEFORE);
generatedContent().updateBeforePseudoElement(element);
}

void RenderTreeUpdater::updateAfterDescendants(Element& element, const Style::ElementUpdates* updates)
void RenderTreeUpdater::updateAfterDescendants(Element& element, Style::Change styleChange)
{
if (updates)
generatedContent().updatePseudoElement(element, updates->afterPseudoElementUpdate, AFTER);
generatedContent().updateAfterPseudoElement(element);

auto* renderer = element.renderer();
if (!renderer)
@@ -270,7 +268,7 @@ void RenderTreeUpdater::updateAfterDescendants(Element& element, const Style::El
if (is<RenderBlockFlow>(*renderer))
MultiColumn::update(downcast<RenderBlockFlow>(*renderer));

if (element.hasCustomStyleResolveCallbacks() && updates && updates->update.change == Style::Detach)
if (element.hasCustomStyleResolveCallbacks() && styleChange == Style::Detach)
element.didAttachRenderers();
}

@@ -495,7 +493,7 @@ void RenderTreeUpdater::invalidateWhitespaceOnlyTextSiblingsAfterAttachIfNeeded(
// the current node gaining or losing the renderer. This can only affect white space text nodes.
for (Node* sibling = current.nextSibling(); sibling; sibling = sibling->nextSibling()) {
if (is<Element>(*sibling)) {
if (m_styleUpdate->elementUpdates(downcast<Element>(*sibling)))
if (m_styleUpdate->elementUpdate(downcast<Element>(*sibling)))
return;
// Text renderers beyond rendered elements can't be affected.
if (sibling->renderer())
@@ -62,23 +62,23 @@ class RenderTreeUpdater {
void updateElementRenderer(Element&, const Style::ElementUpdate&);
void createRenderer(Element&, RenderStyle&&);
void invalidateWhitespaceOnlyTextSiblingsAfterAttachIfNeeded(Node&);
void updateBeforeDescendants(Element&, const Style::ElementUpdates*);
void updateAfterDescendants(Element&, const Style::ElementUpdates*);
void updateBeforeDescendants(Element&);
void updateAfterDescendants(Element&, Style::Change);

struct Parent {
Element* element { nullptr };
const Style::ElementUpdates* updates { nullptr };
Style::Change styleChange { Style::NoChange };
std::optional<RenderTreePosition> renderTreePosition;

Parent(ContainerNode& root);
Parent(Element&, const Style::ElementUpdates*);
Parent(Element&, Style::Change);
};
Parent& parent() { return m_parentStack.last(); }
RenderTreePosition& renderTreePosition();

GeneratedContent& generatedContent() { return *m_generatedContent; }

void pushParent(Element&, const Style::ElementUpdates*);
void pushParent(Element&, Style::Change);
void popParent();
void popParentsToDepth(unsigned depth);

@@ -44,6 +44,16 @@ RenderTreeUpdater::GeneratedContent::GeneratedContent(RenderTreeUpdater& updater
{
}

void RenderTreeUpdater::GeneratedContent::updateBeforePseudoElement(Element& element)
{
updatePseudoElement(element, BEFORE);
}

void RenderTreeUpdater::GeneratedContent::updateAfterPseudoElement(Element& element)
{
updatePseudoElement(element, AFTER);
}

void RenderTreeUpdater::GeneratedContent::updateRemainingQuotes()
{
if (!m_updater.renderView().hasQuotesNeedingUpdate())
@@ -92,14 +102,14 @@ static void updateStyleForContentRenderers(RenderElement& renderer)
}
}

void RenderTreeUpdater::GeneratedContent::updatePseudoElement(Element& current, const std::optional<Style::ElementUpdate>& update, PseudoId pseudoId)
void RenderTreeUpdater::GeneratedContent::updatePseudoElement(Element& current, PseudoId pseudoId)
{
PseudoElement* pseudoElement = pseudoId == BEFORE ? current.beforePseudoElement() : current.afterPseudoElement();

if (auto* renderer = pseudoElement ? pseudoElement->renderer() : nullptr)
m_updater.renderTreePosition().invalidateNextSibling(*renderer);

if (!needsPseudoElement(current, update)) {
if (!needsPseudoElement(current, pseudoId)) {
if (pseudoElement) {
if (pseudoId == BEFORE)
current.clearBeforePseudoElement();
@@ -115,23 +125,28 @@ void RenderTreeUpdater::GeneratedContent::updatePseudoElement(Element& current,
pseudoElement = newPseudoElement.get();
}

if (update->change == Style::NoChange)
auto newStyle = RenderStyle::clonePtr(*current.renderer()->getCachedPseudoStyle(pseudoId, &current.renderer()->style()));

auto elementUpdate = Style::TreeResolver::createAnimatedElementUpdate(WTFMove(newStyle), *pseudoElement, Style::NoChange);

if (elementUpdate.change == Style::NoChange)
return;

if (newPseudoElement) {
InspectorInstrumentation::pseudoElementCreated(m_updater.m_document.page(), *newPseudoElement);
if (pseudoId == BEFORE)
current.setBeforePseudoElement(newPseudoElement.releaseNonNull());
else
current.setAfterPseudoElement(newPseudoElement.releaseNonNull());
}

m_updater.updateElementRenderer(*pseudoElement, *update);
m_updater.updateElementRenderer(*pseudoElement, elementUpdate);

auto* pseudoRenderer = pseudoElement->renderer();
if (!pseudoRenderer)
return;

if (update->change == Style::Detach)
if (elementUpdate.change == Style::Detach)
createContentRenderers(*pseudoRenderer);
else
updateStyleForContentRenderers(*pseudoRenderer);
@@ -144,14 +159,13 @@ void RenderTreeUpdater::GeneratedContent::updatePseudoElement(Element& current,
ListItem::updateMarker(downcast<RenderListItem>(*pseudoRenderer));
}

bool RenderTreeUpdater::GeneratedContent::needsPseudoElement(Element& current, const std::optional<Style::ElementUpdate>& update)
bool RenderTreeUpdater::GeneratedContent::needsPseudoElement(Element& current, PseudoId pseudoId)
{
ASSERT(!current.isPseudoElement());
if (!update)
return false;
if (!current.renderer() || !current.renderer()->canHaveGeneratedChildren())
return false;
if (!pseudoElementRendererIsNeeded(update->style.get()))
if (current.isPseudoElement())
return false;
if (!pseudoElementRendererIsNeeded(current.renderer()->getCachedPseudoStyle(pseudoId)))
return false;
return true;
}
@@ -37,13 +37,15 @@ class RenderTreeUpdater::GeneratedContent {
public:
GeneratedContent(RenderTreeUpdater&);

void updatePseudoElement(Element&, const std::optional<Style::ElementUpdate>&, PseudoId);
void updateBeforePseudoElement(Element&);
void updateAfterPseudoElement(Element&);
void updateRemainingQuotes();

private:
void updatePseudoElement(Element&, PseudoId);
void updateQuotesUpTo(RenderQuote*);

static bool needsPseudoElement(Element&, const std::optional<Style::ElementUpdate>&);
static bool needsPseudoElement(Element&, PseudoId);

RenderTreeUpdater& m_updater;
RenderQuote* m_previousUpdatedQuote { nullptr };
@@ -174,7 +174,7 @@ static const RenderStyle* renderOrDisplayContentsStyle(const Element& element)
return nullptr;
}

ElementUpdates TreeResolver::resolveElement(Element& element)
ElementUpdate TreeResolver::resolveElement(Element& element)
{
if (m_didSeePendingStylesheet && !element.renderer() && !m_document.isIgnoringPendingStylesheets()) {
m_document.setHasNodesWithMissingStyle();
@@ -218,29 +218,7 @@ ElementUpdates TreeResolver::resolveElement(Element& element)
update.change = Detach;
}

auto beforeUpdate = resolvePseudoStyle(element, update, BEFORE);
auto afterUpdate = resolvePseudoStyle(element, update, AFTER);

return { WTFMove(update), WTFMove(beforeUpdate), WTFMove(afterUpdate) };
}

ElementUpdate TreeResolver::resolvePseudoStyle(Element& element, const ElementUpdate& elementUpdate, PseudoId pseudoId)
{
if (!elementUpdate.style->hasPseudoStyle(pseudoId))
return { };
auto pseudoStyle = scope().styleResolver.pseudoStyleForElement(element, { pseudoId }, *elementUpdate.style);

PseudoElement* pseudoElement = pseudoId == BEFORE ? element.beforePseudoElement() : element.afterPseudoElement();
if (!pseudoElement) {
auto newPseudoElement = PseudoElement::create(element, pseudoId);
pseudoElement = newPseudoElement.ptr();
if (pseudoId == BEFORE)
element.setBeforePseudoElement(WTFMove(newPseudoElement));
else
element.setAfterPseudoElement(WTFMove(newPseudoElement));
}

return createAnimatedElementUpdate(WTFMove(pseudoStyle), *pseudoElement, elementUpdate.change);
return update;
}

const RenderStyle* TreeResolver::parentBoxStyle() const
@@ -259,7 +237,7 @@ const RenderStyle* TreeResolver::parentBoxStyle() const

ElementUpdate TreeResolver::createAnimatedElementUpdate(std::unique_ptr<RenderStyle> newStyle, Element& element, Change parentChange)
{
auto& animationController = m_document.frame()->animation();
auto& animationController = element.document().frame()->animation();

auto* oldStyle = renderOrDisplayContentsStyle(element);
auto animationUpdate = animationController.updateAnimations(element, *newStyle, oldStyle);
@@ -447,19 +425,19 @@ void TreeResolver::resolveComposedTree()
if (element.hasCustomStyleResolveCallbacks())
element.willRecalcStyle(parent.change);

auto elementUpdates = resolveElement(element);
auto elementUpdate = resolveElement(element);

if (element.hasCustomStyleResolveCallbacks())
element.didRecalcStyle(elementUpdates.update.change);
element.didRecalcStyle(elementUpdate.change);

style = elementUpdates.update.style.get();
change = elementUpdates.update.change;
style = elementUpdate.style.get();
change = elementUpdate.change;

if (affectedByPreviousSibling && change != Detach)
change = Force;

if (elementUpdates.update.style)
m_update->addElement(element, parent.element, WTFMove(elementUpdates));
if (elementUpdate.style)
m_update->addElement(element, parent.element, WTFMove(elementUpdate));

clearNeedsStyleResolution(element);
}
@@ -51,13 +51,13 @@ class TreeResolver {

std::unique_ptr<Update> resolve();

static ElementUpdate createAnimatedElementUpdate(std::unique_ptr<RenderStyle>, Element&, Change parentChange);

private:
std::unique_ptr<RenderStyle> styleForElement(Element&, const RenderStyle& inheritedStyle);

void resolveComposedTree();
ElementUpdates resolveElement(Element&);
ElementUpdate createAnimatedElementUpdate(std::unique_ptr<RenderStyle>, Element&, Change parentChange);
ElementUpdate resolvePseudoStyle(Element&, const ElementUpdate&, PseudoId);
ElementUpdate resolveElement(Element&);

struct Scope : RefCounted<Scope> {
StyleResolver& styleResolver;

0 comments on commit 365528c

Please sign in to comment.