Bug 893925 - Cleanup nsILayoutHistoryState and nsGenericHTMLElement::GetPrimaryPresState; r=smaug

content/html/content/src/HTMLButtonElement.cpp

@@ -505,15 +505,14 @@ HTMLButtonElement::SaveState()
     return NS_OK;
   }
   
-  nsPresState *state = nullptr;
-  nsresult rv = GetPrimaryPresState(this, &state);
+  nsPresState* state = GetPrimaryPresState();
   if (state) {
     // We do not want to save the real disabled state but the disabled
     // attribute.
     state->SetDisabled(HasAttr(kNameSpaceID_None, nsGkAtoms::disabled));
   }
 
-  return rv;
+  return NS_OK;
 }
 
 bool

content/html/content/src/HTMLInputElement.cpp

@@ -4670,20 +4670,15 @@ HTMLInputElement::SaveState()
       break;
   }
 
-  nsresult rv = NS_OK;
-  nsPresState* state = nullptr;
   if (inputState) {
-    rv = GetPrimaryPresState(this, &state);
+    nsPresState* state = GetPrimaryPresState();
     if (state) {
       state->SetStateProperty(inputState);
     }
   }
 
   if (mDisabledChanged) {
-    nsresult tmp = GetPrimaryPresState(this, &state);
-    if (NS_FAILED(tmp)) {
-      rv = tmp;
-    }
+    nsPresState* state = GetPrimaryPresState();
     if (state) {
       // We do not want to save the real disabled state but the disabled
       // attribute.
@@ -4691,7 +4686,7 @@ HTMLInputElement::SaveState()
     }
   }
 
-  return rv;
+  return NS_OK;
 }
 
 void

content/html/content/src/HTMLSelectElement.cpp

@@ -1531,8 +1531,7 @@ HTMLSelectElement::SaveState()
     }
   }
 
-  nsPresState* presState = nullptr;
-  nsresult rv = GetPrimaryPresState(this, &presState);
+  nsPresState* presState = GetPrimaryPresState();
   if (presState) {
     presState->SetStateProperty(state);
 
@@ -1543,7 +1542,7 @@ HTMLSelectElement::SaveState()
     }
   }
 
-  return rv;
+  return NS_OK;
 }
 
 bool

content/html/content/src/HTMLTextAreaElement.cpp

