Skip to content

Commit

Permalink
Revert "Connect css counters to style containment scope tree and remo…
Browse files Browse the repository at this point in the history 
…ve old code"

This reverts commit 32bca80.

Reason for revert: Crash reports

Original change's description:
> Connect css counters to style containment scope tree and remove old code
>
> This is stage 2 of CSS Counters projects. The idea is to remove old
> style containment code from LayoutCounter and CounterNode and instead
> connect them to StyleContainmentScopeTree.
>
> Design doc:
> https://docs.google.com/document/d/1lc7OouWnelduHuRBxsQ9rmSS85q7E-h42NrOpKVkkoU
>
> Bug: 990657
> Change-Id: I9db3ee4834dc388b137306927f123b2c0813cccf
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4905764
> Reviewed-by: Vladimir Levin <vmpstr@chromium.org>
> Commit-Queue: Daniil Sakhapov <sakhapov@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1209472}

Bug: 990657
Change-Id: I2e10e705677040c3cd392b18dbe69009fc09d383
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4975124
Commit-Queue: Daniil Sakhapov <sakhapov@chromium.org>
Reviewed-by: Vladimir Levin <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1215421}
  • Loading branch information
@danielsakhapov
danielsakhapov authored and Chromium LUCI CQ committed Oct 26, 2023
1 parent e4060c2 commit dc7fa39
Show file tree
Hide file tree
Showing 19 changed files with 1,625 additions and 253 deletions.
3 changes: 0 additions & 3 deletions third_party/blink/renderer/core/css/counters_scope.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,9 +272,6 @@ bool CountersScope::UpdateOwnCounters(bool force_update,
}
counter->SetValueBefore(value);
counter->CalculateValueAfter(should_reset_increment, num_counters_in_scope);
if (auto* layout_counter = DynamicTo<LayoutCounter>(counter->Owner())) {
layout_counter->UpdateCounter();
}
if (!counter->HasUseType()) {
should_reset_increment = false;
}
Expand Down
3 changes: 0 additions & 3 deletions third_party/blink/renderer/core/css/counters_scope_tree.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -475,15 +475,12 @@ void CountersScopeTree::RemoveCounterFromScope(CounterNode& counter,
}

void CountersScopeTree::CreateCounterForLayoutCounter(LayoutCounter& counter) {
CHECK(!counter.GetCounterNode());
CounterNode* counter_node = MakeGarbageCollected<CounterNode>(counter, 0u, 0);
AttachCounter(*counter_node, counter.Identifier());
counter.SetCounterNode(counter_node);
}

