Skip to content

GitLab

T
tangled
Commit 66094461 authored by Rune Lillesveen's avatar Rune Lillesveen Committed by Commit Bot
Browse files
Options

Don't re-attach ::first-letter during style recalc. 

Update (create or destroy) the FirstLetterPseudoElement and its style
at the point where the block level element's layout sub-tree is up-to-
date. This means we will always attach the layout tree for the
::first-letter element in Element::AttachLayoutTree, but the pseudo
element and its style may be created/updated at style recalc time,
layout tree rebuild time, or layout tree attachment time depending on
when we know what will be the LayoutText from which we will format the
first letter if any.

UpdateFirstLetterPseudoElement is split out from UpdatePseudoElement to
make the code easier to read as the former case has some exceptional
cases.

We no longer use the pseudo style cache for ::first-letter as we will
now compute the style only once per pass with the correct inheritance
parent.

Bug: 847218
Change-Id: I7a1e2a60122891fa38998ff85e566bec0a38b513
Reviewed-on: https://chromium-review.googlesource.com/1155591Reviewed-by: default avatarAnders Ruud <andruud@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581320}
parent 616ae14e
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 142 additions and 146 deletions
third_party/WebKit/LayoutTests/external/wpt/css/css-pseudo/first-letter-and-sibling-display-change.html 0 → 100644
View file @ 66094461
<!DOCTYPE html>
<title>CSS Test: Changing ::first-letter color while sibling changes display type.</title>
<link rel="author" title="Rune Lillesveen" href="mailto:futhark@chromium.org">
<link rel="match" href="first-letter-block-to-inline-ref.html">
<link rel="help" href="https://drafts.csswg.org/css-pseudo-4/#first-letter-pseudo">
<style>
div { color: green }
div::first-letter { color: red }
.green::first-letter { color: green }
</style>
<div>This text should be green.<span></span></div>
<script>
document.body.offsetTop;
document.querySelector("span").style.display = "block";
document.querySelector("div").className = "green";
</script>
third_party/WebKit/LayoutTests/fast/css/dynamic-class-pseudo-elements-expected.txt
View file @ 66094461
...@@ -13,7 +13,7 @@ PASS getComputedStyle(r3, '::first-line').backgroundColor is transparent ...@@ -13,7 +13,7 @@ PASS getComputedStyle(r3, '::first-line').backgroundColor is transparent
PASS internals.updateStyleAndReturnAffectedElementCount() is 9 PASS internals.updateStyleAndReturnAffectedElementCount() is 9
PASS getComputedStyle(r3, '::first-line').backgroundColor is green PASS getComputedStyle(r3, '::first-line').backgroundColor is green
PASS getComputedStyle(r4, '::first-letter').backgroundColor is transparent PASS getComputedStyle(r4, '::first-letter').backgroundColor is transparent
PASS internals.updateStyleAndReturnAffectedElementCount() is 10 PASS internals.updateStyleAndReturnAffectedElementCount() is 9
PASS getComputedStyle(r4, '::first-letter').backgroundColor is green PASS getComputedStyle(r4, '::first-letter').backgroundColor is green
PASS successfullyParsed is true PASS successfullyParsed is true
......
third_party/WebKit/LayoutTests/fast/css/dynamic-class-pseudo-elements.html
View file @ 66094461
...@@ -81,8 +81,7 @@ forceLayout(); ...@@ -81,8 +81,7 @@ forceLayout();
t4.className = "a4"; t4.className = "a4";
if (window.internals) if (window.internals)
shouldBe("internals.updateStyleAndReturnAffectedElementCount()", "10"); shouldBe("internals.updateStyleAndReturnAffectedElementCount()", "9");
document.body.offsetLeft; // workaround for issue 351308
shouldBe("getComputedStyle(r4, '::first-letter').backgroundColor", "green"); shouldBe("getComputedStyle(r4, '::first-letter').backgroundColor", "green");
</script> </script>
third_party/blink/renderer/core/dom/element.h
View file @ 66094461
...@@ -985,7 +985,14 @@ class CORE_EXPORT Element : public ContainerNode { ...@@ -985,7 +985,14 @@ class CORE_EXPORT Element : public ContainerNode {
const Node* node_after_change); const Node* node_after_change);
void UpdatePseudoElement(PseudoId, StyleRecalcChange); void UpdatePseudoElement(PseudoId, StyleRecalcChange);
bool UpdateFirstLetter(Element*);
enum class StyleUpdatePhase {
kRecalc,
kRebuildLayoutTree,
kAttachLayoutTree,
};
void UpdateFirstLetterPseudoElement(StyleUpdatePhase);
inline PseudoElement* CreatePseudoElementIfNeeded(PseudoId); inline PseudoElement* CreatePseudoElementIfNeeded(PseudoId);
void AttachPseudoElement(PseudoId, AttachContext&); void AttachPseudoElement(PseudoId, AttachContext&);
......
third_party/blink/renderer/core/dom/first_letter_pseudo_element.cc
View file @ 66094461
...@@ -26,6 +26,7 @@ ...@@ -26,6 +26,7 @@
#include "third_party/blink/renderer/core/css/style_change_reason.h" #include "third_party/blink/renderer/core/css/style_change_reason.h"
#include "third_party/blink/renderer/core/dom/element.h" #include "third_party/blink/renderer/core/dom/element.h"
#include "third_party/blink/renderer/core/dom/node_computed_style.h"
#include "third_party/blink/renderer/core/layout/generated_children.h" #include "third_party/blink/renderer/core/layout/generated_children.h"
#include "third_party/blink/renderer/core/layout/layout_object.h" #include "third_party/blink/renderer/core/layout/layout_object.h"
#include "third_party/blink/renderer/core/layout/layout_object_inlines.h" #include "third_party/blink/renderer/core/layout/layout_object_inlines.h"
...@@ -271,27 +272,16 @@ void FirstLetterPseudoElement::DetachLayoutTree(const AttachContext& context) { ...@@ -271,27 +272,16 @@ void FirstLetterPseudoElement::DetachLayoutTree(const AttachContext& context) {
PseudoElement::DetachLayoutTree(context); PseudoElement::DetachLayoutTree(context);
} }
ComputedStyle* FirstLetterPseudoElement::StyleForFirstLetter( scoped_refptr<ComputedStyle>
LayoutObject* layout_object_container) { FirstLetterPseudoElement::CustomStyleForLayoutObject() {
DCHECK(layout_object_container); LayoutObject* first_letter_text =
FirstLetterPseudoElement::FirstLetterTextLayoutObject(*this);
LayoutObject* style_container = if (!first_letter_text)
ParentOrShadowHostElement()->GetLayoutObject(); return nullptr;
DCHECK(style_container); DCHECK(first_letter_text->Parent());
return ParentOrShadowHostElement()->StyleForPseudoElement(
// We always force the pseudo style to recompute as the first-letter style PseudoStyleRequest(GetPseudoId()),
// computed by the style container may not have taken the layoutObjects styles first_letter_text->Parent()->FirstLineStyle());
// into account.
style_container->MutableStyle()->RemoveCachedPseudoStyle(
kPseudoIdFirstLetter);
ComputedStyle* pseudo_style = style_container->GetCachedPseudoStyle(
kPseudoIdFirstLetter, layout_object_container->FirstLineStyle());
DCHECK(pseudo_style);
pseudo_style->UpdateIsStackingContext(false /* is_document_element */,
false /* is_in_top_layer */,
false /* is_svg_stacking */);
return pseudo_style;
} }
void FirstLetterPseudoElement::AttachFirstLetterTextLayoutObjects(LayoutText* first_letter_text) { void FirstLetterPseudoElement::AttachFirstLetterTextLayoutObjects(LayoutText* first_letter_text) {
...@@ -304,12 +294,6 @@ void FirstLetterPseudoElement::AttachFirstLetterTextLayoutObjects(LayoutText* fi ...@@ -304,12 +294,6 @@ void FirstLetterPseudoElement::AttachFirstLetterTextLayoutObjects(LayoutText* fi
String old_text = first_letter_text->IsTextFragment() ? ToLayoutTextFragment(first_letter_text)->CompleteText() : first_letter_text->OriginalText(); String old_text = first_letter_text->IsTextFragment() ? ToLayoutTextFragment(first_letter_text)->CompleteText() : first_letter_text->OriginalText();
DCHECK(old_text.Impl()); DCHECK(old_text.Impl());
// :first-letter inherits from the parent of the text. It may not be
// this->Parent() when e.g., <div><span>text</span></div>.
ComputedStyle* pseudo_style =
StyleForFirstLetter(first_letter_text->Parent());
GetLayoutObject()->SetStyle(pseudo_style);
// FIXME: This would already have been calculated in firstLetterLayoutObject. // FIXME: This would already have been calculated in firstLetterLayoutObject.
// Can we pass the length through? // Can we pass the length through?
unsigned length = FirstLetterPseudoElement::FirstLetterLength(old_text); unsigned length = FirstLetterPseudoElement::FirstLetterLength(old_text);
...@@ -344,7 +328,7 @@ void FirstLetterPseudoElement::AttachFirstLetterTextLayoutObjects(LayoutText* fi ...@@ -344,7 +328,7 @@ void FirstLetterPseudoElement::AttachFirstLetterTextLayoutObjects(LayoutText* fi
LayoutTextFragment* letter = LayoutTextFragment* letter =
LayoutTextFragment::CreateAnonymous(*this, old_text.Impl(), 0, length); LayoutTextFragment::CreateAnonymous(*this, old_text.Impl(), 0, length);
letter->SetFirstLetterPseudoElement(this); letter->SetFirstLetterPseudoElement(this);
letter->SetStyle(pseudo_style); letter->SetStyle(MutableComputedStyle());
GetLayoutObject()->AddChild(letter); GetLayoutObject()->AddChild(letter);
first_letter_text->Destroy(); first_letter_text->Destroy();
...@@ -355,30 +339,15 @@ void FirstLetterPseudoElement::DidRecalcStyle(StyleRecalcChange) { ...@@ -355,30 +339,15 @@ void FirstLetterPseudoElement::DidRecalcStyle(StyleRecalcChange) {
if (!layout_object) if (!layout_object)
return; return;
// :first-letter inherits from the parent of the text. It may not be // The layout objects inside pseudo elements are anonymous so they don't get
// this->Parent() when e.g., <div><span>text</span></div>. // notified of RecalcStyle and must have the style propagated downward
DCHECK(remaining_text_layout_object_); // manually similar to LayoutObject::PropagateStyleToAnonymousChildren.
ComputedStyle* pseudo_style =
StyleForFirstLetter(remaining_text_layout_object_->Parent());
DCHECK(pseudo_style);
// TODO(kojii): While setting to GetLayoutObject() looks correct all the time,
// as we do so in AttachFirstLetterTextLayoutObjects(), it is required only
// when inline box has text children, and can break layout tree when changing
// :first-letter to floats. The check in Element::UpdatePseudoElement() does
// not catch all such cases.
if (!pseudo_style->IsDisplayBlockContainer())
layout_object->SetStyle(pseudo_style);
// The layoutObjects inside pseudo elements are anonymous so they don't get
// notified of recalcStyle and must have
// the style propagated downward manually similar to
// LayoutObject::propagateStyleToAnonymousChildren.
for (LayoutObject* child = layout_object->NextInPreOrder(layout_object); for (LayoutObject* child = layout_object->NextInPreOrder(layout_object);
child; child = child->NextInPreOrder(layout_object)) { child; child = child->NextInPreOrder(layout_object)) {
// We need to re-calculate the correct style for the first letter element // We need to re-calculate the correct style for the first letter element
// and then apply that to the container and the text fragment inside. // and then apply that to the container and the text fragment inside.
if (child->Style()->StyleType() == kPseudoIdFirstLetter) { if (child->Style()->StyleType() == kPseudoIdFirstLetter) {
child->SetPseudoStyle(pseudo_style); child->SetPseudoStyle(layout_object->MutableStyle());
continue; continue;
} }
......
third_party/blink/renderer/core/dom/first_letter_pseudo_element.h
View file @ 66094461
...@@ -33,7 +33,6 @@ ...@@ -33,7 +33,6 @@
namespace blink { namespace blink {
class Element; class Element;
class LayoutObject;
class LayoutText; class LayoutText;
class LayoutTextFragment; class LayoutTextFragment;
...@@ -61,10 +60,10 @@ class CORE_EXPORT FirstLetterPseudoElement final : public PseudoElement { ...@@ -61,10 +60,10 @@ class CORE_EXPORT FirstLetterPseudoElement final : public PseudoElement {
private: private:
explicit FirstLetterPseudoElement(Element*); explicit FirstLetterPseudoElement(Element*);
scoped_refptr<ComputedStyle> CustomStyleForLayoutObject() override;
void DidRecalcStyle(StyleRecalcChange) override; void DidRecalcStyle(StyleRecalcChange) override;
void AttachFirstLetterTextLayoutObjects(LayoutText* first_letter_text); void AttachFirstLetterTextLayoutObjects(LayoutText* first_letter_text);
ComputedStyle* StyleForFirstLetter(LayoutObject*);
LayoutTextFragment* remaining_text_layout_object_; LayoutTextFragment* remaining_text_layout_object_;
DISALLOW_COPY_AND_ASSIGN(FirstLetterPseudoElement); DISALLOW_COPY_AND_ASSIGN(FirstLetterPseudoElement);
......
third_party/blink/renderer/core/dom/layout_tree_builder.cc
View file @ 66094461
...@@ -131,11 +131,7 @@ bool LayoutTreeBuilderForElement::ShouldCreateLayoutObject() const { ...@@ -131,11 +131,7 @@ bool LayoutTreeBuilderForElement::ShouldCreateLayoutObject() const {
} }
ComputedStyle& LayoutTreeBuilderForElement::Style() const { ComputedStyle& LayoutTreeBuilderForElement::Style() const {
if (!style_) { DCHECK(style_);
DCHECK(!node_->GetNonAttachedStyle());
DCHECK(node_->IsFirstLetterPseudoElement());
style_ = node_->StyleForLayoutObject();
}
return *style_; return *style_;
} }
......
third_party/blink/renderer/core/style/computed_style.h
View file @ 66094461
...@@ -2220,9 +2220,7 @@ class ComputedStyle : public ComputedStyleBase, ...@@ -2220,9 +2220,7 @@ class ComputedStyle : public ComputedStyleBase,
InterpolationQuality GetInterpolationQuality() const; InterpolationQuality GetInterpolationQuality() const;
bool CanGeneratePseudoElement(PseudoId pseudo) const { bool CanGeneratePseudoElement(PseudoId pseudo) const {
// The first letter pseudo element has to look up the tree and see if any if (!HasPseudoStyle(pseudo))
// of the ancestors are first letter.
if (pseudo != kPseudoIdFirstLetter && !HasPseudoStyle(pseudo))
return false; return false;
if (Display() == EDisplay::kNone) if (Display() == EDisplay::kNone)
return false; return false;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment