[css-pseudo] Check the marker argument in ListMarker methods

The ListMarker class contains common logic for LayoutNGOutsideListMarker
and LayoutNGInsideListMarker. But it's not a superclass, since multiple
inheritance would be slow. Instead, the layout marker classes have a
ListMarker member. But the ListMarker methods need to reference back
their LayoutObject owner, this could have been done with a pointer, but
it would take more memory. Instead, the ListMarker methods accept a
LayoutObject& parameter, which is supposed to be the marker that owns
that ListMarker instance.

The problem was that there was no check that the LayoutObject argument
was actually the owner of that ListMarker, or even that it was a marker.
So if by mistake some caller passes another LayoutObject, something will
go wrong but it may go unnoticed, or the cause of the misbehavior may
not be obvious.

This patch adds some DCHECKs to address this.

Bug: 457718
Change-Id: I6cd34fceaf763f8d8ce8032c288a8066eaf90a54
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2218311
Reviewed-by: Rune Lillesveen <[email protected]>
Reviewed-by: Koji Ishii <[email protected]>
Commit-Queue: Koji Ishii <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#773059}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 92704098d48f09be36674bdb811e392d77b3d685

blink/renderer/core/layout/ng/list/list_marker.cc

@@ -32,6 +32,7 @@ ListMarker* ListMarker::Get(LayoutObject* object) {
 // If the value of ListStyleType changed, we need to the marker text has been
 // updated.
 void ListMarker::ListStyleTypeChanged(LayoutObject& marker) {
+  DCHECK_EQ(Get(&marker), this);
   if (marker_text_type_ == kNotText || marker_text_type_ == kUnresolved)
     return;
 
@@ -41,6 +42,7 @@ void ListMarker::ListStyleTypeChanged(LayoutObject& marker) {
 }
 
 void ListMarker::OrdinalValueChanged(LayoutObject& marker) {
+  DCHECK_EQ(Get(&marker), this);
   if (marker_text_type_ == kOrdinalValue) {
     marker_text_type_ = kUnresolved;
     marker.SetNeedsLayoutAndIntrinsicWidthsRecalcAndFullPaintInvalidation(
@@ -49,6 +51,7 @@ void ListMarker::OrdinalValueChanged(LayoutObject& marker) {
 }
 
 void ListMarker::UpdateMarkerText(LayoutObject& marker, LayoutText* text) {
+  DCHECK_EQ(Get(&marker), this);
   DCHECK(text);
   DCHECK_EQ(marker_text_type_, kUnresolved);
   StringBuilder marker_text_builder;
@@ -59,17 +62,20 @@ void ListMarker::UpdateMarkerText(LayoutObject& marker, LayoutText* text) {
 }
 
 void ListMarker::UpdateMarkerText(LayoutObject& marker) {
+  DCHECK_EQ(Get(&marker), this);
   UpdateMarkerText(marker, ToLayoutText(marker.SlowFirstChild()));
 }
 
-LayoutNGListItem* ListMarker::ListItem(const LayoutObject& marker) {
+LayoutNGListItem* ListMarker::ListItem(const LayoutObject& marker) const {
+  DCHECK_EQ(Get(&marker), this);
   return ToLayoutNGListItem(marker.GetNode()->parentNode()->GetLayoutObject());
 }
 
 ListMarker::MarkerTextType ListMarker::MarkerText(
     const LayoutObject& marker,
     StringBuilder* text,
     MarkerTextFormat format) const {
+  DCHECK_EQ(Get(&marker), this);
   if (IsMarkerImage(marker)) {
     if (format == kWithSuffix)
       text->Append(' ');
@@ -159,26 +165,28 @@ ListMarker::MarkerTextType ListMarker::MarkerText(
 }
 
 String ListMarker::MarkerTextWithSuffix(const LayoutObject& marker) const {
+  DCHECK_EQ(Get(&marker), this);
   StringBuilder text;
   MarkerText(marker, &text, kWithSuffix);
   return text.ToString();
 }
 
 String ListMarker::MarkerTextWithoutSuffix(const LayoutObject& marker) const {
+  DCHECK_EQ(Get(&marker), this);
   StringBuilder text;
   MarkerText(marker, &text, kWithoutSuffix);
   return text.ToString();
 }
 
 String ListMarker::TextAlternative(const LayoutObject& marker) const {
+  DCHECK_EQ(Get(&marker), this);
   // For accessibility, return the marker string in the logical order even in
   // RTL, reflecting speech order.
   return MarkerTextWithSuffix(marker);
 }
 
 void ListMarker::UpdateMarkerContentIfNeeded(LayoutObject& marker) {
-  LayoutNGListItem* list_item = ListItem(marker);
-
+  DCHECK_EQ(Get(&marker), this);
   if (!marker.StyleRef().ContentBehavesAsNormal()) {
     marker_text_type_ = kNotText;
     return;
@@ -188,6 +196,7 @@ void ListMarker::UpdateMarkerContentIfNeeded(LayoutObject& marker) {
   LayoutObject* child = marker.SlowFirstChild();
   DCHECK(!child || !child->NextSibling());
 
+  LayoutNGListItem* list_item = ListItem(marker);
   if (IsMarkerImage(marker)) {
     StyleImage* list_style_image = list_item->StyleRef().ListStyleImage();
     if (child) {
@@ -248,6 +257,7 @@ void ListMarker::UpdateMarkerContentIfNeeded(LayoutObject& marker) {
 
 LayoutObject* ListMarker::SymbolMarkerLayoutText(
     const LayoutObject& marker) const {
+  DCHECK_EQ(Get(&marker), this);
   if (marker_text_type_ != kSymbolValue)
     return nullptr;
   return marker.SlowFirstChild();

blink/renderer/core/layout/ng/list/list_marker.h

@@ -21,19 +21,21 @@ class CORE_EXPORT ListMarker {
   static const ListMarker* Get(const LayoutObject*);
   static ListMarker* Get(LayoutObject*);
 
-  static LayoutNGListItem* ListItem(const LayoutObject&);
+  LayoutNGListItem* ListItem(const LayoutObject&) const;
 
   String MarkerTextWithSuffix(const LayoutObject&) const;
   String MarkerTextWithoutSuffix(const LayoutObject&) const;
 
   // Marker text with suffix, e.g. "1. ", for use in accessibility.
   String TextAlternative(const LayoutObject&) const;
 
-  static bool IsMarkerImage(const LayoutObject& marker) {
+  bool IsMarkerImage(const LayoutObject& marker) const {
+    DCHECK_EQ(Get(&marker), this);
     return ListItem(marker)->StyleRef().GeneratesMarkerImage();
   }
 
   void UpdateMarkerTextIfNeeded(LayoutObject& marker) {
+    DCHECK_EQ(Get(&marker), this);
     if (marker_text_type_ == kUnresolved)
       UpdateMarkerText(marker);
   }