[css-pseudo] Detect list style changes in the list item, not the marker

The list-style-type and list-style-image properties can affect the kind
of list marker that we have. However, what matters are the values on the
originating list item, not on the ::marker itself.

Right now the difference doesn't matter, since ::marker doesn't accept
these properties, and thus must be inherited. But in the future we might
remove the restrictions and accept arbitrary properties in ::marker.

This patch makes LayoutListMarker stop detecting changes in the list
properties. They are instead detected in LayoutListItem, which notifies
the marker, using a similar logic as for LayoutInsideListMarker and
LayoutOutideListMarker.

Also, the SetNeedsLayoutAndIntrinsicWidthsRecalcAndFullPaintInvalidation
call when list-style-type changes is now avoided if list-style-image is
a valid image, since then it overrides list-style-type. And there is no
need to check list-style-position, since it affects the display, thus
forcing a layout tree reattachment. So the layout invalidation reason
can be changed from kStyleChange to kListStyleTypeChange.

Bug: 457718
Change-Id: Ifde0068bbc3fbf8802c1c9c4fed13bc682b65d80
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2279775Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/master@{#786225}

third_party/blink/renderer/core/layout/layout_list_item.cc

@@ -56,17 +56,27 @@ void LayoutListItem::StyleDidChange(StyleDifference diff,
   }
 
   LayoutObject* marker = Marker();
-  ListMarker* list_marker = ListMarker::Get(marker);
-  if (!list_marker)
+  if (!marker)
     return;
 
-  list_marker->UpdateMarkerContentIfNeeded(*marker);
+  LayoutListMarker* legacy_marker = ToLayoutListMarkerOrNull(marker);
+  ListMarker* list_marker = legacy_marker ? nullptr : ListMarker::Get(marker);
+  DCHECK(legacy_marker || list_marker);
+
+  if (legacy_marker)
+    legacy_marker->UpdateMarkerImageIfNeeded(current_image);
+  else
+    list_marker->UpdateMarkerContentIfNeeded(*marker);
 
   if (old_style && (old_style->ListStyleType() != StyleRef().ListStyleType() ||
                     (StyleRef().ListStyleType() == EListStyleType::kString &&
                      old_style->ListStyleStringValue() !=
-                         StyleRef().ListStyleStringValue())))
-    list_marker->ListStyleTypeChanged(*marker);
+                         StyleRef().ListStyleStringValue()))) {
+    if (legacy_marker)
+      legacy_marker->ListStyleTypeChanged();
+    else
+      list_marker->ListStyleTypeChanged(*marker);
+  }
 }
 
 void LayoutListItem::InsertedIntoTree() {
@@ -86,8 +96,12 @@ void LayoutListItem::SubtreeDidChange() {
   if (!marker)
     return;
 
-  if (ListMarker* list_marker = ListMarker::Get(marker))
+  if (LayoutListMarker* legacy_marker = ToLayoutListMarkerOrNull(marker))
+    legacy_marker->UpdateMarkerImageIfNeeded(StyleRef().ListStyleImage());
+  else if (ListMarker* list_marker = ListMarker::Get(marker))
     list_marker->UpdateMarkerContentIfNeeded(*marker);
+  else
+    NOTREACHED();
 
   if (!UpdateMarkerLocation())
     return;

third_party/blink/renderer/core/layout/layout_list_marker.cc

@@ -75,29 +75,18 @@ LayoutSize LayoutListMarker::ImageBulletSize() const {
                         LayoutObject::ShouldRespectImageOrientation(this)));
 }
 
-void LayoutListMarker::StyleWillChange(StyleDifference diff,
-                                       const ComputedStyle& new_style) {
-  if (Style() &&
-      (new_style.ListStylePosition() != StyleRef().ListStylePosition() ||
-       new_style.ListStyleType() != StyleRef().ListStyleType() ||
-       (new_style.ListStyleType() == EListStyleType::kString &&
-        new_style.ListStyleStringValue() !=
-            StyleRef().ListStyleStringValue()))) {
-    SetNeedsLayoutAndIntrinsicWidthsRecalcAndFullPaintInvalidation(
-        layout_invalidation_reason::kStyleChange);
-  }
-
-  LayoutBox::StyleWillChange(diff, new_style);
+void LayoutListMarker::ListStyleTypeChanged() {
+  if (IsImage())
+    return;
+  SetNeedsLayoutAndIntrinsicWidthsRecalcAndFullPaintInvalidation(
+      layout_invalidation_reason::kListStyleTypeChange);
 }
 
-void LayoutListMarker::StyleDidChange(StyleDifference diff,
-                                      const ComputedStyle* old_style) {
-  LayoutBox::StyleDidChange(diff, old_style);
-
-  if (image_ != StyleRef().ListStyleImage()) {
+void LayoutListMarker::UpdateMarkerImageIfNeeded(StyleImage* image) {
+  if (image_ != image) {
     if (image_)
       image_->RemoveClient(this);
-    image_ = StyleRef().ListStyleImage();
+    image_ = image;
     if (image_)
       image_->AddClient(this);
   }

third_party/blink/renderer/core/layout/layout_list_marker.h

@@ -35,6 +35,8 @@ class LayoutListItem;
 // Used to layout a list item's marker with 'content: normal'.
 // The LayoutListMarker always has to be a child of a LayoutListItem.
 class CORE_EXPORT LayoutListMarker final : public LayoutBox {
+  friend class LayoutListItem;
+
  public:
   explicit LayoutListMarker(Element*);
   ~LayoutListMarker() override;
@@ -97,9 +99,8 @@ class CORE_EXPORT LayoutListMarker final : public LayoutBox {
   void UpdateMargins(LayoutUnit marker_inline_size);
   void UpdateContent();
 
-  void StyleWillChange(StyleDifference,
-                       const ComputedStyle& new_style) override;
-  void StyleDidChange(StyleDifference, const ComputedStyle* old_style) override;
+  void UpdateMarkerImageIfNeeded(StyleImage* image);
+  void ListStyleTypeChanged();
 
   String text_;
   Persistent<StyleImage> image_;