Bug 1553705 - Make GenerateStateKey() infallible. r=smaug
authorCameron McCormack <cam@mcc.id.au>
Wed, 26 Jun 2019 21:44:03 +0000
changeset 543080 dd6e7c0970d5a55be8f8ca98dd3b3af6267b5ea1
parent 543079 9b96152c5e6ecf97af63865f357e8106e0763ea4
child 543081 fbb26a04ec1f69879c85249da04fe3b971928ff0
push id2131
push userffxbld-merge
push dateMon, 26 Aug 2019 18:30:20 +0000
treeherdermozilla-release@b19ffb3ca153 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1553705
milestone69.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1553705 - Make GenerateStateKey() infallible. r=smaug Differential Revision: https://phabricator.services.mozilla.com/D32258
dom/base/nsContentUtils.cpp
dom/base/nsContentUtils.h
dom/html/HTMLButtonElement.cpp
dom/html/HTMLInputElement.cpp
dom/html/HTMLSelectElement.cpp
dom/html/HTMLTextAreaElement.cpp
dom/html/nsGenericHTMLElement.cpp
dom/html/nsGenericHTMLElement.h
layout/base/nsFrameManager.cpp
layout/forms/nsComboboxControlFrame.cpp
layout/forms/nsComboboxControlFrame.h
layout/generic/nsIStatefulFrame.h
--- a/dom/base/nsContentUtils.cpp
+++ b/dom/base/nsContentUtils.cpp
@@ -2649,33 +2649,31 @@ static inline void KeyAppendInt(int32_t 
 static inline bool IsAutocompleteOff(const nsIContent* aContent) {
   return aContent->IsElement() &&
          aContent->AsElement()->AttrValueIs(
              kNameSpaceID_None, nsGkAtoms::autocomplete,
              NS_LITERAL_STRING("off"), eIgnoreCase);
 }
 
 /*static*/
-nsresult nsContentUtils::GenerateStateKey(nsIContent* aContent,
-                                          Document* aDocument,
-                                          nsACString& aKey) {
+void nsContentUtils::GenerateStateKey(nsIContent* aContent, Document* aDocument,
+                                      nsACString& aKey) {
+  MOZ_ASSERT(aContent);
+
   aKey.Truncate();
 
   uint32_t partID = aDocument ? aDocument->GetPartID() : 0;
 
-  // We must have content if we're not using a special state id
-  NS_ENSURE_TRUE(aContent, NS_ERROR_FAILURE);
-
   // Don't capture state for anonymous content
   if (aContent->IsInAnonymousSubtree()) {
-    return NS_OK;
+    return;
   }
 
   if (IsAutocompleteOff(aContent)) {
-    return NS_OK;
+    return;
   }
 
   RefPtr<Document> doc = aContent->GetUncomposedDoc();
 
   KeyAppendInt(partID, aKey);  // first append a partID
   bool generatedUniqueKey = false;
 
   if (doc && doc->IsHTMLOrXHTML()) {
@@ -2704,17 +2702,17 @@ nsresult nsContentUtils::GenerateStateKe
       // 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();
       if (formElement) {
         if (IsAutocompleteOff(formElement)) {
           aKey.Truncate();
-          return NS_OK;
+          return;
         }
 
         KeyAppendString(NS_LITERAL_CSTRING("f"), aKey);
 
         // Append the index of the form in the document
         int32_t index = htmlForms->IndexOf(formElement, false);
         if (index <= -1) {
           //
@@ -2788,18 +2786,16 @@ nsresult nsContentUtils::GenerateStateKe
     nsINode* parent = aContent->GetParentNode();
     nsINode* content = aContent;
     while (parent) {
       KeyAppendInt(parent->ComputeIndexOf(content), aKey);
       content = parent;
       parent = content->GetParentNode();
     }
   }
-
-  return NS_OK;
 }
 
 // static
 nsIPrincipal* nsContentUtils::SubjectPrincipal(JSContext* aCx) {
   MOZ_ASSERT(NS_IsMainThread());
 
   // As opposed to SubjectPrincipal(), we do in fact assume that
   // we're in a realm here; anyone who calls this function in
--- a/dom/base/nsContentUtils.h
+++ b/dom/base/nsContentUtils.h
@@ -717,18 +717,18 @@ class nsContentUtils {
   static nsIPrincipal* SubjectPrincipal();
 
   // Returns the prinipal of the given JS object. This may only be called on
   // the main thread for objects from the main thread's JSRuntime. The object
   // must not be a cross-compartment wrapper, because CCWs are not associated
   // with a single realm.
   static nsIPrincipal* ObjectPrincipal(JSObject* aObj);
 
-  static nsresult GenerateStateKey(nsIContent* aContent, Document* aDocument,
-                                   nsACString& aKey);
+  static void GenerateStateKey(nsIContent* aContent, Document* aDocument,
+                               nsACString& aKey);
 
   /**
    * Create a new nsIURI from aSpec, using aBaseURI as the base.  The
    * origin charset of the new nsIURI will be the document charset of
    * aDocument.
    */
   static nsresult NewURIWithDocumentCharset(nsIURI** aResult,
                                             const nsAString& aSpec,
--- a/dom/html/HTMLButtonElement.cpp
+++ b/dom/html/HTMLButtonElement.cpp
@@ -341,20 +341,18 @@ HTMLButtonElement::SubmitNamesValues(HTM
   //
   // Submit
   //
   return aFormSubmission->AddNameValuePair(name, value);
 }
 
 void HTMLButtonElement::DoneCreatingElement() {
   if (!mInhibitStateRestoration) {
-    nsresult rv = GenerateStateKey();
-    if (NS_SUCCEEDED(rv)) {
-      RestoreFormControlState();
-    }
+    GenerateStateKey();
+    RestoreFormControlState();
   }
 }
 
 nsresult HTMLButtonElement::BeforeSetAttr(int32_t aNameSpaceID, nsAtom* aName,
                                           const nsAttrValueOrString* aValue,
                                           bool aNotify) {
   if (aNotify && aName == nsGkAtoms::disabled &&
       aNameSpaceID == kNameSpaceID_None) {
--- a/dom/html/HTMLInputElement.cpp
+++ b/dom/html/HTMLInputElement.cpp
@@ -5889,19 +5889,21 @@ HTMLInputElement::SaveState() {
 
 void HTMLInputElement::DoneCreatingElement() {
   mDoneCreating = true;
 
   //
   // Restore state as needed.  Note that disabled state applies to all control
   // types.
   //
-  bool restoredCheckedState = !mInhibitRestoration &&
-                              NS_SUCCEEDED(GenerateStateKey()) &&
-                              RestoreFormControlState();
+  bool restoredCheckedState = false;
+  if (!mInhibitRestoration) {
+    GenerateStateKey();
+    restoredCheckedState = RestoreFormControlState();
+  }
 
   //
   // If restore does not occur, we initialize .checked using the CHECKED
   // property.
   //
   if (!restoredCheckedState && mShouldInitChecked) {
     DoSetChecked(DefaultChecked(), false, false);
   }
--- a/dom/html/HTMLSelectElement.cpp
+++ b/dom/html/HTMLSelectElement.cpp
@@ -1108,20 +1108,18 @@ void HTMLSelectElement::DoneAddingChildr
   }
 
   // Notify the frame
   if (selectFrame) {
     selectFrame->DoneAddingChildren(true);
   }
 
   if (!mInhibitStateRestoration) {
-    nsresult rv = GenerateStateKey();
-    if (NS_SUCCEEDED(rv)) {
-      RestoreFormControlState();
-    }
+    GenerateStateKey();
+    RestoreFormControlState();
   }
 
   // Now that we're done, select something (if it's a single select something
   // must be selected)
   if (!CheckSelectSomething(false)) {
     // If an option has @selected set, it will be selected during parsing but
     // with an empty value. We have to make sure the select element updates it's
     // validity state to take this into account.
--- a/dom/html/HTMLTextAreaElement.cpp
+++ b/dom/html/HTMLTextAreaElement.cpp
@@ -513,20 +513,18 @@ void HTMLTextAreaElement::DoneAddingChil
   if (!mValueChanged) {
     if (!mDoneAddingChildren) {
       // Reset now that we're done adding children if the content sink tried to
       // sneak some text in without calling AppendChildTo.
       Reset();
     }
 
     if (!mInhibitStateRestoration) {
-      nsresult rv = GenerateStateKey();
-      if (NS_SUCCEEDED(rv)) {
-        RestoreFormControlState();
-      }
+      GenerateStateKey();
+      RestoreFormControlState();
     }
   }
 
   mDoneAddingChildren = true;
 }
 
 bool HTMLTextAreaElement::IsDoneAddingChildren() { return mDoneAddingChildren; }
 
--- a/dom/html/nsGenericHTMLElement.cpp
+++ b/dom/html/nsGenericHTMLElement.cpp
@@ -2479,42 +2479,36 @@ void nsGenericHTMLElement::ChangeEditabl
 //----------------------------------------------------------------------
 
 nsGenericHTMLFormElementWithState::nsGenericHTMLFormElementWithState(
     already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo, uint8_t aType)
     : nsGenericHTMLFormElement(std::move(aNodeInfo), aType) {
   mStateKey.SetIsVoid(true);
 }
 
-nsresult nsGenericHTMLFormElementWithState::GenerateStateKey() {
+void nsGenericHTMLFormElementWithState::GenerateStateKey() {
   // Keep the key if already computed
   if (!mStateKey.IsVoid()) {
-    return NS_OK;
+    return;
   }
 
   Document* doc = GetUncomposedDoc();
   if (!doc) {
-    return NS_OK;
+    return;
   }
 
   // Generate the state key
-  nsresult rv = nsContentUtils::GenerateStateKey(this, doc, mStateKey);
-
-  if (NS_FAILED(rv)) {
-    mStateKey.SetIsVoid(true);
-    return rv;
-  }
+  nsContentUtils::GenerateStateKey(this, doc, mStateKey);
 
   // If the state key is blank, this is anonymous content or for whatever
   // reason we are not supposed to save/restore state: keep it as such.
   if (!mStateKey.IsEmpty()) {
     // Add something unique to content so layout doesn't muck us up.
     mStateKey += "-C";
   }
-  return NS_OK;
 }
 
 PresState* nsGenericHTMLFormElementWithState::GetPrimaryPresState() {
   if (mStateKey.IsEmpty()) {
     return nullptr;
   }
 
   nsCOMPtr<nsILayoutHistoryState> history = GetLayoutHistory(false);
--- a/dom/html/nsGenericHTMLElement.h
+++ b/dom/html/nsGenericHTMLElement.h
@@ -1100,18 +1100,18 @@ class nsGenericHTMLFormElementWithState 
   /**
    * Called when we have been cloned and adopted, and the information of the
    * node has been changed.
    */
   virtual void NodeInfoChanged(Document* aOldDoc) override;
 
  protected:
   /* Generates the state key for saving the form state in the session if not
-     computed already. The result is stored in mStateKey on success */
-  nsresult GenerateStateKey();
+     computed already. The result is stored in mStateKey. */
+  void GenerateStateKey();
 
   /* Used to store the key to that element in the session. Is void until
      GenerateStateKey has been used */
   nsCString mStateKey;
 };
 
 #define NS_INTERFACE_MAP_ENTRY_IF_TAG(_interface, _tag) \
   NS_INTERFACE_MAP_ENTRY_CONDITIONAL(_interface,        \
--- a/layout/base/nsFrameManager.cpp
+++ b/layout/base/nsFrameManager.cpp
@@ -141,18 +141,18 @@ void nsFrameManager::CaptureFrameStateFo
     return;
   }
 
   // Generate the hash key to store the state under
   // Exit early if we get empty key
   nsAutoCString stateKey;
   nsIContent* content = aFrame->GetContent();
   Document* doc = content ? content->GetUncomposedDoc() : nullptr;
-  nsresult rv = statefulFrame->GenerateStateKey(content, doc, stateKey);
-  if (NS_FAILED(rv) || stateKey.IsEmpty()) {
+  statefulFrame->GenerateStateKey(content, doc, stateKey);
+  if (stateKey.IsEmpty()) {
     return;
   }
 
   // Store the state. aState owns frameState now.
   aState->AddState(stateKey, std::move(frameState));
 }
 
 void nsFrameManager::CaptureFrameState(nsIFrame* aFrame,
@@ -202,29 +202,29 @@ void nsFrameManager::RestoreFrameStateFo
   // If we don't have content, we can't generate a hash
   // key and there's probably no state information for us.
   if (!content) {
     return;
   }
 
   nsAutoCString stateKey;
   Document* doc = content->GetUncomposedDoc();
-  nsresult rv = statefulFrame->GenerateStateKey(content, doc, stateKey);
-  if (NS_FAILED(rv) || stateKey.IsEmpty()) {
+  statefulFrame->GenerateStateKey(content, doc, stateKey);
+  if (stateKey.IsEmpty()) {
     return;
   }
 
   // Get the state from the hash
   PresState* frameState = aState->GetState(stateKey);
   if (!frameState) {
     return;
   }
 
   // Restore it
-  rv = statefulFrame->RestoreState(frameState);
+  nsresult rv = statefulFrame->RestoreState(frameState);
   if (NS_FAILED(rv)) {
     return;
   }
 
   // If we restore ok, remove the state from the state table
   aState->RemoveState(stateKey);
 }
 
--- a/layout/forms/nsComboboxControlFrame.cpp
+++ b/layout/forms/nsComboboxControlFrame.cpp
@@ -1561,26 +1561,24 @@ nsComboboxControlFrame::RestoreState(Pre
   }
   ShowList(aState->droppedDown());  // might destroy us
   return NS_OK;
 }
 
 // Append a suffix so that the state key for the combobox is different
 // from the state key the list control uses to sometimes save the scroll
 // position for the same Element
-NS_IMETHODIMP
-nsComboboxControlFrame::GenerateStateKey(nsIContent* aContent,
-                                         Document* aDocument,
-                                         nsACString& aKey) {
-  nsresult rv = nsContentUtils::GenerateStateKey(aContent, aDocument, aKey);
-  if (NS_FAILED(rv) || aKey.IsEmpty()) {
-    return rv;
+void nsComboboxControlFrame::GenerateStateKey(nsIContent* aContent,
+                                              Document* aDocument,
+                                              nsACString& aKey) {
+  nsContentUtils::GenerateStateKey(aContent, aDocument, aKey);
+  if (aKey.IsEmpty()) {
+    return;
   }
   aKey.AppendLiteral("CCF");
-  return NS_OK;
 }
 
 // Fennec uses a custom combobox built-in widget.
 //
 
 /* static */
 bool nsComboboxControlFrame::ToolkitHasNativePopup() {
 #ifdef MOZ_USE_NATIVE_POPUP_WINDOWS
--- a/layout/forms/nsComboboxControlFrame.h
+++ b/layout/forms/nsComboboxControlFrame.h
@@ -204,19 +204,18 @@ class nsComboboxControlFrame final : pub
   }
 
   virtual nsIWidget* GetRollupWidget() override;
 
   // nsIStatefulFrame
   mozilla::UniquePtr<mozilla::PresState> SaveState() override;
   MOZ_CAN_RUN_SCRIPT_BOUNDARY
   NS_IMETHOD RestoreState(mozilla::PresState* aState) override;
-  NS_IMETHOD GenerateStateKey(nsIContent* aContent,
-                              mozilla::dom::Document* aDocument,
-                              nsACString& aKey) override;
+  void GenerateStateKey(nsIContent* aContent, mozilla::dom::Document* aDocument,
+                        nsACString& aKey) override;
 
   static bool ToolkitHasNativePopup();
 
  protected:
   friend class RedisplayTextEvent;
   friend class nsAsyncResize;
   friend class nsResizeDropdownAtFinalPosition;
 
--- a/layout/generic/nsIStatefulFrame.h
+++ b/layout/generic/nsIStatefulFrame.h
@@ -25,16 +25,16 @@ class nsIStatefulFrame {
 
   // Save the state for this frame.
   virtual mozilla::UniquePtr<mozilla::PresState> SaveState() = 0;
 
   // Restore the state for this frame from aState
   NS_IMETHOD RestoreState(mozilla::PresState* aState) = 0;
 
   // Generate a key for this stateful frame
-  NS_IMETHOD GenerateStateKey(nsIContent* aContent,
-                              mozilla::dom::Document* aDocument,
-                              nsACString& aKey) {
-    return nsContentUtils::GenerateStateKey(aContent, aDocument, aKey);
+  virtual void GenerateStateKey(nsIContent* aContent,
+                                mozilla::dom::Document* aDocument,
+                                nsACString& aKey) {
+    nsContentUtils::GenerateStateKey(aContent, aDocument, aKey);
   };
 };
 
 #endif /* _nsIStatefulFrame_h */