Remove MediaQuery[ExpNode]::Copy

Since these classes are immutable, and now GarbageCollected, there
isn't a reason to copy them anymore.

Bug: 1312000
Change-Id: I1d464e7438b8db25d39da35f7dc90b5e3c5a005d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3683445
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1010507}
NOKEYCHECK=True
GitOrigin-RevId: 7dba6d5c6433fb0ca92404ea2351c73270a1cfea

blink/renderer/core/css/container_query.cc

@@ -36,7 +36,7 @@ ContainerQuery::ContainerQuery(ContainerSelector selector,
     : selector_(std::move(selector)), query_(query) {}
 
 ContainerQuery::ContainerQuery(const ContainerQuery& other)
-    : selector_(other.selector_), query_(other.query_->Copy()) {}
+    : selector_(other.selector_), query_(other.query_) {}
 
 String ContainerQuery::ToString() const {
   return query_->Serialize();

blink/renderer/core/css/container_query_test.cc

@@ -306,8 +306,8 @@ TEST_F(ContainerQueryTest, RuleCopy) {
   // The ContainerQuery should be copied.
   EXPECT_NE(&container->GetContainerQuery(), &copy->GetContainerQuery());
 
-  // The inner MediaQueryExpNode should be copied.
-  EXPECT_NE(&GetInnerQuery(container->GetContainerQuery()),
+  // The inner MediaQueryExpNode is immutable, and does not need to be copied.
+  EXPECT_EQ(&GetInnerQuery(container->GetContainerQuery()),
             &GetInnerQuery(copy->GetContainerQuery()));
 }
 

blink/renderer/core/css/media_list.cc

@@ -55,11 +55,7 @@ namespace blink {
 
 MediaQuerySet::MediaQuerySet() = default;
 
-MediaQuerySet::MediaQuerySet(const MediaQuerySet& o)
-    : queries_(o.queries_.size()) {
-  for (unsigned i = 0; i < queries_.size(); ++i)
-    queries_[i] = o.queries_[i]->Copy();
-}
+MediaQuerySet::MediaQuerySet(const MediaQuerySet&) = default;
 
 MediaQuerySet* MediaQuerySet::Create(
     const String& media_string,
@@ -96,14 +92,14 @@ bool MediaQuerySet::Add(const String& query_string,
   if (result->queries_.size() != 1)
     return false;
 
-  MediaQuery* new_query = result->queries_[0].Get();
+  const MediaQuery* new_query = result->queries_[0].Get();
   // TODO(keishi) Changed DCHECK to CHECK for crbug.com/699269 diagnosis
   CHECK(new_query);
 
   // If comparing with any of the media queries in the collection of media
   // queries returns true terminate these steps.
   for (wtf_size_t i = 0; i < queries_.size(); ++i) {
-    MediaQuery& query = *queries_[i];
+    const MediaQuery& query = *queries_[i];
     if (query == *new_query)
       return false;
   }
@@ -123,15 +119,15 @@ bool MediaQuerySet::Remove(const String& query_string_to_remove,
   if (result->queries_.size() != 1)
     return true;
 
-  MediaQuery* new_query = result->queries_[0];
+  const MediaQuery* new_query = result->queries_[0];
   // TODO(keishi) Changed DCHECK to CHECK for crbug.com/699269 diagnosis
   CHECK(new_query);
 
   // Remove any media query from the collection of media queries for which
   // comparing with the media query returns true.
   bool found = false;
   for (wtf_size_t i = 0; i < queries_.size(); ++i) {
-    MediaQuery& query = *queries_[i];
+    const MediaQuery& query = *queries_[i];
     if (query == *new_query) {
       queries_.EraseAt(i);
       --i;
@@ -142,7 +138,7 @@ bool MediaQuerySet::Remove(const String& query_string_to_remove,
   return found;
 }
 
-void MediaQuerySet::AddMediaQuery(MediaQuery* media_query) {
+void MediaQuerySet::AddMediaQuery(const MediaQuery* media_query) {
   // TODO(keishi) Changed DCHECK to CHECK for crbug.com/699269 diagnosis
   CHECK(media_query);
   queries_.push_back(media_query);
@@ -197,7 +193,8 @@ void MediaList::setMediaText(const ExecutionContext* execution_context,
 }
 
 String MediaList::item(unsigned index) const {
-  const HeapVector<Member<MediaQuery>>& queries = media_queries_->QueryVector();
+  const HeapVector<Member<const MediaQuery>>& queries =
+      media_queries_->QueryVector();
   if (index < queries.size())
     return queries[index]->CssText();
   return String();

blink/renderer/core/css/media_list.h

@@ -57,9 +57,11 @@ class CORE_EXPORT MediaQuerySet : public GarbageCollected<MediaQuerySet> {
   bool Add(const String&, const ExecutionContext*);
   bool Remove(const String&, const ExecutionContext*);
 
-  void AddMediaQuery(MediaQuery*);
+  void AddMediaQuery(const MediaQuery*);
 
-  const HeapVector<Member<MediaQuery>>& QueryVector() const { return queries_; }
+  const HeapVector<Member<const MediaQuery>>& QueryVector() const {
+    return queries_;
+  }
 
   String MediaText() const;
   bool HasUnknown() const;
@@ -69,7 +71,7 @@ class CORE_EXPORT MediaQuerySet : public GarbageCollected<MediaQuerySet> {
   }
 
  private:
-  HeapVector<Member<MediaQuery>> queries_;
+  HeapVector<Member<const MediaQuery>> queries_;
 };
 
 class MediaList final : public ScriptWrappable {

blink/renderer/core/css/media_query.cc

@@ -86,7 +86,7 @@ MediaQuery::MediaQuery(RestrictorType restrictor,
 MediaQuery::MediaQuery(const MediaQuery& o)
     : restrictor_(o.restrictor_),
       media_type_(o.media_type_),
-      exp_node_(o.exp_node_ ? o.exp_node_->Copy() : nullptr),
+      exp_node_(o.exp_node_),
       serialization_cache_(o.serialization_cache_),
       has_unknown_(o.has_unknown_) {}
 

blink/renderer/core/css/media_query.h

@@ -63,8 +63,6 @@ class CORE_EXPORT MediaQuery : public GarbageCollected<MediaQuery> {
   bool operator==(const MediaQuery& other) const;
   String CssText() const;
 
-  MediaQuery* Copy() const { return MakeGarbageCollected<MediaQuery>(*this); }
-
  private:
   MediaQuery& operator=(const MediaQuery&) = delete;
   bool BehaveAsNotAll() const;

blink/renderer/core/css/media_query_evaluator.cc

@@ -155,7 +155,7 @@ bool MediaQueryEvaluator::Eval(const MediaQuerySet& query_set) const {
 
 bool MediaQueryEvaluator::Eval(const MediaQuerySet& query_set,
                                Results results) const {
-  const HeapVector<Member<MediaQuery>>& queries = query_set.QueryVector();
+  const HeapVector<Member<const MediaQuery>>& queries = query_set.QueryVector();
   if (!queries.size())
     return true;  // Empty query list evaluates to true.
 

blink/renderer/core/css/media_query_exp.cc

@@ -667,10 +667,6 @@ void MediaQueryFeatureExpNode::Trace(Visitor* visitor) const {
   MediaQueryExpNode::Trace(visitor);
 }
 
-const MediaQueryExpNode* MediaQueryFeatureExpNode::Copy() const {
-  return MakeGarbageCollected<MediaQueryFeatureExpNode>(exp_);
-}
-
 void MediaQueryUnaryExpNode::Trace(Visitor* visitor) const {
   visitor->Trace(operand_);
   MediaQueryExpNode::Trace(visitor);
@@ -692,31 +688,18 @@ void MediaQueryNestedExpNode::SerializeTo(StringBuilder& builder) const {
   builder.Append(")");
 }
 
-const MediaQueryExpNode* MediaQueryNestedExpNode::Copy() const {
-  return MakeGarbageCollected<MediaQueryNestedExpNode>(Operand().Copy());
-}
-
 void MediaQueryFunctionExpNode::SerializeTo(StringBuilder& builder) const {
   builder.Append(name_);
   builder.Append("(");
   Operand().SerializeTo(builder);
   builder.Append(")");
 }
 
-const MediaQueryExpNode* MediaQueryFunctionExpNode::Copy() const {
-  return MakeGarbageCollected<MediaQueryFunctionExpNode>(Operand().Copy(),
-                                                         name_);
-}
-
 void MediaQueryNotExpNode::SerializeTo(StringBuilder& builder) const {
   builder.Append("not ");
   Operand().SerializeTo(builder);
 }
 
-const MediaQueryExpNode* MediaQueryNotExpNode::Copy() const {
-  return MakeGarbageCollected<MediaQueryNotExpNode>(Operand().Copy());
-}
-
 void MediaQueryCompoundExpNode::Trace(Visitor* visitor) const {
   visitor->Trace(left_);
   visitor->Trace(right_);
@@ -740,22 +723,12 @@ void MediaQueryAndExpNode::SerializeTo(StringBuilder& builder) const {
   Right().SerializeTo(builder);
 }
 
-const MediaQueryExpNode* MediaQueryAndExpNode::Copy() const {
-  return MakeGarbageCollected<MediaQueryAndExpNode>(Left().Copy(),
-                                                    Right().Copy());
-}
-
 void MediaQueryOrExpNode::SerializeTo(StringBuilder& builder) const {
   Left().SerializeTo(builder);
   builder.Append(" or ");
   Right().SerializeTo(builder);
 }
 
-const MediaQueryExpNode* MediaQueryOrExpNode::Copy() const {
-  return MakeGarbageCollected<MediaQueryOrExpNode>(Left().Copy(),
-                                                   Right().Copy());
-}
-
 void MediaQueryUnknownExpNode::SerializeTo(StringBuilder& builder) const {
   builder.Append(string_);
 }
@@ -768,8 +741,4 @@ MediaQueryExpNode::FeatureFlags MediaQueryUnknownExpNode::CollectFeatureFlags()
   return kFeatureUnknown;
 }
 
-const MediaQueryExpNode* MediaQueryUnknownExpNode::Copy() const {
-  return MakeGarbageCollected<MediaQueryUnknownExpNode>(string_);
-}
-
 }  // namespace blink

blink/renderer/core/css/media_query_exp.h

@@ -330,7 +330,6 @@ class CORE_EXPORT MediaQueryExpNode
   virtual void SerializeTo(StringBuilder&) const = 0;
   virtual void CollectExpressions(HeapVector<MediaQueryExp>&) const = 0;
   virtual FeatureFlags CollectFeatureFlags() const = 0;
-  virtual const MediaQueryExpNode* Copy() const = 0;
 
   // These helper functions return nullptr if any argument is nullptr.
   static const MediaQueryExpNode* Not(const MediaQueryExpNode*);
@@ -354,7 +353,6 @@ class CORE_EXPORT MediaQueryFeatureExpNode : public MediaQueryExpNode {
   void SerializeTo(StringBuilder&) const override;
   void CollectExpressions(HeapVector<MediaQueryExp>&) const override;
   FeatureFlags CollectFeatureFlags() const override;
-  const MediaQueryExpNode* Copy() const override;
 
  private:
   MediaQueryExp exp_;
@@ -383,7 +381,6 @@ class CORE_EXPORT MediaQueryNestedExpNode : public MediaQueryUnaryExpNode {
 
   Type GetType() const override { return Type::kNested; }
   void SerializeTo(StringBuilder&) const override;
-  const MediaQueryExpNode* Copy() const override;
 };
 
 class CORE_EXPORT MediaQueryFunctionExpNode : public MediaQueryUnaryExpNode {
@@ -394,7 +391,6 @@ class CORE_EXPORT MediaQueryFunctionExpNode : public MediaQueryUnaryExpNode {
 
   Type GetType() const override { return Type::kFunction; }
   void SerializeTo(StringBuilder&) const override;
-  const MediaQueryExpNode* Copy() const override;
 
  private:
   AtomicString name_;
@@ -407,7 +403,6 @@ class CORE_EXPORT MediaQueryNotExpNode : public MediaQueryUnaryExpNode {
 
   Type GetType() const override { return Type::kNot; }
   void SerializeTo(StringBuilder&) const override;
-  const MediaQueryExpNode* Copy() const override;
 };
 
 class CORE_EXPORT MediaQueryCompoundExpNode : public MediaQueryExpNode {
@@ -438,7 +433,6 @@ class CORE_EXPORT MediaQueryAndExpNode : public MediaQueryCompoundExpNode {
 
   Type GetType() const override { return Type::kAnd; }
   void SerializeTo(StringBuilder&) const override;
-  const MediaQueryExpNode* Copy() const override;
 };
 
 class CORE_EXPORT MediaQueryOrExpNode : public MediaQueryCompoundExpNode {
@@ -449,7 +443,6 @@ class CORE_EXPORT MediaQueryOrExpNode : public MediaQueryCompoundExpNode {
 
   Type GetType() const override { return Type::kOr; }
   void SerializeTo(StringBuilder&) const override;
-  const MediaQueryExpNode* Copy() const override;
 };
 
 class CORE_EXPORT MediaQueryUnknownExpNode : public MediaQueryExpNode {
@@ -460,7 +453,6 @@ class CORE_EXPORT MediaQueryUnknownExpNode : public MediaQueryExpNode {
   void SerializeTo(StringBuilder&) const override;
   void CollectExpressions(HeapVector<MediaQueryExp>&) const override;
   FeatureFlags CollectFeatureFlags() const override;
-  const MediaQueryExpNode* Copy() const override;
 
  private:
   String string_;

blink/renderer/core/css/media_query_exp_test.cc

@@ -222,30 +222,6 @@ TEST(MediaQueryExpTest, Serialize) {
       PairExp("width", GtCmp(PxValue(20.0)), GeCmp(PxValue(10.0))).Serialize());
 }
 
-TEST(MediaQueryExpTest, Copy) {
-  // width < 10px
-  MediaQueryExp width_lt10 = RightExp("width", LtCmp(PxValue(10)));
-  // height < 10px
-  MediaQueryExp height_lt10 = RightExp("height", LtCmp(PxValue(10)));
-
-  HeapVector<Member<const MediaQueryExpNode>> nodes;
-  nodes.push_back(FeatureNode(width_lt10));
-  nodes.push_back(EnclosedFeatureNode(width_lt10));
-  nodes.push_back(NotNode(EnclosedFeatureNode(width_lt10)));
-  nodes.push_back(NestedNode(EnclosedFeatureNode(width_lt10)));
-  nodes.push_back(FunctionNode(EnclosedFeatureNode(width_lt10), "special"));
-  nodes.push_back(AndNode(EnclosedFeatureNode(width_lt10),
-                          EnclosedFeatureNode(height_lt10)));
-  nodes.push_back(OrNode(EnclosedFeatureNode(width_lt10),
-                         EnclosedFeatureNode(height_lt10)));
-  nodes.push_back(UnknownNode("foo(1)"));
-
-  for (const auto& node : nodes) {
-    EXPECT_EQ(node->GetType(), node->Copy()->GetType());
-    EXPECT_EQ(node->Serialize(), node->Copy()->Serialize());
-  }
-}
-
 TEST(MediaQueryExpTest, SerializeNode) {
   EXPECT_EQ("width < 10px",
             FeatureNode(RightExp("width", LtCmp(PxValue(10))))->Serialize());

blink/renderer/core/inspector/inspector_css_agent.cc

@@ -1849,7 +1849,8 @@ std::unique_ptr<protocol::CSS::CSSMedia> InspectorCSSAgent::BuildMediaObject(
   }
 
   const MediaQuerySet* queries = media->Queries();
-  const HeapVector<Member<MediaQuery>>& query_vector = queries->QueryVector();
+  const HeapVector<Member<const MediaQuery>>& query_vector =
+      queries->QueryVector();
   LocalFrame* frame = nullptr;
   if (parent_style_sheet) {
     if (Document* document = parent_style_sheet->OwnerDocument())
@@ -1871,7 +1872,7 @@ std::unique_ptr<protocol::CSS::CSSMedia> InspectorCSSAgent::BuildMediaObject(
   MediaValues* media_values = MediaValues::CreateDynamicIfFrameExists(frame);
   bool has_media_query_items = false;
   for (wtf_size_t i = 0; i < query_vector.size(); ++i) {
-    MediaQuery& query = *query_vector.at(i);
+    const MediaQuery& query = *query_vector.at(i);
     HeapVector<MediaQueryExp> expressions;
     if (query.ExpNode())
       query.ExpNode()->CollectExpressions(expressions);