Bug 345339. Stop using a generic hashtable to store element session history state in nsPresState. patch by Karthik Sarma, r+sr=roc,a=sicking

content/html/content/src/nsHTMLButtonElement.cpp

@@ -595,14 +595,7 @@ nsHTMLButtonElement::SaveState()
   if (state) {
     PRBool disabled;
     GetDisabled(&disabled);
-    if (disabled) {
+    state->SetDisabled(disabled);
-      rv |= state->SetStateProperty(NS_LITERAL_STRING("disabled"),
-                                    NS_LITERAL_STRING("t"));
-    } else {
-      rv |= state->SetStateProperty(NS_LITERAL_STRING("disabled"),
-                                    NS_LITERAL_STRING("f"));
-    }
-    NS_ASSERTION(NS_SUCCEEDED(rv), "disabled save failed!");
   }
 
   return rv;
@@ -611,13 +604,9 @@ nsHTMLButtonElement::SaveState()
 PRBool
 nsHTMLButtonElement::RestoreState(nsPresState* aState)
 {
-  nsAutoString disabled;
+  NS_ENSURE_ARG_POINTER(aState);
-  nsresult rv =
+
-    aState->GetStateProperty(NS_LITERAL_STRING("disabled"), disabled);
+  SetDisabled(aState->GetDisabled());
-  NS_ASSERTION(NS_SUCCEEDED(rv), "disabled restore failed!");
-  if (rv == NS_STATE_PROPERTY_EXISTS) {
-    SetDisabled(disabled.EqualsLiteral("t"));
-  }
 
   return PR_FALSE;
 }

content/html/content/src/nsHTMLInputElement.cpp

@@ -35,6 +35,9 @@
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
+
+#include "nsISupportsPrimitives.h"
+#include "nsISupportsImpl.h"
 #include "nsCOMPtr.h"
 #include "nsIDOMHTMLInputElement.h"
 #include "nsIDOMNSHTMLInputElement.h"
@@ -143,6 +146,55 @@ static NS_DEFINE_CID(kXULControllersCID,  NS_XULCONTROLLERS_CID);
 
 static const char kWhitespace[] = "\n\r\t\b";
 
+#define NS_INPUT_ELEMENT_STATE_IID                 \
+{ /* dc3b3d14-23e2-4479-b513-7b369343e3a0 */       \
+  0xdc3b3d14,                                      \
+  0x23e2,                                          \
+  0x4479,                                          \
+  {0xb5, 0x13, 0x7b, 0x36, 0x93, 0x43, 0xe3, 0xa0} \
+}
+
+class nsHTMLInputElementState : public nsISupports
+{
+  public:
+    NS_DECLARE_STATIC_IID_ACCESSOR(NS_INPUT_ELEMENT_STATE_IID)
+    NS_DECL_ISUPPORTS
+
+    enum { UNKNOWN = -1 };
+    
+    void SetChecked(PRInt8 aChecked) {
+      mChecked = aChecked;
+    }
+    PRInt8 GetChecked() {
+      return mChecked;
+    }
+
+    void SetValue(const nsAString &aValue) {
+     mValue = aValue;
+    }
+    const nsString& GetValue() {
+      return mValue;
+    }
+
+    void SetFileName(const nsAString &aFilename) {
+      mFilename = aFilename;
+    }
+    const nsString& GetFileName() {
+      return mFilename;
+    }
+
+    nsHTMLInputElementState() : mValue(), mFilename(), mChecked(UNKNOWN) {};
+ 
+  protected:
+    nsString mValue;
+    nsString mFilename;
+    PRInt8 mChecked;
+};
+
+NS_IMPL_ISUPPORTS0(nsHTMLInputElementState)
+NS_DEFINE_STATIC_IID_ACCESSOR(nsHTMLInputElementState,
+                              NS_INPUT_ELEMENT_STATE_IID)
+
 class nsHTMLInputElement : public nsGenericHTMLFormElement,
                            public nsImageLoadingContent,
                            public nsIDOMHTMLInputElement,
