Bug 1553705 - Use a cheaper to compute state key for parser inserted form controls. r=smaug

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

--HG--
extra : moz-landing-system : lando

accessible/generic/Accessible.cpp

@@ -38,6 +38,7 @@
 #include "nsPIDOMWindow.h"
 
 #include "mozilla/dom/Document.h"
+#include "mozilla/dom/HTMLFormElement.h"
 #include "nsIContent.h"
 #include "nsIForm.h"
 #include "nsIFormControl.h"
@@ -1730,8 +1731,7 @@ Relation Accessible::RelationByType(RelationType aType) const {
         // HTML form controls implements nsIFormControl interface.
         nsCOMPtr<nsIFormControl> control(do_QueryInterface(mContent));
         if (control) {
-          nsCOMPtr<nsIForm> form(do_QueryInterface(control->GetFormElement()));
+          if (dom::HTMLFormElement* form = control->GetFormElement()) {
-          if (form) {
             nsCOMPtr<nsIContent> formContent =
                 do_QueryInterface(form->GetDefaultSubmitElement());
             return Relation(mDoc, formContent);

dom/base/Document.cpp

@@ -1338,7 +1338,9 @@ Document::Document(const char* aContentType)
       mPendingInitialTranslation(false),
       mGeneration(0),
       mCachedTabSizeGeneration(0),
-      mInRDMPane(false) {
+      mInRDMPane(false),
+      mNextFormNumber(0),
+      mNextControlNumber(0) {
   MOZ_LOG(gDocumentLeakPRLog, LogLevel::Debug, ("DOCUMENT %p created", this));
 
   SetIsInDocument();

dom/base/Document.h

@@ -1697,6 +1697,18 @@ class Document : public nsINode,
 
   void SetKeyPressEventModel(uint16_t aKeyPressEventModel);
 
+  // Gets the next form number.
+  //
+  // Used by nsContentUtils::GenerateStateKey to get a unique number for each
+  // parser inserted form element.
+  int32_t GetNextFormNumber() { return mNextFormNumber++; }
+
+  // Gets the next form control number.
+  //
+  // Used by nsContentUtils::GenerateStateKey to get a unique number for each
+  // parser inserted form control element.
+  int32_t GetNextControlNumber() { return mNextControlNumber++; }
+
  protected:
   friend class nsUnblockOnloadEvent;
 
@@ -5197,6 +5209,10 @@ class Document : public nsINode,
   // The principal to use for the storage area of this document.
   nsCOMPtr<nsIPrincipal> mIntrinsicStoragePrincipal;
 
+  // See GetNextFormNumber and GetNextControlNumber.
+  int32_t mNextFormNumber;
+  int32_t mNextControlNumber;
+
  public:
   // Needs to be public because the bindings code pokes at it.
   js::ExpandoAndGeneration mExpandoAndGeneration;

dom/base/nsContentUtils.cpp

@@ -2692,10 +2692,6 @@ void nsContentUtils::GenerateStateKey(nsIContent* aContent, Document* aDocument,
 
   if (doc && doc->IsHTMLOrXHTML()) {
     nsHTMLDocument* htmlDoc = doc->AsHTMLDocument();
-    RefPtr<nsContentList> htmlForms;
-    RefPtr<nsContentList> htmlFormControls;
-    htmlDoc->GetFormsAndFormControls(getter_AddRefs(htmlForms),
-                                     getter_AddRefs(htmlFormControls));
 
     // If we have a form control and can calculate form information, use that
     // as the key - it is more reliable than just recording position in the
@@ -2703,47 +2699,84 @@ void nsContentUtils::GenerateStateKey(nsIContent* aContent, Document* aDocument,
     // XXXbz Is it, really?  We have bugs on this, I think...
     // Important to have a unique key, and tag/type/name may not be.
     //
-    // If the control has a form, the format of the key is:
+    // The format of the key depends on whether the control has a form,
-    // f>type>IndOfFormInDoc>IndOfControlInForm>FormName>name
+    // and whether the element was parser inserted:
-    // else:
+    //
-    // d>type>IndOfControlInDoc>name
+    // [Has Form, Parser Inserted]:
+    //   fp>type>FormNum>IndOfControlInForm>FormName>name
+    //
+    // [No Form, Parser Inserted]:
+    //   dp>type>ControlNum>name
+    //
+    // [Has Form, Not Parser Inserted]:
+    //   fn>type>IndOfFormInDoc>IndOfControlInForm>FormName>name
+    //
+    // [No Form, Not Parser Inserted]:
+    //   dn>type>IndOfControlInDoc>name
     //
     // XXX We don't need to use index if name is there
     // XXXbz We don't?  Why not?  I don't follow.
     //
     nsCOMPtr<nsIFormControl> control(do_QueryInterface(aContent));
     if (control) {
+      // Get the control number if this was a parser inserted element from the
+      // network.
+      int32_t controlNumber =
+          control->GetParserInsertedControlNumberForStateKey();
+      bool parserInserted = controlNumber != -1;
+
+      RefPtr<nsContentList> htmlForms;
+      RefPtr<nsContentList> htmlFormControls;
+      if (!parserInserted) {
+        // Getting these lists is expensive, as we need to keep them up to date
+        // as the document loads, so we avoid it if we don't need them.
+        htmlDoc->GetFormsAndFormControls(getter_AddRefs(htmlForms),
+                                         getter_AddRefs(htmlFormControls));
+      }
+
       // Append the control type
       KeyAppendInt(control->ControlType(), aKey);
 
       // If in a form, add form name / index of form / index in form
-      Element* formElement = control->GetFormElement();
+      HTMLFormElement* formElement = control->GetFormElement();
       if (formElement) {
         if (IsAutocompleteOff(formElement)) {
           aKey.Truncate();
           return;
         }
 
-        KeyAppendString(NS_LITERAL_CSTRING("f"), aKey);
+        // Append the form number, if this is a parser inserted control, or
-
+        // the index of the form in the document otherwise.
-        // Append the index of the form in the document
+        bool appendedForm = false;
-        int32_t index = htmlForms->IndexOf(formElement, false);
+        if (parserInserted) {
-        if (index <= -1) {
+          MOZ_ASSERT(formElement->GetFormNumberForStateKey() != -1,
-          //
+                     "when generating a state key for a parser inserted form "
-          // XXX HACK this uses some state that was dumped into the document
+                     "control we should have a parser inserted <form> element");
-          // specifically to fix bug 138892.  What we are trying to do is
+          KeyAppendString(NS_LITERAL_CSTRING("fp"), aKey);
-          // *guess* which form this control's state is found in, with the
+          KeyAppendInt(formElement->GetFormNumberForStateKey(), aKey);
-          // highly likely guess that the highest form parsed so far is the one.
+          appendedForm = true;
-          // This code should not be on trunk, only branch.
+        } else {
-          //
+          KeyAppendString(NS_LITERAL_CSTRING("fn"), aKey);
-          index = htmlDoc->GetNumFormsSynchronous() - 1;
+          int32_t index = htmlForms->IndexOf(formElement, false);
+          if (index <= -1) {
+            //
+            // XXX HACK this uses some state that was dumped into the document
+            // specifically to fix bug 138892.  What we are trying to do is
+            // *guess* which form this control's state is found in, with the
+            // highly likely guess that the highest form parsed so far is the
+            // one. This code should not be on trunk, only branch.
+            //
+            index = htmlDoc->GetNumFormsSynchronous() - 1;
+          }
+          if (index > -1) {
+            KeyAppendInt(index, aKey);
+            appendedForm = true;
+          }
         }
-        if (index > -1) {
-          KeyAppendInt(index, aKey);
 
+        if (appendedForm) {
           // Append the index of the control in the form
-          nsCOMPtr<nsIForm> form(do_QueryInterface(formElement));
+          int32_t index = formElement->IndexOfControl(control);
-          index = form->IndexOfControl(control);
 
           if (index > -1) {
             KeyAppendInt(index, aKey);
@@ -2755,29 +2788,30 @@ void nsContentUtils::GenerateStateKey(nsIContent* aContent, Document* aDocument,
         nsAutoString formName;
         formElement->GetAttr(kNameSpaceID_None, nsGkAtoms::name, formName);
         KeyAppendString(formName, aKey);
-
       } else {
-        KeyAppendString(NS_LITERAL_CSTRING("d"), aKey);
+        // Not in a form.  Append the control number, if this is a parser
-
+        // inserted control, or the index of the control in the document
-        // If not in a form, add index of control in document
+        // otherwise.
-        // Less desirable than indexing by form info.
+        if (parserInserted) {
-
+          KeyAppendString(NS_LITERAL_CSTRING("dp"), aKey);
-        // Hash by index of control in doc (we are not in a form)
+          KeyAppendInt(control->GetParserInsertedControlNumberForStateKey(),
-        // These are important as they are unique, and type/name may not be.
+                       aKey);
-
-        // We have to flush sink notifications at this point to make
-        // sure that htmlFormControls is up to date.
-        int32_t index = htmlFormControls->IndexOf(aContent, true);
-        if (index > -1) {
-          KeyAppendInt(index, aKey);
           generatedUniqueKey = true;
+        } else {
+          KeyAppendString(NS_LITERAL_CSTRING("dn"), aKey);
+          int32_t index = htmlFormControls->IndexOf(aContent, true);
+          if (index > -1) {
+            KeyAppendInt(index, aKey);
+            generatedUniqueKey = true;
+          }
         }
-      }
 
-      // Append the control name
+        // Append the control name
-      nsAutoString name;
+        nsAutoString name;
-      aContent->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::name, name);
+        aContent->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::name,
-      KeyAppendString(name, aKey);
+                                       name);
+        KeyAppendString(name, aKey);
+      }
     }
   }
 

dom/html/HTMLButtonElement.cpp

@@ -54,7 +54,7 @@ static const nsAttrValue::EnumTable* kButtonDefaultType = &kButtonTypeTable[2];
 HTMLButtonElement::HTMLButtonElement(
     already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo,
     FromParser aFromParser)
-    : nsGenericHTMLFormElementWithState(std::move(aNodeInfo),
+    : nsGenericHTMLFormElementWithState(std::move(aNodeInfo), aFromParser,
                                         kButtonDefaultType->value),
       mDisabledChanged(false),
       mInInternalActivate(false),

dom/html/HTMLFormElement.cpp

@@ -110,6 +110,7 @@ HTMLFormElement::HTMLFormElement(
       mPastNameLookupTable(FORM_CONTROL_LIST_HASHTABLE_LENGTH),
       mSubmitPopupState(PopupBlocker::openAbused),
       mInvalidElementsCount(0),
+      mFormNumber(-1),
       mGeneratingSubmit(false),
       mGeneratingReset(false),
       mIsSubmitting(false),
@@ -2312,5 +2313,26 @@ JSObject* HTMLFormElement::WrapNode(JSContext* aCx,
   return HTMLFormElement_Binding::Wrap(aCx, this, aGivenProto);
 }
 
+int32_t HTMLFormElement::GetFormNumberForStateKey() {
+  if (mFormNumber == -1) {
+    mFormNumber = OwnerDoc()->GetNextFormNumber();
+  }
+  return mFormNumber;
+}
+
+void HTMLFormElement::NodeInfoChanged(Document* aOldDoc) {
+  nsGenericHTMLElement::NodeInfoChanged(aOldDoc);
+
+  // When a <form> element is adopted into a new document, we want any state
+  // keys generated from it to no longer consider this element to be parser
+  // inserted, and so have state keys based on the position of the <form>
+  // element in the document, rather than the order it was inserted in.
+  //
+  // This is not strictly necessary, since we only ever look at the form number
+  // for parser inserted form controls, and we do that at the time the form
+  // control element is inserted into its original document by the parser.
+  mFormNumber = -1;
+}
+
 }  // namespace dom
 }  // namespace mozilla

dom/html/HTMLFormElement.h

@@ -520,6 +520,17 @@ class HTMLFormElement final : public nsGenericHTMLElement,
    */
   nsresult GetActionURL(nsIURI** aActionURL, Element* aOriginatingElement);
 
+  // Returns a number for this form that is unique within its owner document.
+  // This is used by nsContentUtils::GenerateStateKey to identify form controls
+  // that are inserted into the document by the parser.
+  int32_t GetFormNumberForStateKey();
+
+  /**
+   * Called when we have been cloned and adopted, and the information of the
+   * node has been changed.
+   */
+  void NodeInfoChanged(Document* aOldDoc) override;
+
  protected:
   //
   // Data members
@@ -580,6 +591,9 @@ class HTMLFormElement final : public nsGenericHTMLElement,
    */
   int32_t mInvalidElementsCount;
 
+  // See GetFormNumberForStateKey.
+  int32_t mFormNumber;
+
   /** Whether we are currently processing a submit event or not */
   bool mGeneratingSubmit;
   /** Whether we are currently processing a reset event or not */

dom/html/HTMLInputElement.cpp

@@ -939,7 +939,7 @@ void HTMLInputElement::Shutdown() {
 HTMLInputElement::HTMLInputElement(
     already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo,
     FromParser aFromParser, FromClone aFromClone)
-    : nsGenericHTMLFormElementWithState(std::move(aNodeInfo),
+    : nsGenericHTMLFormElementWithState(std::move(aNodeInfo), aFromParser,
                                         kInputDefaultType->value),
       mAutocompleteAttrState(nsContentUtils::eAutocompleteAttrState_Unknown),
       mAutocompleteInfoState(nsContentUtils::eAutocompleteAttrState_Unknown),

dom/html/HTMLSelectElement.cpp

@@ -111,7 +111,8 @@ SafeOptionListMutation::~SafeOptionListMutation() {
 HTMLSelectElement::HTMLSelectElement(
     already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo,
     FromParser aFromParser)
-    : nsGenericHTMLFormElementWithState(std::move(aNodeInfo), NS_FORM_SELECT),
+    : nsGenericHTMLFormElementWithState(std::move(aNodeInfo), aFromParser,
+                                        NS_FORM_SELECT),
       mOptions(new HTMLOptionsCollection(this)),
       mAutocompleteAttrState(nsContentUtils::eAutocompleteAttrState_Unknown),
       mAutocompleteInfoState(nsContentUtils::eAutocompleteAttrState_Unknown),

dom/html/HTMLTextAreaElement.cpp

@@ -51,7 +51,8 @@ namespace dom {
 HTMLTextAreaElement::HTMLTextAreaElement(
     already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo,
     FromParser aFromParser)
-    : nsGenericHTMLFormElementWithState(std::move(aNodeInfo), NS_FORM_TEXTAREA),
+    : nsGenericHTMLFormElementWithState(std::move(aNodeInfo), aFromParser,
+                                        NS_FORM_TEXTAREA),
       mValueChanged(false),
       mLastValueChangeWasInteractive(false),
       mHandlingSelect(false),

dom/html/nsGenericHTMLElement.cpp

@@ -1611,7 +1611,7 @@ void nsGenericHTMLFormElement::ClearForm(bool aRemoveFromForm,
   AfterClearForm(aUnbindOrDelete);
 }
 
-Element* nsGenericHTMLFormElement::GetFormElement() { return mForm; }
+HTMLFormElement* nsGenericHTMLFormElement::GetFormElement() { return mForm; }
 
 HTMLFieldSetElement* nsGenericHTMLFormElement::GetFieldSet() {
   return mFieldSet;
@@ -2520,8 +2520,12 @@ void nsGenericHTMLElement::ChangeEditableState(int32_t aChange) {
 //----------------------------------------------------------------------
 
 nsGenericHTMLFormElementWithState::nsGenericHTMLFormElementWithState(
-    already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo, uint8_t aType)
+    already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo,
-    : nsGenericHTMLFormElement(std::move(aNodeInfo), aType) {
+    FromParser aFromParser, uint8_t aType)
+    : nsGenericHTMLFormElement(std::move(aNodeInfo), aType),
+      mControlNumber(!!(aFromParser & FROM_PARSER_NETWORK)
+                         ? OwnerDoc()->GetNextControlNumber()
+                         : -1) {
   mStateKey.SetIsVoid(true);
 }
 
@@ -2533,6 +2537,7 @@ void nsGenericHTMLFormElementWithState::GenerateStateKey() {
 
   Document* doc = GetUncomposedDoc();
   if (!doc) {
+    mStateKey.Truncate();
     return;
   }
 
@@ -2592,6 +2597,9 @@ nsGenericHTMLFormElementWithState::GetLayoutHistory(bool aRead) {
 }
 
 bool nsGenericHTMLFormElementWithState::RestoreFormControlState() {
+  MOZ_ASSERT(!mStateKey.IsVoid(),
+             "GenerateStateKey must already have been called");
+
   if (mStateKey.IsEmpty()) {
     return false;
   }
@@ -2614,6 +2622,12 @@ bool nsGenericHTMLFormElementWithState::RestoreFormControlState() {
 
 void nsGenericHTMLFormElementWithState::NodeInfoChanged(Document* aOldDoc) {
   nsGenericHTMLElement::NodeInfoChanged(aOldDoc);
+
+  // We need to regenerate the state key now we're in a new document.  Clearing
+  // mControlNumber means we stop considering this control to be parser
+  // inserted, and we'll generate a state key based on its position in the
+  // document rather than the order it was inserted into the document.
+  mControlNumber = -1;
   mStateKey.SetIsVoid(true);
 }
 

dom/html/nsGenericHTMLElement.h

@@ -930,7 +930,7 @@ class nsGenericHTMLFormElement : public nsGenericHTMLElement,
 
   // nsIFormControl
   virtual mozilla::dom::HTMLFieldSetElement* GetFieldSet() override;
-  virtual mozilla::dom::Element* GetFormElement() override;
+  virtual mozilla::dom::HTMLFormElement* GetFormElement() override;
   mozilla::dom::HTMLFormElement* GetForm() const { return mForm; }
   virtual void SetForm(mozilla::dom::HTMLFormElement* aForm) override;
   virtual void ClearForm(bool aRemoveFromForm, bool aUnbindOrDelete) override;
@@ -1083,7 +1083,8 @@ class nsGenericHTMLFormElement : public nsGenericHTMLElement,
 class nsGenericHTMLFormElementWithState : public nsGenericHTMLFormElement {
  public:
   nsGenericHTMLFormElementWithState(
-      already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo, uint8_t aType);
+      already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo,
+      mozilla::dom::FromParser aFromParser, uint8_t aType);
 
   /**
    * Get the presentation state for a piece of content, or create it if it does
@@ -1100,15 +1101,6 @@ class nsGenericHTMLFormElementWithState : public nsGenericHTMLFormElement {
    */
   already_AddRefed<nsILayoutHistoryState> GetLayoutHistory(bool aRead);
 
-  /**
-   * Restore the state for a form control.  Ends up calling
-   * nsIFormControl::RestoreState().
-   *
-   * @return false if RestoreState() was not called, the return
-   *         value of RestoreState() otherwise.
-   */
-  bool RestoreFormControlState();
-
   /**
    * Called when we have been cloned and adopted, and the information of the
    * node has been changed.
@@ -1116,13 +1108,34 @@ class nsGenericHTMLFormElementWithState : public nsGenericHTMLFormElement {
   virtual void NodeInfoChanged(Document* aOldDoc) override;
 
  protected:
+  /**
+   * Restore the state for a form control in response to the element being
+   * inserted into the document by the parser.  Ends up calling
+   * nsIFormControl::RestoreState().
+   *
+   * GenerateStateKey() must already have been called.
+   *
+   * @return false if RestoreState() was not called, the return
+   *         value of RestoreState() otherwise.
+   */
+  bool RestoreFormControlState();
+
   /* Generates the state key for saving the form state in the session if not
      computed already. The result is stored in mStateKey. */
   void GenerateStateKey();
 
+  int32_t GetParserInsertedControlNumberForStateKey() const override {
+    return mControlNumber;
+  }
+
   /* Used to store the key to that element in the session. Is void until
      GenerateStateKey has been used */
   nsCString mStateKey;
+
+  // A number for this form control that is unique within its owner document.
+  // This is only set to a number for elements inserted into the document by
+  // the parser from the network.  Otherwise, it is -1.
+  int32_t mControlNumber;
 };
 
 #define NS_INTERFACE_MAP_ENTRY_IF_TAG(_interface, _tag) \

dom/html/nsIFormControl.h

@@ -107,7 +107,7 @@ class nsIFormControl : public nsISupports {
    * Get the form for this form control.
    * @return the form
    */
-  virtual mozilla::dom::Element* GetFormElement() = 0;
+  virtual mozilla::dom::HTMLFormElement* GetFormElement() = 0;
 
   /**
    * Set the form for this form control.
@@ -222,6 +222,16 @@ class nsIFormControl : public nsISupports {
     return false;
   }
 
+  // Returns a number for this form control that is unique within its
+  // owner document.  This is used by nsContentUtils::GenerateStateKey
+  // to identify form controls that are inserted into the document by
+  // the parser.  -1 is returned for form controls with no state or
+  // which were inserted into the document by some other means than
+  // the parser from the network.
+  virtual int32_t GetParserInsertedControlNumberForStateKey() const {
+    return -1;
+  };
+
  protected:
   /**
    * Returns whether mType corresponds to a single line text control type.