From c2b8ac8e2ec35a647475e47f0da7f6e7ccf4e0c2 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Mon, 1 Jul 2019 07:20:04 +0000 Subject: [PATCH] Bug 1553705 - Make GenerateStateKey() infallible. r=smaug Differential Revision: https://phabricator.services.mozilla.com/D32258 --- dom/base/nsContentUtils.cpp | 18 +++++++----------- dom/base/nsContentUtils.h | 4 ++-- dom/html/HTMLButtonElement.cpp | 6 ++---- dom/html/HTMLInputElement.cpp | 8 +++++--- dom/html/HTMLSelectElement.cpp | 6 ++---- dom/html/HTMLTextAreaElement.cpp | 6 ++---- dom/html/nsGenericHTMLElement.cpp | 14 ++++---------- dom/html/nsGenericHTMLElement.h | 4 ++-- layout/base/nsFrameManager.cpp | 10 +++++----- layout/forms/nsComboboxControlFrame.cpp | 14 ++++++-------- layout/forms/nsComboboxControlFrame.h | 5 ++--- layout/generic/nsIStatefulFrame.h | 8 ++++---- 12 files changed, 43 insertions(+), 60 deletions(-) diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index 7ffca74687b1..86d2cb6dc503 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -2668,23 +2668,21 @@ static inline bool IsAutocompleteOff(const nsIContent* aContent) { } /*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 doc = aContent->GetUncomposedDoc(); @@ -2723,7 +2721,7 @@ nsresult nsContentUtils::GenerateStateKey(nsIContent* aContent, if (formElement) { if (IsAutocompleteOff(formElement)) { aKey.Truncate(); - return NS_OK; + return; } KeyAppendString(NS_LITERAL_CSTRING("f"), aKey); @@ -2807,8 +2805,6 @@ nsresult nsContentUtils::GenerateStateKey(nsIContent* aContent, parent = content->GetParentNode(); } } - - return NS_OK; } // static diff --git a/dom/base/nsContentUtils.h b/dom/base/nsContentUtils.h index 14c96cf407e4..24580f129536 100644 --- a/dom/base/nsContentUtils.h +++ b/dom/base/nsContentUtils.h @@ -723,8 +723,8 @@ class nsContentUtils { // 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 diff --git a/dom/html/HTMLButtonElement.cpp b/dom/html/HTMLButtonElement.cpp index 8c41ddae3279..a0c33a6cd748 100644 --- a/dom/html/HTMLButtonElement.cpp +++ b/dom/html/HTMLButtonElement.cpp @@ -346,10 +346,8 @@ HTMLButtonElement::SubmitNamesValues(HTMLFormSubmission* aFormSubmission) { void HTMLButtonElement::DoneCreatingElement() { if (!mInhibitStateRestoration) { - nsresult rv = GenerateStateKey(); - if (NS_SUCCEEDED(rv)) { - RestoreFormControlState(); - } + GenerateStateKey(); + RestoreFormControlState(); } } diff --git a/dom/html/HTMLInputElement.cpp b/dom/html/HTMLInputElement.cpp index 3b95ff33dfff..1e7220d74d0d 100644 --- a/dom/html/HTMLInputElement.cpp +++ b/dom/html/HTMLInputElement.cpp @@ -5891,9 +5891,11 @@ void HTMLInputElement::DoneCreatingElement() { // 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 diff --git a/dom/html/HTMLSelectElement.cpp b/dom/html/HTMLSelectElement.cpp index cb31b26c50cc..77431e60a7d2 100644 --- a/dom/html/HTMLSelectElement.cpp +++ b/dom/html/HTMLSelectElement.cpp @@ -1113,10 +1113,8 @@ void HTMLSelectElement::DoneAddingChildren(bool aHaveNotified) { } 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 diff --git a/dom/html/HTMLTextAreaElement.cpp b/dom/html/HTMLTextAreaElement.cpp index d8753622dbd7..624fed979a0c 100644 --- a/dom/html/HTMLTextAreaElement.cpp +++ b/dom/html/HTMLTextAreaElement.cpp @@ -518,10 +518,8 @@ void HTMLTextAreaElement::DoneAddingChildren(bool aHaveNotified) { } if (!mInhibitStateRestoration) { - nsresult rv = GenerateStateKey(); - if (NS_SUCCEEDED(rv)) { - RestoreFormControlState(); - } + GenerateStateKey(); + RestoreFormControlState(); } } diff --git a/dom/html/nsGenericHTMLElement.cpp b/dom/html/nsGenericHTMLElement.cpp index f06dcbe6920a..d7c36c2dbb9f 100644 --- a/dom/html/nsGenericHTMLElement.cpp +++ b/dom/html/nsGenericHTMLElement.cpp @@ -2525,24 +2525,19 @@ nsGenericHTMLFormElementWithState::nsGenericHTMLFormElementWithState( 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. @@ -2550,7 +2545,6 @@ nsresult nsGenericHTMLFormElementWithState::GenerateStateKey() { // Add something unique to content so layout doesn't muck us up. mStateKey += "-C"; } - return NS_OK; } PresState* nsGenericHTMLFormElementWithState::GetPrimaryPresState() { diff --git a/dom/html/nsGenericHTMLElement.h b/dom/html/nsGenericHTMLElement.h index 2c2eac99138e..e7840201dfc6 100644 --- a/dom/html/nsGenericHTMLElement.h +++ b/dom/html/nsGenericHTMLElement.h @@ -1117,8 +1117,8 @@ class nsGenericHTMLFormElementWithState : public nsGenericHTMLFormElement { 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 */ diff --git a/layout/base/nsFrameManager.cpp b/layout/base/nsFrameManager.cpp index b3d35b9ed728..c5c08bc8f14d 100644 --- a/layout/base/nsFrameManager.cpp +++ b/layout/base/nsFrameManager.cpp @@ -146,8 +146,8 @@ void nsFrameManager::CaptureFrameStateFor(nsIFrame* aFrame, 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; } @@ -207,8 +207,8 @@ void nsFrameManager::RestoreFrameStateFor(nsIFrame* aFrame, 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; } @@ -219,7 +219,7 @@ void nsFrameManager::RestoreFrameStateFor(nsIFrame* aFrame, } // Restore it - rv = statefulFrame->RestoreState(frameState); + nsresult rv = statefulFrame->RestoreState(frameState); if (NS_FAILED(rv)) { return; } diff --git a/layout/forms/nsComboboxControlFrame.cpp b/layout/forms/nsComboboxControlFrame.cpp index a476fd02ba71..76fff7492fad 100644 --- a/layout/forms/nsComboboxControlFrame.cpp +++ b/layout/forms/nsComboboxControlFrame.cpp @@ -1569,16 +1569,14 @@ nsComboboxControlFrame::RestoreState(PresState* aState) { // 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. diff --git a/layout/forms/nsComboboxControlFrame.h b/layout/forms/nsComboboxControlFrame.h index d15d263c59c0..4ccc1b97ad40 100644 --- a/layout/forms/nsComboboxControlFrame.h +++ b/layout/forms/nsComboboxControlFrame.h @@ -209,9 +209,8 @@ class nsComboboxControlFrame final : public nsBlockFrame, mozilla::UniquePtr 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(); diff --git a/layout/generic/nsIStatefulFrame.h b/layout/generic/nsIStatefulFrame.h index 1e152b0075ce..33a24e5e05d0 100644 --- a/layout/generic/nsIStatefulFrame.h +++ b/layout/generic/nsIStatefulFrame.h @@ -30,10 +30,10 @@ class nsIStatefulFrame { 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); }; };