@@ -2573,13 +2625,19 @@ nsHTMLInputElement::SaveState()
 {
   nsresult rv = NS_OK;
   
+  nsRefPtr<nsHTMLInputElementState> inputState
+    (new nsHTMLInputElementState());
+
+  if (!inputState)
+    return NS_ERROR_OUT_OF_MEMORY;
+
   nsPresState *state = nsnull;
   switch (mType) {
     case NS_FORM_INPUT_CHECKBOX:
     case NS_FORM_INPUT_RADIO:
       {
         PRBool checked = PR_FALSE;
-        GetChecked(&checked);
+        rv = GetChecked(&checked);
         PRBool defaultChecked = PR_FALSE;
         GetDefaultChecked(&defaultChecked);
         // Only save if checked != defaultChecked (bug 62713)
@@ -2588,14 +2646,7 @@ nsHTMLInputElement::SaveState()
         if (mType == NS_FORM_INPUT_RADIO || checked != defaultChecked) {
           rv = GetPrimaryPresState(this, &state);
           if (state) {
-            if (checked) {
+            inputState->SetChecked(checked);
-              rv = state->SetStateProperty(NS_LITERAL_STRING("checked"),
-                                           NS_LITERAL_STRING("t"));
-            } else {
-              rv = state->SetStateProperty(NS_LITERAL_STRING("checked"),
-                                           NS_LITERAL_STRING("f"));
-            }
-            NS_ASSERTION(NS_SUCCEEDED(rv), "checked save failed!");
           }
         }
         break;
@@ -2617,8 +2668,7 @@ nsHTMLInputElement::SaveState()
                      nsLinebreakConverter::eLinebreakPlatform,
                      nsLinebreakConverter::eLinebreakContent);
             NS_ASSERTION(NS_SUCCEEDED(rv), "Converting linebreaks failed!");
-            rv = state->SetStateProperty(NS_LITERAL_STRING("v"), value);
+            inputState->SetValue(value);
-            NS_ASSERTION(NS_SUCCEEDED(rv), "value save failed!");
           }
         }
         break;
@@ -2628,8 +2678,7 @@ nsHTMLInputElement::SaveState()
         if (mFileName) {
           rv = GetPrimaryPresState(this, &state);
           if (state) {
-            rv = state->SetStateProperty(NS_LITERAL_STRING("f"), *mFileName);
+            inputState->SetFileName(*mFileName);
-            NS_ASSERTION(NS_SUCCEEDED(rv), "value save failed!");
           }
         }
         break;
@@ -2641,15 +2690,13 @@ nsHTMLInputElement::SaveState()
     if (state) {
       PRBool disabled;
       GetDisabled(&disabled);
-      if (disabled) {
+      state->SetDisabled(disabled);
-        rv |= state->SetStateProperty(NS_LITERAL_STRING("disabled"),
-                                     NS_LITERAL_STRING("t"));
-      } else {
-        rv |= state->SetStateProperty(NS_LITERAL_STRING("disabled"),
-                                     NS_LITERAL_STRING("f"));
     }
-      NS_ASSERTION(NS_SUCCEEDED(rv), "disabled save failed!");
   }
+  
+  rv |= GetPrimaryPresState(this, &state);
+  if (state) {
+     state->SetStateProperty(inputState);
   }
 
   return rv;