@@ -946,7 +946,7 @@ HTMLTextAreaElement::SaveState()
   // Only save if value != defaultValue (bug 62713)
   nsPresState *state = nullptr;
   if (mValueChanged) {
-    rv = GetPrimaryPresState(this, &state);
+    state = GetPrimaryPresState();
     if (state) {
       nsAutoString value;
       GetValueInternal(value, true);
@@ -969,7 +969,8 @@ HTMLTextAreaElement::SaveState()
 
   if (mDisabledChanged) {
     if (!state) {
-      rv = GetPrimaryPresState(this, &state);
+      state = GetPrimaryPresState();
+      rv = NS_OK;
     }
     if (state) {
       // We do not want to save the real disabled state but the disabled

content/html/content/src/nsGenericHTMLElement.cpp

@@ -1175,28 +1175,22 @@ nsGenericHTMLElement::GetFormControlFrame(bool aFlushFrames)
   return nullptr;
 }
 
-/* static */ nsresult
-nsGenericHTMLElement::GetPrimaryPresState(nsGenericHTMLElement* aContent,
-                                          nsPresState** aPresState)
+nsPresState*
+nsGenericHTMLElement::GetPrimaryPresState()
 {
-  NS_ENSURE_ARG_POINTER(aPresState);
-  *aPresState = nullptr;
-
-  nsresult result = NS_OK;
-
   nsAutoCString key;
-  nsCOMPtr<nsILayoutHistoryState> history = GetLayoutHistoryAndKey(aContent, false, key);
-
-  if (history) {
-    // Get the pres state for this key, if it doesn't exist, create one
-    result = history->GetState(key, aPresState);
-    if (!*aPresState) {
-      *aPresState = new nsPresState();
-      result = history->AddState(key, *aPresState);
-    }
+  nsCOMPtr<nsILayoutHistoryState> history = GetLayoutHistoryAndKey(this, false, key);
+  if (!history) {
+    return nullptr;
   }
 
-  return result;
+  // Get the pres state for this key, if it doesn't exist, create one
+  nsPresState* presState = history->GetState(key);
+  if (!presState) {
+    presState = new nsPresState();
+    history->AddState(key, presState);
+  }
+  return presState;
 }
 
 
@@ -1255,10 +1249,8 @@ nsGenericHTMLElement::RestoreFormControlState(nsGenericHTMLElement* aContent,
     return false;
   }
 
-  nsPresState *state;
-  // Get the pres state for this key
-  nsresult rv = history->GetState(key, &state);
-  if (NS_SUCCEEDED(rv) && state) {
+  nsPresState* state = history->GetState(key);
+  if (state) {
     bool result = aControl->RestoreState(state);
     history->RemoveState(key);
     return result;

content/html/content/src/nsGenericHTMLElement.h

@@ -625,14 +625,12 @@ public:
   static void MapScrollingAttributeInto(const nsMappedAttributes* aAttributes,
                                         nsRuleData* aData);
   /**
-   * Get the presentation state for a piece of content, or create it if it does
-   * not exist.  Generally used by SaveState().
+   * Get the presentation state for this, or create it if it does not exist.
+   * Generally used by SaveState().
    *
-   * @param aContent the content to get presentation state for.
-   * @param aPresState the presentation state (out param)
+   * @return the presentation state (out param)
    */
-  static nsresult GetPrimaryPresState(nsGenericHTMLElement* aContent,
-                                      nsPresState** aPresState);
+  nsPresState* GetPrimaryPresState();
   /**
    * Get the layout history object *and* generate the key for a particular
    * piece of content.

layout/base/nsFrameManager.cpp

@@ -489,12 +489,8 @@ nsFrameManager::CaptureFrameStateFor(nsIFrame* aFrame,
     return;
   }
 
-  // Store the state
-  rv = aState->AddState(stateKey, frameState);
-  if (NS_SUCCEEDED(rv)) {
-    // aState owns frameState now.
-    frameState.forget();
-  }
+  // Store the state. aState owns frameState now.
+  aState->AddState(stateKey, frameState.forget());
 }
 
 void
@@ -557,8 +553,7 @@ nsFrameManager::RestoreFrameStateFor(nsIFrame* aFrame,
   }
 
   // Get the state from the hash
-  nsPresState *frameState;
-  rv = aState->GetState(stateKey, &frameState);
+  nsPresState* frameState = aState->GetState(stateKey);
   if (!frameState) {
     return;
   }

layout/base/nsILayoutHistoryState.h

@@ -15,10 +15,11 @@
 #include "nsStringFwd.h"
 
 class nsPresState;
+template<typename> class already_AddRefed;
 
 #define NS_ILAYOUTHISTORYSTATE_IID \
-{ 0x003919e2, 0x5e6b, 0x4d76, \
- { 0xa9, 0x4f, 0xbc, 0x5d, 0x15, 0x5b, 0x1c, 0x67 } }
+{ 0x5208993e, 0xd812, 0x431e, \
+  { 0x95, 0x9c, 0xc3, 0x84, 0x5b, 0x6e, 0x5a, 0xce } }
 
 class nsILayoutHistoryState : public nsISupports {
  public: 
@@ -30,35 +31,35 @@ class nsILayoutHistoryState : public nsISupports {
    * It will be freed when RemoveState() is called or when the
    * LayoutHistoryState is destroyed.
    */
-  NS_IMETHOD AddState(const nsCString& aKey, nsPresState* aState) = 0;
+  virtual void AddState(const nsCString& aKey, nsPresState* aState) = 0;
 
   /**
    * Look up the state object for |aKey|.
    */
-  NS_IMETHOD GetState(const nsCString& aKey, nsPresState** aState) = 0;
+  virtual nsPresState* GetState(const nsCString& aKey) = 0;
 
   /**
    * Remove the state object for |aKey|.
    */
-  NS_IMETHOD RemoveState(const nsCString& aKey) = 0;
+  virtual void RemoveState(const nsCString& aKey) = 0;
 
   /**
    * Check whether this history has any states in it
    */
-  NS_IMETHOD_(bool) HasStates() const = 0;
+  virtual bool HasStates() const = 0;
 
   /**
    * Sets whether this history can contain only scroll position history
    * or all possible history
    */
-   NS_IMETHOD SetScrollPositionOnly(const bool aFlag) = 0;
+  virtual void SetScrollPositionOnly(const bool aFlag) = 0;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsILayoutHistoryState,
                               NS_ILAYOUTHISTORYSTATE_IID)
 
-nsresult
-NS_NewLayoutHistoryState(nsILayoutHistoryState** aState);
+already_AddRefed<nsILayoutHistoryState>
+NS_NewLayoutHistoryState();
 
 #endif /* _nsILayoutHistoryState_h */
 

layout/base/nsLayoutHistoryState.cpp

@@ -18,16 +18,25 @@ class nsLayoutHistoryState MOZ_FINAL : public nsILayoutHistoryState,
                                        public nsSupportsWeakReference
 {
 public:
-  NS_HIDDEN_(nsresult) Init();
+  nsLayoutHistoryState()
+    : mScrollPositionOnly(false)
+  {
+    mStates.Init();
+  }
 
   NS_DECL_ISUPPORTS
 
   // nsILayoutHistoryState
-  NS_IMETHOD AddState(const nsCString& aKey, nsPresState* aState);
-  NS_IMETHOD GetState(const nsCString& aKey, nsPresState** aState);
-  NS_IMETHOD RemoveState(const nsCString& aKey);
-  NS_IMETHOD_(bool) HasStates() const;
-  NS_IMETHOD SetScrollPositionOnly(const bool aFlag);
+  virtual void
+  AddState(const nsCString& aKey, nsPresState* aState) MOZ_OVERRIDE;
+  virtual nsPresState*
+  GetState(const nsCString& aKey) MOZ_OVERRIDE;
+  virtual void
+  RemoveState(const nsCString& aKey) MOZ_OVERRIDE;
+  virtual bool
+  HasStates() const MOZ_OVERRIDE;
+  virtual void
+  SetScrollPositionOnly(const bool aFlag) MOZ_OVERRIDE;
 
 
 private:
@@ -38,72 +47,51 @@ private:
 };
 
 
-nsresult
-NS_NewLayoutHistoryState(nsILayoutHistoryState** aState)
+already_AddRefed<nsILayoutHistoryState>
+NS_NewLayoutHistoryState()
 {
-  nsLayoutHistoryState *state;
-
-  *aState = nullptr;
-  state = new nsLayoutHistoryState();
-
-  NS_ADDREF(state);
-  nsresult rv = state->Init();
-  if (NS_SUCCEEDED(rv))
-    *aState = state;
-  else
-    NS_RELEASE(state);
-
-  return rv;
+  nsRefPtr<nsLayoutHistoryState> state = new nsLayoutHistoryState();
+  return state.forget();
 }
 
 NS_IMPL_ISUPPORTS2(nsLayoutHistoryState,
                    nsILayoutHistoryState,
                    nsISupportsWeakReference)
 
-nsresult
-nsLayoutHistoryState::Init()
-{
-  mScrollPositionOnly = false;
-  mStates.Init();
-  return NS_OK;
-}
-
-NS_IMETHODIMP
+void
 nsLayoutHistoryState::AddState(const nsCString& aStateKey, nsPresState* aState)
 {
   mStates.Put(aStateKey, aState);
-  return NS_OK;
 }
 
-NS_IMETHODIMP
-nsLayoutHistoryState::GetState(const nsCString& aKey, nsPresState** aState)
+nsPresState*
+nsLayoutHistoryState::GetState(const nsCString& aKey)
 {
-  bool entryExists = mStates.Get(aKey, aState);
+  nsPresState* state = nullptr;
+  bool entryExists = mStates.Get(aKey, &state);
 
   if (entryExists && mScrollPositionOnly) {
     // Ensure any state that shouldn't be restored is removed
-    (*aState)->ClearNonScrollState();
+    state->ClearNonScrollState();
   }
 
-  return NS_OK;
+  return state;
 }
 
-NS_IMETHODIMP
+void
 nsLayoutHistoryState::RemoveState(const nsCString& aKey)
 {
   mStates.Remove(aKey);
-  return NS_OK;
 }
 
-NS_IMETHODIMP_(bool)
+bool
 nsLayoutHistoryState::HasStates() const
 {
   return mStates.Count() != 0;
 }
 
-NS_IMETHODIMP
+void
 nsLayoutHistoryState::SetScrollPositionOnly(const bool aFlag)
 {
   mScrollPositionOnly = aFlag;
-  return NS_OK;
 }

layout/base/nsPresShell.cpp

@@ -3512,8 +3512,6 @@ nsIPresShell::ClearMouseCapture(nsIFrame* aFrame)
 nsresult
 PresShell::CaptureHistoryState(nsILayoutHistoryState** aState)
 {
-  nsresult rv = NS_OK;
-
   NS_PRECONDITION(nullptr != aState, "null state pointer");
 
   // We actually have to mess with the docshell here, since we want to
@@ -3534,13 +3532,7 @@ PresShell::CaptureHistoryState(nsILayoutHistoryState** aState)
   docShell->GetLayoutHistoryState(getter_AddRefs(historyState));
   if (!historyState) {
     // Create the document state object
-    rv = NS_NewLayoutHistoryState(getter_AddRefs(historyState));
-  
-    if (NS_FAILED(rv)) { 
-      *aState = nullptr;
-      return rv;
-    }    
-
+    historyState = NS_NewLayoutHistoryState();
     docShell->SetLayoutHistoryState(historyState);
   }