Bug 1735574 - Cleanup HTMLMetaElement handling of <meta name content>. r=smaug

This shouldn't change behavior, but I find it a bit easier to reason
about (and should be marginally faster by not doing double attribute
lookups, but not like the should usually matter).

Differential Revision: https://phabricator.services.mozilla.com/D128389

dom/base/Document.cpp

@@ -10579,11 +10579,11 @@ ViewportMetaData Document::GetViewportMetaData() const {
                                    : ViewportMetaData();
 }
 
-void Document::AddMetaViewportElement(HTMLMetaElement* aElement,
+void Document::AddMetaViewportElement(HTMLMetaElement& aElement,
                                       ViewportMetaData&& aData) {
   for (size_t i = 0; i < mMetaViewports.Length(); i++) {
     MetaViewportElementAndData& viewport = mMetaViewports[i];
-    if (viewport.mElement == aElement) {
+    if (viewport.mElement == &aElement) {
       if (viewport.mData == aData) {
         return;
       }
@@ -10594,7 +10594,7 @@ void Document::AddMetaViewportElement(HTMLMetaElement* aElement,
     }
   }
 
-  mMetaViewports.AppendElement(MetaViewportElementAndData{aElement, aData});
+  mMetaViewports.AppendElement(MetaViewportElementAndData{&aElement, aData});
   // Trigger recomputation of the nsViewportInfo the next time it's queried.
   mViewportType = Unknown;
 
@@ -10604,9 +10604,9 @@ void Document::AddMetaViewportElement(HTMLMetaElement* aElement,
   asyncDispatcher->RunDOMEventWhenSafe();
 }
 
-void Document::RemoveMetaViewportElement(HTMLMetaElement* aElement) {
+void Document::RemoveMetaViewportElement(HTMLMetaElement& aElement) {
   for (MetaViewportElementAndData& viewport : mMetaViewports) {
-    if (viewport.mElement == aElement) {
+    if (viewport.mElement == &aElement) {
       mMetaViewports.RemoveElement(viewport);
       // Trigger recomputation of the nsViewportInfo the next time it's queried.
       mViewportType = Unknown;

dom/base/Document.h

@@ -1291,9 +1291,8 @@ class Document : public nsINode,
    */
   nsViewportInfo GetViewportInfo(const ScreenIntSize& aDisplaySize);
 
-  void AddMetaViewportElement(HTMLMetaElement* aElement,
+  void AddMetaViewportElement(HTMLMetaElement& aElement, ViewportMetaData&& aData);
-                              ViewportMetaData&& aData);
+  void RemoveMetaViewportElement(HTMLMetaElement& aElement);
-  void RemoveMetaViewportElement(HTMLMetaElement* aElement);
 
   // Returns a ViewportMetaData for this document.
   ViewportMetaData GetViewportMetaData() const;
@@ -5177,6 +5176,10 @@ class Document : public nsINode,
 
   struct MetaViewportElementAndData;
   // An array of <meta name="viewport"> elements and their data.
+  //
+  // NOTE(emilio): We only need this array to deal with removal somewhat
+  // sanely. But other browsers don't do that and removal just seems to leave
+  // the last "viewport data" applying.
   nsTArray<MetaViewportElementAndData> mMetaViewports;
 
   // These member variables cache information about the viewport so we don't

dom/html/HTMLMetaElement.cpp

@@ -34,42 +34,28 @@ HTMLMetaElement::~HTMLMetaElement() = default;
 
 NS_IMPL_ELEMENT_CLONE(HTMLMetaElement)
 
-void HTMLMetaElement::SetMetaReferrer(Document* aDocument) {
-  if (!aDocument || !AttrValueIs(kNameSpaceID_None, nsGkAtoms::name,
-                                 nsGkAtoms::referrer, eIgnoreCase)) {
-    return;
-  }
-  nsAutoString content;
-  GetContent(content);
-  content =
-      nsContentUtils::TrimWhitespace<nsContentUtils::IsHTMLWhitespace>(content);
-  aDocument->UpdateReferrerInfoFromMeta(content, false);
-}
-
 nsresult HTMLMetaElement::AfterSetAttr(int32_t aNameSpaceID, nsAtom* aName,
                                        const nsAttrValue* aValue,
                                        const nsAttrValue* aOldValue,
                                        nsIPrincipal* aSubjectPrincipal,
                                        bool aNotify) {
   if (aNameSpaceID == kNameSpaceID_None) {
-    Document* document = GetUncomposedDoc();
+    if (Document* document = GetUncomposedDoc()) {
-    if (aName == nsGkAtoms::content) {
+      if (aName == nsGkAtoms::content) {
-      if (document && AttrValueIs(kNameSpaceID_None, nsGkAtoms::name,
+        if (const nsAttrValue* name = GetParsedAttr(nsGkAtoms::name)) {
-                                  nsGkAtoms::viewport, eIgnoreCase)) {
+          MetaAddedOrChanged(*document, *name, FromChange::Yes);
-        ProcessViewportContent(document);
+        }
+        CreateAndDispatchEvent(*document, u"DOMMetaChanged"_ns);
+      } else if (aName == nsGkAtoms::name) {
+        if (aOldValue) {
+          MetaRemoved(*document, *aOldValue, FromChange::Yes);
+        }
+        if (aValue) {
+          MetaAddedOrChanged(*document, *aValue, FromChange::Yes);
+        }
+        CreateAndDispatchEvent(*document, u"DOMMetaChanged"_ns);
       }
-      CreateAndDispatchEvent(document, u"DOMMetaChanged"_ns);
-    } else if (document && aName == nsGkAtoms::name) {
-      if (aValue && aValue->Equals(nsGkAtoms::viewport, eIgnoreCase)) {
-        ProcessViewportContent(document);
-      } else if (aOldValue &&
-                 aOldValue->Equals(nsGkAtoms::viewport, eIgnoreCase)) {
-        DiscardViewportContent(document);
-      }
-      CreateAndDispatchEvent(document, u"DOMMetaChanged"_ns);
     }
-    // Update referrer policy when it got changed from JS
-    SetMetaReferrer(document);
   }
 
   return nsGenericHTMLElement::AfterSetAttr(
@@ -101,11 +87,6 @@ nsresult HTMLMetaElement::BindToTree(BindContext& aContext, nsINode& aParent) {
     doc.ProcessMETATag(this);
   }
 
-  if (AttrValueIs(kNameSpaceID_None, nsGkAtoms::name, nsGkAtoms::viewport,
-                  eIgnoreCase)) {
-    ProcessViewportContent(&doc);
-  }
-
   if (AttrValueIs(kNameSpaceID_None, nsGkAtoms::httpEquiv, nsGkAtoms::headerCSP,
                   eIgnoreCase)) {
     // only accept <meta http-equiv="Content-Security-Policy" content=""> if it
@@ -131,25 +112,25 @@ nsresult HTMLMetaElement::BindToTree(BindContext& aContext, nsINode& aParent) {
     }
   }
 
-  SetMetaReferrer(&doc);
+  if (const nsAttrValue* name = GetParsedAttr(nsGkAtoms::name)) {
-  CreateAndDispatchEvent(&doc, u"DOMMetaAdded"_ns);
+    MetaAddedOrChanged(doc, *name, FromChange::No);
+  }
+  CreateAndDispatchEvent(doc, u"DOMMetaAdded"_ns);
   return rv;
 }
 
 void HTMLMetaElement::UnbindFromTree(bool aNullParent) {
-  nsCOMPtr<Document> oldDoc = GetUncomposedDoc();
+  if (Document* oldDoc = GetUncomposedDoc()) {
-  if (oldDoc && AttrValueIs(kNameSpaceID_None, nsGkAtoms::name,
+    if (const nsAttrValue* name = GetParsedAttr(nsGkAtoms::name)) {
-                            nsGkAtoms::viewport, eIgnoreCase)) {
+      MetaRemoved(*oldDoc, *name, FromChange::No);
-    DiscardViewportContent(oldDoc);
+    }
+    CreateAndDispatchEvent(*oldDoc, u"DOMMetaRemoved"_ns);
   }
-  CreateAndDispatchEvent(oldDoc, u"DOMMetaRemoved"_ns);
   nsGenericHTMLElement::UnbindFromTree(aNullParent);
 }
 
-void HTMLMetaElement::CreateAndDispatchEvent(Document* aDoc,
+void HTMLMetaElement::CreateAndDispatchEvent(Document&,
                                              const nsAString& aEventName) {
-  if (!aDoc) return;
-
   RefPtr<AsyncEventDispatcher> asyncDispatcher = new AsyncEventDispatcher(
       this, aEventName, CanBubble::eYes, ChromeOnlyDispatch::eYes);
   asyncDispatcher->RunDOMEventWhenSafe();
@@ -160,29 +141,36 @@ JSObject* HTMLMetaElement::WrapNode(JSContext* aCx,
   return HTMLMetaElement_Binding::Wrap(aCx, this, aGivenProto);
 }
 
-void HTMLMetaElement::ProcessViewportContent(Document* aDocument) {
+void HTMLMetaElement::MetaAddedOrChanged(Document& aDoc,
-  if (!HasAttr(kNameSpaceID_None, nsGkAtoms::content)) {
+                                         const nsAttrValue& aName,
-    // Call Document::RemoveMetaViewportElement for cases that the content
+                                         FromChange aFromChange) {
-    // attribute is removed.
+  nsAutoString content;
-    // NOTE: RemoveMetaViewportElement enumerates all existing meta viewport
+  const bool hasContent = GetAttr(nsGkAtoms::content, content);
-    // tags in the case where this element hasn't been there, i.e. this element
+  if (aName.Equals(nsGkAtoms::viewport, eIgnoreCase)) {
-    // is newly added to the document, but it should be fine because a document
+    if (hasContent) {
-    // unlikely has a bunch of meta viewport tags.
+      aDoc.SetHeaderData(nsGkAtoms::viewport, content);
-    aDocument->RemoveMetaViewportElement(this);
+      aDoc.AddMetaViewportElement(*this, ViewportMetaData(content));
+    } else if (aFromChange == FromChange::Yes) {
+      aDoc.RemoveMetaViewportElement(*this);
+    }
     return;
   }
 
-  nsAutoString content;
+  if (aName.Equals(nsGkAtoms::referrer, eIgnoreCase)) {
-  GetContent(content);
+    content = nsContentUtils::TrimWhitespace<nsContentUtils::IsHTMLWhitespace>(
-
+        content);
-  aDocument->SetHeaderData(nsGkAtoms::viewport, content);
+    return aDoc.UpdateReferrerInfoFromMeta(content,
-
+                                           /* aPreload = */ false);
-  ViewportMetaData data(content);
+  }
-  aDocument->AddMetaViewportElement(this, std::move(data));
 }
 
-void HTMLMetaElement::DiscardViewportContent(Document* aDocument) {
+void HTMLMetaElement::MetaRemoved(Document& aDoc, const nsAttrValue& aName,
-  aDocument->RemoveMetaViewportElement(this);
+                                  FromChange aFromChange) {
+  if (aName.Equals(nsGkAtoms::viewport, eIgnoreCase)) {
+    return aDoc.RemoveMetaViewportElement(*this);
+  }
+  // FIXME: referrer doesn't do anything on removal or when its name changes to
+  // something else?
 }
 
 }  // namespace mozilla::dom

dom/html/HTMLMetaElement.h

@@ -30,7 +30,7 @@ class HTMLMetaElement final : public nsGenericHTMLElement {
                                 nsIPrincipal* aSubjectPrincipal,
                                 bool aNotify) override;
 
-  void CreateAndDispatchEvent(Document* aDoc, const nsAString& aEventName);
+  void CreateAndDispatchEvent(Document&, const nsAString& aEventName);
 
   virtual nsresult Clone(dom::NodeInfo*, nsINode** aResult) const override;
 
@@ -62,9 +62,11 @@ class HTMLMetaElement final : public nsGenericHTMLElement {
   virtual ~HTMLMetaElement();
 
  private:
-  void SetMetaReferrer(Document* aDocument);
+  enum class FromChange : bool { No, Yes };
-  void ProcessViewportContent(Document* aDocument);
+  void MetaRemoved(Document& aDoc, const nsAttrValue& aName,
-  void DiscardViewportContent(Document* aDocument);
+                   FromChange aFromChange);
+  void MetaAddedOrChanged(Document& aDoc, const nsAttrValue& aName,
+                          FromChange aFromChange);
 };
 
 }  // namespace dom