@@ -2714,18 +2761,19 @@ nsHTMLInputElement::RestoreState(nsPresState* aState)
 {
   PRBool restoredCheckedState = PR_FALSE;
 
-  nsresult rv;
+  nsCOMPtr<nsHTMLInputElementState> inputState
+    (do_QueryInterface(aState->GetStateProperty()));
+  if (!inputState)
+    return PR_FALSE;
 
   switch (mType) {
     case NS_FORM_INPUT_CHECKBOX:
     case NS_FORM_INPUT_RADIO:
       {
-        nsAutoString checked;
+        PRInt8 checked = inputState->GetChecked();
-        rv = aState->GetStateProperty(NS_LITERAL_STRING("checked"), checked);
+        if (checked != nsHTMLInputElementState::UNKNOWN) {
-        NS_ASSERTION(NS_SUCCEEDED(rv), "checked restore failed!");
-        if (rv == NS_STATE_PROPERTY_EXISTS) {
           restoredCheckedState = PR_TRUE; 
-          DoSetChecked(checked.EqualsLiteral("t"), PR_FALSE);
+          DoSetChecked(checked, PR_FALSE);
         }
         break;
       }
@@ -2733,31 +2781,18 @@ nsHTMLInputElement::RestoreState(nsPresState* aState)
     case NS_FORM_INPUT_TEXT:
     case NS_FORM_INPUT_HIDDEN:
       {
-        nsAutoString value;
+        SetValueInternal(inputState->GetValue(), nsnull, PR_FALSE);
-        rv = aState->GetStateProperty(NS_LITERAL_STRING("v"), value);
-        NS_ASSERTION(NS_SUCCEEDED(rv), "value restore failed!");
-        if (rv == NS_STATE_PROPERTY_EXISTS) {
-          SetValueInternal(value, nsnull, PR_FALSE);
-        }
         break;
       }
     case NS_FORM_INPUT_FILE:
       {
-        nsAutoString value;
+        SetFileName(inputState->GetFileName());
-        rv = aState->GetStateProperty(NS_LITERAL_STRING("f"), value);
-        NS_ASSERTION(NS_SUCCEEDED(rv), "value restore failed!");
-        if (rv == NS_STATE_PROPERTY_EXISTS) {
-          SetFileName(value);
-        }
         break;
       }
   }
   
-  nsAutoString disabled;
+  if (aState->DisabledIsSet()) {
-  rv = aState->GetStateProperty(NS_LITERAL_STRING("disabled"), disabled);
+    SetDisabled(aState->GetDisabled());
-  NS_ASSERTION(NS_SUCCEEDED(rv), "disabled restore failed!");
-  if (rv == NS_STATE_PROPERTY_EXISTS) {
-    SetDisabled(disabled.EqualsLiteral("t"));
   }
 
   return restoredCheckedState;

content/html/content/src/nsHTMLSelectElement.cpp

@@ -70,6 +70,7 @@
 #include "nsEventDispatcher.h"
 
 NS_IMPL_ISUPPORTS0(nsSelectState)
+NS_DEFINE_STATIC_IID_ACCESSOR(nsSelectState, NS_SELECT_STATE_IID)
 
 //----------------------------------------------------------------------
 //
@@ -1512,21 +1513,14 @@ nsHTMLSelectElement::SaveState()
   nsPresState *presState = nsnull;
   nsresult rv = GetPrimaryPresState(this, &presState);
   if (presState) {
-    rv = presState->SetStatePropertyAsSupports(NS_LITERAL_STRING("selecteditems"),
+
-                                           state);
+    presState->SetStateProperty(state);
-    NS_ASSERTION(NS_SUCCEEDED(rv), "selecteditems set failed!");
+
 
     if (mDisabledChanged) {
       PRBool disabled;
       GetDisabled(&disabled);
-      if (disabled) {
+      presState->SetDisabled(disabled);
-        rv |= presState->SetStateProperty(NS_LITERAL_STRING("disabled"),
-                                          NS_LITERAL_STRING("t"));
-      } else {
-        rv |= presState->SetStateProperty(NS_LITERAL_STRING("disabled"),
-                                          NS_LITERAL_STRING("f"));
-      }
-      NS_ASSERTION(NS_SUCCEEDED(rv), "disabled save failed!");
     }
   }
 
@@ -1537,22 +1531,20 @@ PRBool
 nsHTMLSelectElement::RestoreState(nsPresState* aState)
 {
   // Get the presentation state object to retrieve our stuff out of.
-  nsCOMPtr<nsISupports> state;
+
-  nsresult rv = aState->GetStatePropertyAsSupports(NS_LITERAL_STRING("selecteditems"),
+  nsCOMPtr<nsSelectState> state(do_QueryInterface(aState->GetStateProperty()));
-                                                   getter_AddRefs(state));
+
-  if (NS_SUCCEEDED(rv)) {
+  if (!state) return PR_FALSE;
-    RestoreStateTo((nsSelectState*)(nsISupports*)state);
+
+  RestoreStateTo(state);
+
 
   // Don't flush, if the frame doesn't exist yet it doesn't care if
   // we're reset or not.
   DispatchContentReset();
-  }
 
-  nsAutoString disabled;
+  if (aState->DisabledIsSet()) {
-  rv = aState->GetStateProperty(NS_LITERAL_STRING("disabled"), disabled);
+    SetDisabled(aState->GetDisabled());
-  NS_ASSERTION(NS_SUCCEEDED(rv), "disabled restore failed!");
-  if (rv == NS_STATE_PROPERTY_EXISTS) {
-    SetDisabled(disabled.EqualsLiteral("t"));
   }
 
   return PR_FALSE;
@@ -1590,8 +1582,8 @@ nsHTMLSelectElement::RestoreStateTo(nsSelectState* aNewSelected)
     nsIDOMHTMLOptionElement *option = mOptions->ItemAsOption(i);
     if (option) {
       nsAutoString value;
-      option->GetValue(value);
+      nsresult rv = option->GetValue(value);
-      if (aNewSelected->ContainsOption(i, value)) {
+      if (NS_SUCCEEDED(rv) && aNewSelected->ContainsOption(i, value)) {
         SetOptionsSelectedByIndex(i, i, PR_TRUE, PR_FALSE, PR_TRUE, PR_TRUE, nsnull);
       }
     }

content/html/content/src/nsHTMLSelectElement.h

@@ -156,6 +156,15 @@ private:
 };
 
 
+#define NS_SELECT_STATE_IID                        \
+{ /* 4db54c7c-d159-455f-9d8e-f60ee466dbf3 */       \
+  0x4db54c7c,                                      \
+  0xd159,                                          \
+  0x455f,                                          \
+  {0x9d, 0x8e, 0xf6, 0x0e, 0xe4, 0x66, 0xdb, 0xf3} \
+}
+
+
 /**
  * The restore state used by select
  */
@@ -168,6 +177,7 @@ public:
   {
   }
 
+  NS_DECLARE_STATIC_IID_ACCESSOR(NS_SELECT_STATE_IID)
   NS_DECL_ISUPPORTS
 
   void PutOption(PRInt32 aIndex, const nsAString& aValue)
@@ -490,7 +500,7 @@ protected:
    * The temporary restore state in case we try to restore before parser is
    * done adding options
    */
-  nsRefPtr<nsSelectState> mRestoreState;
+  nsCOMPtr<nsSelectState> mRestoreState;
 };
 
 #endif

content/html/content/src/nsHTMLTextAreaElement.cpp

@@ -75,6 +75,7 @@
 #include "nsLayoutErrors.h"
 #include "nsStubMutationObserver.h"
 #include "nsDOMError.h"
+#include "nsISupportsPrimitives.h"
 
 static NS_DEFINE_CID(kXULControllersCID,  NS_XULCONTROLLERS_CID);
 
@@ -908,6 +909,11 @@ nsHTMLTextAreaElement::SaveState()
   if (mValueChanged) {
     rv = GetPrimaryPresState(this, &state);
     if (state) {
+      nsCOMPtr<nsISupportsString> pState
+        (do_CreateInstance(NS_SUPPORTS_STRING_CONTRACTID));
+
+      if (!pState) return NS_ERROR_OUT_OF_MEMORY;
+
       nsAutoString value;
       GetValueInternal(value, PR_TRUE);
 
@@ -915,9 +921,10 @@ nsHTMLTextAreaElement::SaveState()
                value,
                nsLinebreakConverter::eLinebreakPlatform,
                nsLinebreakConverter::eLinebreakContent);
-      NS_ASSERTION(NS_SUCCEEDED(rv), "Converting linebreaks failed!");
+      NS_ENSURE_SUCCESS(rv, rv);
-      rv = state->SetStateProperty(NS_LITERAL_STRING("value"), value);
+
-      NS_ASSERTION(NS_SUCCEEDED(rv), "value save failed!");
+      pState->SetData(value);
+      state->SetStateProperty(pState);
     }
   }
 
@@ -928,14 +935,7 @@ nsHTMLTextAreaElement::SaveState()
     if (state) {
       PRBool disabled;
       GetDisabled(&disabled);
-      if (disabled) {
+      state->SetDisabled(disabled);
-        rv |= state->SetStateProperty(NS_LITERAL_STRING("disabled"),
-                                      NS_LITERAL_STRING("t"));
-      } else {
-        rv |= state->SetStateProperty(NS_LITERAL_STRING("disabled"),
-                                      NS_LITERAL_STRING("f"));
-      }
-      NS_ASSERTION(NS_SUCCEEDED(rv), "disabled save failed!");
     }
   }
   return rv;
@@ -944,17 +944,18 @@ nsHTMLTextAreaElement::SaveState()
 PRBool
 nsHTMLTextAreaElement::RestoreState(nsPresState* aState)
 {
-  nsAutoString value;
-  nsresult rv =
-    aState->GetStateProperty(NS_LITERAL_STRING("value"), value);
-  NS_ASSERTION(NS_SUCCEEDED(rv), "value restore failed!");
-  SetValue(value);
 
-  nsAutoString disabled;
+  nsCOMPtr<nsISupportsString> state
-  rv = aState->GetStateProperty(NS_LITERAL_STRING("disabled"), disabled);
+    (do_QueryInterface(aState->GetStateProperty()));
-  NS_ASSERTION(NS_SUCCEEDED(rv), "disabled restore failed!");
+
-  if (rv == NS_STATE_PROPERTY_EXISTS) {
+  if (!state) return false;
-    SetDisabled(disabled.EqualsLiteral("t"));
+
+  nsAutoString data;
+  state->GetData(data);
+  SetValue(data);
+
+  if (aState->DisabledIsSet()) {
+    SetDisabled(aState->GetDisabled());
   }
 
   return PR_FALSE;

layout/base/nsPresState.cpp

@@ -51,75 +51,6 @@
 #include "nsString.h"
 // Implementation /////////////////////////////////////////////////////////////////
 
-nsresult
-nsPresState::Init()
-{
-  return mPropertyTable.Init(8) ? NS_OK : NS_ERROR_FAILURE;
-}
-
-nsresult
-nsPresState::GetStateProperty(const nsAString& aName, nsAString& aResult)
-{
-  nsresult rv = NS_STATE_PROPERTY_NOT_THERE;
-  aResult.Truncate();
-
-  // Retrieve from hashtable.
-  nsISupports *data = mPropertyTable.GetWeak(aName);
-
-  // Strings are stored in the table as UTF-8, to save space.
-  // XXX minimize conversions here...
-
-  nsCOMPtr<nsISupportsCString> supportsStr = do_QueryInterface(data);
-  if (supportsStr) {
-    nsCAutoString data;
-    supportsStr->GetData(data);
-
-    CopyUTF8toUTF16(data, aResult);
-    aResult.SetIsVoid(data.IsVoid());
-    rv = NS_STATE_PROPERTY_EXISTS;
-  }
-
-  return rv;
-}
-
-nsresult
-nsPresState::SetStateProperty(const nsAString& aName, const nsAString& aValue)
-{
-  // Add to hashtable
-  nsCOMPtr<nsISupportsCString> supportsStr(do_CreateInstance(NS_SUPPORTS_CSTRING_CONTRACTID));
-  NS_ENSURE_TRUE(supportsStr, NS_ERROR_OUT_OF_MEMORY);
-  NS_ConvertUTF16toUTF8 data(aValue);
-  data.SetIsVoid(aValue.IsVoid());
-  supportsStr->SetData(data);
-
-  mPropertyTable.Put(aName, supportsStr);
-  return NS_OK;
-}
-
-nsresult
-nsPresState::RemoveStateProperty(const nsAString& aName)
-{
-  mPropertyTable.Remove(aName);
-  return NS_OK;
-}
-
-nsresult
-nsPresState::GetStatePropertyAsSupports(const nsAString& aName,
-                                        nsISupports** aResult)
-{
-  // Retrieve from hashtable.
-  mPropertyTable.Get(aName, aResult);
-  return NS_OK;
-}
-
-nsresult
-nsPresState::SetStatePropertyAsSupports(const nsAString& aName,
-                                        nsISupports* aValue)
-{
-  mPropertyTable.Put(aName, aValue);
-  return NS_OK;
-}
-
 nsresult
 nsPresState::SetScrollState(const nsRect& aRect)
 {
@@ -147,20 +78,16 @@ nsPresState::GetScrollState()
 nsresult
 NS_NewPresState(nsPresState** aState)
 {
-  nsPresState *state;
+  NS_ENSURE_ARG_POINTER(aState);
+
+  nsPresState *state = new nsPresState();
 
-  *aState = nsnull;
-  state = new nsPresState();
   if (!state)
     return NS_ERROR_OUT_OF_MEMORY;
 
-  nsresult rv = state->Init();
-  if (NS_SUCCEEDED(rv))
-    *aState = state;
-  else
-    delete state;
 
-  return rv;
+  *aState = state;
+  return NS_OK;
 }
 
   

layout/base/nsPresState.h

@@ -53,21 +53,29 @@
 class nsPresState
 {
 public:
-  NS_HIDDEN_(nsresult) Init();
+  nsPresState() : mContentState(nsnull), mDisabledSet(PR_FALSE), 
+                  mScrollState(nsnull) {};
 
-  NS_HIDDEN_(nsresult) GetStatePropertyAsSupports(const nsAString& aName,
+  PRBool GetDisabled() {
-                                                  nsISupports** aResult);
+    return mDisabled;
+  }
 
-  NS_HIDDEN_(nsresult) SetStatePropertyAsSupports(const nsAString& aName,
+  PRBool DisabledIsSet() {
-                                                  nsISupports* aValue);
+    return mDisabledSet;
+  }
 
-  NS_HIDDEN_(nsresult) GetStateProperty(const nsAString& aProperty,
+  void SetDisabled(PRBool aDisabled) {
-                                        nsAString& aResult);
+    mDisabled = aDisabled;
+    mDisabledSet = PR_TRUE;
+  }
 
-  NS_HIDDEN_(nsresult) SetStateProperty(const nsAString& aProperty,
+  nsISupports* GetStateProperty() {
-                                        const nsAString& aValue);
+    return mContentState;
+  }
 
-  NS_HIDDEN_(nsresult) RemoveStateProperty(const nsAString& aProperty);
+  void SetStateProperty(nsISupports *aProperty) {
+    mContentState = aProperty;
+  }
 
   NS_HIDDEN_(nsresult) SetScrollState(const nsRect& aState);
 
@@ -75,7 +83,10 @@ public:
 
 // MEMBER VARIABLES
 protected:
-  nsInterfaceHashtable<nsStringHashKey,nsISupports> mPropertyTable;
+  nsCOMPtr<nsISupports> mContentState;
+  PRPackedBool mDisabled;
+  PRPackedBool mDisabledSet;
+
   nsAutoPtr<nsRect> mScrollState;
 };
 

layout/forms/nsIsIndexFrame.cpp

@@ -534,7 +534,13 @@ nsIsIndexFrame::SaveState(SpecialStateID aStateID, nsPresState** aState)
     // Construct a pres state and store value in it.
     res = NS_NewPresState(aState);
     NS_ENSURE_SUCCESS(res, res);
-    res = (*aState)->SetStateProperty(NS_LITERAL_STRING("value"), stateString);
+    
+    nsCOMPtr<nsISupportsString> state
+      (do_CreateInstance(NS_SUPPORTS_STRING_CONTRACTID));
+    if (!state) return NS_ERROR_OUT_OF_MEMORY;
+
+    state->SetData(stateString);
+    (*aState)->SetStateProperty(state);
   }
 
   return res;
@@ -546,10 +552,11 @@ nsIsIndexFrame::RestoreState(nsPresState* aState)
   NS_ENSURE_ARG_POINTER(aState);
 
   // Set the value to the stored state.
-  nsAutoString stateString;
+  nsCOMPtr<nsISupportsString> stateString
-  nsresult res = aState->GetStateProperty(NS_LITERAL_STRING("value"), stateString);
+    (do_QueryInterface(aState->GetStateProperty()));
-  NS_ENSURE_SUCCESS(res, res);
 
-  SetInputValue(stateString);
+  nsAutoString data;
+  stateString->GetData(data);
+  SetInputValue(data);
   return NS_OK;
 }

layout/xul/base/src/nsBoxObject.cpp

@@ -394,11 +394,8 @@ nsBoxObject::GetProperty(const PRUnichar* aPropertyName, PRUnichar** aResult)
   nsCOMPtr<nsISupportsString> supportsStr = do_QueryInterface(data);
   if (!supportsStr) 
     return NS_ERROR_FAILURE;
-  nsAutoString result;  
-  supportsStr->GetData(result);
 
-  *aResult = result.IsVoid() ? nsnull : ToNewUnicode(result);
+  return supportsStr->ToString(aResult);
-  return NS_OK;
 }
 
 NS_IMETHODIMP