void CountersScopeTree::RemoveCounterForLayoutCounter(LayoutCounter& counter) {
CounterNode* counter_node = counter.GetCounterNode();
counter.SetCounterNode(nullptr);
CHECK(counter_node);
CHECK(counter_node->HasUseType());
CountersScope* scope = counter_node->Scope();
Expand Down
5 changes: 2 additions & 3 deletions third_party/blink/renderer/core/css/style_containment_scope.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_CSS_STYLE_CONTAINMENT_SCOPE_H_
#define THIRD_PARTY_BLINK_RENDERER_CORE_CSS_STYLE_CONTAINMENT_SCOPE_H_

#include "third_party/blink/renderer/core/core_export.h"
#include "third_party/blink/renderer/core/dom/element.h"
#include "third_party/blink/renderer/core/layout/layout_quote.h"

Expand Down Expand Up @@ -34,10 +33,10 @@ class StyleContainmentScope final
CORE_EXPORT CountersScope* FindCountersScopeForElement(
const Element&,
const AtomicString&) const;
void CreateCounterNodesForLayoutObject(LayoutObject&);
CORE_EXPORT void CreateCounterNodesForLayoutObject(LayoutObject&);
void CreateCounterNodeForLayoutObject(LayoutObject& object,
const AtomicString& identifier);
void CreateCounterNodeForLayoutCounter(LayoutCounter&);
CORE_EXPORT void CreateCounterNodeForLayoutCounter(LayoutCounter&);
void CreateListItemCounterNodeForLayoutObject(LayoutObject&);
void RemoveCounterNodeForLayoutCounter(LayoutCounter&);
void ReparentCountersToStyleScope(StyleContainmentScope&);
Expand Down
135 changes: 135 additions & 0 deletions third_party/blink/renderer/core/css/style_containment_scope_tree_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "third_party/blink/renderer/core/css/style_containment_scope_tree.h"

#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/renderer/core/css/counters_scope_tree.h"
#include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/core/html/html_element.h"
#include "third_party/blink/renderer/core/layout/counter_node.h"
Expand All @@ -16,6 +17,37 @@ namespace blink {

class StyleContainmentScopeTreeTest : public RenderingTest {
public:
void CreateCounterNodeForLayoutObject(const char* id) {
StyleContainmentScopeTree& tree =
GetDocument().GetStyleEngine().EnsureStyleContainmentScopeTree();
LayoutObject* object = GetLayoutObjectByElementId(id);
Element* element = To<Element>(object->GetNode());
if (AtomicString(id).Contains("use")) {
object = object->SlowFirstChild()->SlowFirstChild();
element = To<Element>(object->Parent()->GetNode());
}
StyleContainmentScope* scope =
tree.FindOrCreateEnclosingScopeForElement(*element);
scope->CreateCounterNodesForLayoutObject(*object);
tree.UpdateOutermostCountersDirtyScope(scope);
}

void AttachLayoutCounter(const char* id) {
StyleContainmentScopeTree& tree =
GetDocument().GetStyleEngine().EnsureStyleContainmentScopeTree();
LayoutObject* object = GetLayoutObjectByElementId(id);
LayoutObject* it = object->NextInPreOrder();
for (; it && !it->IsCounter(); it = it->NextInPreOrder(object)) {
}
LayoutCounter* layout_counter = DynamicTo<LayoutCounter>(it);
Element& element = To<Element>(*layout_counter->Parent()->GetNode());
ASSERT_TRUE(layout_counter);
StyleContainmentScope* scope =
tree.FindOrCreateEnclosingScopeForElement(element);
scope->CreateCounterNodeForLayoutCounter(*layout_counter);
tree.UpdateOutermostCountersDirtyScope(scope);
}

CountersScope* GetCountersScopeById(const char* id, const char* identifier) {
StyleContainmentScopeTree& tree =
GetDocument().GetStyleEngine().EnsureStyleContainmentScopeTree();
Expand Down Expand Up @@ -47,6 +79,15 @@ TEST_F(StyleContainmentScopeTreeTest, ManualInsertion) {
)HTML");
GetDocument().UpdateStyleAndLayoutTree();

CreateCounterNodeForLayoutObject("counter-set");
CreateCounterNodeForLayoutObject("counter-increment-1");
CreateCounterNodeForLayoutObject("counter-increment-2");
AttachLayoutCounter("counter-use");

StyleContainmentScopeTree& tree =
GetDocument().GetStyleEngine().EnsureStyleContainmentScopeTree();
tree.UpdateCounters();

CountersScope* scope = GetCountersScopeById("counter-use", "counter");

EXPECT_EQ(scope->Counters().size(), 4u);
Expand All @@ -69,6 +110,16 @@ TEST_F(StyleContainmentScopeTreeTest, ManualInsertionStyleContainment) {
)HTML");
GetDocument().UpdateStyleAndLayoutTree();

CreateCounterNodeForLayoutObject("counter-increment-1");
CreateCounterNodeForLayoutObject("counter-increment-2");
AttachLayoutCounter("counter-use-1");
AttachLayoutCounter("counter-use-2");
CreateCounterNodeForLayoutObject("counter-set");

StyleContainmentScopeTree& tree =
GetDocument().GetStyleEngine().EnsureStyleContainmentScopeTree();
tree.UpdateCounters();

CountersScope* scope = GetCountersScopeById("counter-use-2", "counter");
EXPECT_EQ(scope->Counters().size(), 2u);
EXPECT_EQ(scope->Counters().back()->ValueBefore(), 1);
Expand Down Expand Up @@ -103,6 +154,19 @@ TEST_F(StyleContainmentScopeTreeTest, ManualInsertionStyleContainmentComplex) {
)HTML");
GetDocument().UpdateStyleAndLayoutTree();

// Add counters in wrong order to see that the result scope tree is invariant
// to order in which the elements are added.
AttachLayoutCounter("counter-use-2");
AttachLayoutCounter("counter-use-1");
CreateCounterNodeForLayoutObject("counter-increment-2");
CreateCounterNodeForLayoutObject("counter-increment-1");
CreateCounterNodeForLayoutObject("counter-reset");
CreateCounterNodeForLayoutObject("counter-set");

StyleContainmentScopeTree& tree =
GetDocument().GetStyleEngine().EnsureStyleContainmentScopeTree();
tree.UpdateCounters();

CountersScope* scope = GetCountersScopeById("counter-use-2", "counter");
EXPECT_EQ(scope->Counters().size(), 2u);
EXPECT_EQ(scope->Counters().back()->ValueBefore(), 1);
Expand Down Expand Up @@ -145,6 +209,24 @@ TEST_F(StyleContainmentScopeTreeTest,
)HTML");
GetDocument().UpdateStyleAndLayoutTree();

AttachLayoutCounter("counter-use-1");
AttachLayoutCounter("counter-use-2");
CreateCounterNodeForLayoutObject("counter-increment-1");
CreateCounterNodeForLayoutObject("counter-increment-2");
CreateCounterNodeForLayoutObject("counter-increment-3");
AttachLayoutCounter("counter-use-3");
AttachLayoutCounter("counter-use-4");
CreateCounterNodeForLayoutObject("counter-increment-4");
CreateCounterNodeForLayoutObject("counter-increment-5");
CreateCounterNodeForLayoutObject("counter-increment-6");
AttachLayoutCounter("counter-use-5");
CreateCounterNodeForLayoutObject("counter-reset-1");
CreateCounterNodeForLayoutObject("counter-reset-2");

StyleContainmentScopeTree& tree =
GetDocument().GetStyleEngine().EnsureStyleContainmentScopeTree();
tree.UpdateCounters();

CountersScope* scope = GetCountersScopeById("counter-use-2", "counter");
EXPECT_EQ(scope->Counters().size(), 2u);
EXPECT_EQ(scope->Counters().back()->ValueBefore(), 2);
Expand Down Expand Up @@ -203,6 +285,26 @@ TEST_F(StyleContainmentScopeTreeTest,
)HTML");
GetDocument().UpdateStyleAndLayoutTree();

// Add counters in wrong order to see that the result scope tree is invariant
// to order in which the elements are added.
AttachLayoutCounter("counter-use-5");
CreateCounterNodeForLayoutObject("counter-increment-4");
CreateCounterNodeForLayoutObject("counter-increment-5");
AttachLayoutCounter("counter-use-2");
CreateCounterNodeForLayoutObject("counter-increment-6");
CreateCounterNodeForLayoutObject("counter-increment-1");
CreateCounterNodeForLayoutObject("counter-increment-2");
AttachLayoutCounter("counter-use-4");
CreateCounterNodeForLayoutObject("counter-increment-3");
CreateCounterNodeForLayoutObject("counter-reset-1");
AttachLayoutCounter("counter-use-3");
CreateCounterNodeForLayoutObject("counter-reset-2");
AttachLayoutCounter("counter-use-1");

StyleContainmentScopeTree& tree =
GetDocument().GetStyleEngine().EnsureStyleContainmentScopeTree();
tree.UpdateCounters();

CountersScope* scope = GetCountersScopeById("counter-use-5", "counter");
EXPECT_EQ(scope->Counters().size(), 1u);
EXPECT_EQ(scope->Counters().back()->ValueAfter(), 1);
Expand Down Expand Up @@ -244,6 +346,18 @@ TEST_F(StyleContainmentScopeTreeTest, ManualInsertionSelfStyleContainment) {
)HTML");
GetDocument().UpdateStyleAndLayoutTree();

for (const AtomicString& letter : {AtomicString("A"), AtomicString("B")}) {
for (int i = 1; i <= 3; ++i) {
AtomicString id = letter + char(i + '0');
CreateCounterNodeForLayoutObject(id.Ascii().c_str());
AttachLayoutCounter(id.Ascii().c_str());
}
}

StyleContainmentScopeTree& tree =
GetDocument().GetStyleEngine().EnsureStyleContainmentScopeTree();
tree.UpdateCounters();

for (const AtomicString& letter : {AtomicString("A"), AtomicString("B")}) {
for (int i = 1; i <= 3; ++i) {
AtomicString id = letter + char(i + '0');
Expand Down Expand Up @@ -284,6 +398,17 @@ TEST_F(StyleContainmentScopeTreeTest, ManualInsertionSanityCheck) {
)HTML");
GetDocument().UpdateStyleAndLayoutTree();

AttachLayoutCounter("counter-use-1");
AttachLayoutCounter("counter-use-2");
AttachLayoutCounter("counter-use-3");
AttachLayoutCounter("counter-use-4");
CreateCounterNodeForLayoutObject("counter-reset-1");
CreateCounterNodeForLayoutObject("counter-reset-2");

StyleContainmentScopeTree& tree =
GetDocument().GetStyleEngine().EnsureStyleContainmentScopeTree();
tree.UpdateCounters();

CountersScope* scope = GetCountersScopeById("counter-use-1", "counter");
EXPECT_EQ(scope->Counters().size(), 3u);
EXPECT_EQ(scope->Counters().back()->ValueBefore(), 5);
Expand Down Expand Up @@ -313,6 +438,16 @@ TEST_F(StyleContainmentScopeTreeTest,
)HTML");
GetDocument().UpdateStyleAndLayoutTree();

AttachLayoutCounter("counter-use-1");
AttachLayoutCounter("counter-use-2");
AttachLayoutCounter("counter-use-3");
AttachLayoutCounter("counter-use-4");
CreateCounterNodeForLayoutObject("counter-reset-1");

StyleContainmentScopeTree& tree =
GetDocument().GetStyleEngine().EnsureStyleContainmentScopeTree();
tree.UpdateCounters();

CountersScope* scope = GetCountersScopeById("counter-use-1", "counter");
EXPECT_EQ(scope->Counters().size(), 3u);
EXPECT_EQ(scope->Counters().back()->ValueBefore(), 5);
Expand Down
2 changes: 0 additions & 2 deletions third_party/blink/renderer/core/css/style_engine.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3371,7 +3371,6 @@ void StyleEngine::UpdateStyleAndLayoutTreeForContainer(
// Update quotes only if there are any scopes marked dirty.
if (StyleContainmentScopeTree* tree = GetStyleContainmentScopeTree()) {
tree->UpdateQuotes();
tree->UpdateCounters();
}
if (container == GetDocument().documentElement()) {
// If the container is the root element, there may be body styles which have
Expand Down Expand Up @@ -3549,7 +3548,6 @@ void StyleEngine::UpdateStyleAndLayoutTree() {
// Update quotes only if there are any scopes marked dirty.
if (StyleContainmentScopeTree* tree = GetStyleContainmentScopeTree()) {
tree->UpdateQuotes();
tree->UpdateCounters();
}
} else {
style_recalc_root_.Clear();
Expand Down
9 changes: 0 additions & 9 deletions third_party/blink/renderer/core/dom/pseudo_element.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
#include "third_party/blink/renderer/core/html/forms/html_input_element.h"
#include "third_party/blink/renderer/core/input_type_names.h"
#include "third_party/blink/renderer/core/layout/generated_children.h"
#include "third_party/blink/renderer/core/layout/layout_counter.h"
#include "third_party/blink/renderer/core/layout/layout_object.h"
#include "third_party/blink/renderer/core/layout/layout_quote.h"
#include "third_party/blink/renderer/core/layout/list/list_marker.h"
Expand Down Expand Up @@ -317,14 +316,6 @@ void PseudoElement::AttachLayoutTree(AttachContext& context) {
scope->AttachQuote(*To<LayoutQuote>(child));
tree.UpdateOutermostQuotesDirtyScope(scope);
}
if (auto* counter = DynamicTo<LayoutCounter>(child)) {
StyleContainmentScopeTree& tree =
GetDocument().GetStyleEngine().EnsureStyleContainmentScopeTree();
StyleContainmentScope* scope =
tree.FindOrCreateEnclosingScopeForElement(*this);
scope->CreateCounterNodeForLayoutCounter(*counter);
tree.UpdateOutermostCountersDirtyScope(scope);
}
} else {
child->Destroy();
}
Expand Down

0 comments on commit dc7fa39

Please sign in to comment.