Bug 1450882 - part 2: Make nsICommandParams::GetCStringValue() and nsICommandParams::SetCStringValue() treat nsACString instead of char r?ehsan draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 10 Jul 2018 18:04:46 +0900
changeset 815995 e80852b2c767d39b301d36ce0c8be66a71ac8bce
parent 815994 4ae7c4abd12f750420dd1a7685548267d29a4167
child 815996 cbc58ec7ce42fccae04f9681fda940e4d7c7e206
push id115711
push usermasayuki@d-toybox.com
push dateTue, 10 Jul 2018 12:46:56 +0000
reviewersehsan
bugs1450882
milestone63.0a1
Bug 1450882 - part 2: Make nsICommandParams::GetCStringValue() and nsICommandParams::SetCStringValue() treat nsACString instead of char r?ehsan nsICommandParams::GetCStringValue() and nsICommandParams::SetCStringValue() treat char. However, this makes their callers complicated. So, they should be rewritten as treating nsACString. MozReview-Commit-ID: DWO9veSyzyG
dom/base/nsGlobalWindowCommands.cpp
dom/commandhandler/nsCommandParams.cpp
dom/commandhandler/nsICommandParams.idl
dom/html/nsHTMLDocument.cpp
editor/libeditor/HTMLEditorCommands.cpp
editor/libeditor/HTMLEditorDocumentCommands.cpp
--- a/dom/base/nsGlobalWindowCommands.cpp
+++ b/dom/base/nsGlobalWindowCommands.cpp
@@ -772,19 +772,20 @@ nsClipboardGetContentsCommand::IsClipboa
 
 nsresult
 nsClipboardGetContentsCommand::DoClipboardCommand(const char *aCommandName, nsIContentViewerEdit* aEdit, nsICommandParams* aParams)
 {
   NS_ENSURE_ARG(aParams);
 
   nsAutoCString mimeType("text/plain");
 
-  nsCString format;    // nsICommandParams needs to use nsACString
-  if (NS_SUCCEEDED(aParams->GetCStringValue("format", getter_Copies(format))))
+  nsAutoCString format;
+  if (NS_SUCCEEDED(aParams->GetCStringValue("format", format))) {
     mimeType.Assign(format);
+  }
 
   bool selectionOnly = false;
   aParams->GetBooleanValue("selection_only", &selectionOnly);
 
   nsAutoString contents;
   nsresult rv = aEdit->GetContents(mimeType.get(), selectionOnly, contents);
   if (NS_FAILED(rv))
     return rv;
--- a/dom/commandhandler/nsCommandParams.cpp
+++ b/dom/commandhandler/nsCommandParams.cpp
@@ -98,27 +98,25 @@ nsCommandParams::GetStringValue(const ch
     aRetVal.Assign(*foundEntry->mData.mString);
     return NS_OK;
   }
   aRetVal.Truncate();
   return NS_ERROR_FAILURE;
 }
 
 NS_IMETHODIMP
-nsCommandParams::GetCStringValue(const char* aName, char** aRetVal)
+nsCommandParams::GetCStringValue(const char* aName, nsACString& aRetVal)
 {
-  NS_ENSURE_ARG_POINTER(aRetVal);
-
   HashEntry* foundEntry = GetNamedEntry(aName);
   if (foundEntry && foundEntry->mEntryType == eStringType) {
     NS_ASSERTION(foundEntry->mData.mCString, "Null string");
-    *aRetVal = ToNewCString(*foundEntry->mData.mCString);
+    aRetVal.Assign(*foundEntry->mData.mCString);
     return NS_OK;
   }
-  *aRetVal = nullptr;
+  aRetVal.Truncate();
   return NS_ERROR_FAILURE;
 }
 
 NS_IMETHODIMP
 nsCommandParams::GetISupportsValue(const char* aName, nsISupports** aRetVal)
 {
   NS_ENSURE_ARG_POINTER(aRetVal);
 
@@ -171,17 +169,17 @@ nsCommandParams::SetStringValue(const ch
   if (!foundEntry) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
   foundEntry->mData.mString = new nsString(aValue);
   return NS_OK;
 }
 
 NS_IMETHODIMP
-nsCommandParams::SetCStringValue(const char* aName, const char* aValue)
+nsCommandParams::SetCStringValue(const char* aName, const nsACString& aValue)
 {
   HashEntry* foundEntry = GetOrMakeEntry(aName, eStringType);
   if (!foundEntry) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
   foundEntry->mData.mCString = new nsCString(aValue);
   return NS_OK;
 }
--- a/dom/commandhandler/nsICommandParams.idl
+++ b/dom/commandhandler/nsICommandParams.idl
@@ -43,17 +43,17 @@ interface nsICommandParams : nsISupports
    * as documented for the command. It is permissible
    * for it to contain nsICommandParams, but not *this*
    * one (i.e. self-containing is not allowed).
    */
   boolean     getBooleanValue(in string name);
   long        getLongValue(in string name);
   double      getDoubleValue(in string name);
   AString     getStringValue(in string name);
-  string      getCStringValue(in string name);
+  ACString    getCStringValue(in string name);
   nsISupports getISupportsValue(in string name);
 
   /*
    * set_Value
    *
    * Set the value of a specified parameter (thus creating
    * an entry for it).
    *
@@ -61,17 +61,17 @@ interface nsICommandParams : nsISupports
    * as documented for the command. It is permissible
    * for it to contain nsICommandParams, but not *this*
    * one (i.e. self-containing is not allowed).
    */
   void        setBooleanValue(in string name, in boolean value);
   void        setLongValue(in string name, in long value);
   void        setDoubleValue(in string name, in double value);
   void        setStringValue(in string name, in AString value);
-  void        setCStringValue(in string name, in string value);
+  void        setCStringValue(in string name, in ACString value);
   void        setISupportsValue(in string name, in nsISupports value);
 
   /*
    * removeValue
    *
    * Remove the specified parameter from the list.
    */
   void        removeValue(in string name);
--- a/dom/html/nsHTMLDocument.cpp
+++ b/dom/html/nsHTMLDocument.cpp
@@ -3001,17 +3001,17 @@ nsHTMLDocument::ExecCommand(const nsAStr
     if (isBool) {
       rv = cmdParams->SetBooleanValue("state_attribute", boolVal);
     } else if (cmdToDispatch.EqualsLiteral("cmd_fontFace")) {
       rv = cmdParams->SetStringValue("state_attribute", value);
     } else if (cmdToDispatch.EqualsLiteral("cmd_insertHTML") ||
                cmdToDispatch.EqualsLiteral("cmd_insertText")) {
       rv = cmdParams->SetStringValue("state_data", value);
     } else {
-      rv = cmdParams->SetCStringValue("state_attribute", paramStr.get());
+      rv = cmdParams->SetCStringValue("state_attribute", paramStr);
     }
     if (rv.Failed()) {
       return false;
     }
     rv = cmdMgr->DoCommand(cmdToDispatch.get(), cmdParams, window);
   }
 
   return !rv.Failed();
@@ -3162,26 +3162,20 @@ nsHTMLDocument::QueryCommandState(const 
 
   // handle alignment as a special case (possibly other commands too?)
   // Alignment is special because the external api is individual
   // commands but internally we use cmd_align with different
   // parameters.  When getting the state of this command, we need to
   // return the boolean for this particular alignment rather than the
   // string of 'which alignment is this?'
   if (cmdToDispatch.EqualsLiteral("cmd_align")) {
-    char * actualAlignmentType = nullptr;
-    rv = cmdParams->GetCStringValue("state_attribute", &actualAlignmentType);
-    bool retval = false;
-    if (!rv.Failed() && actualAlignmentType && actualAlignmentType[0]) {
-      retval = paramToCheck.Equals(actualAlignmentType);
-    }
-    if (actualAlignmentType) {
-      free(actualAlignmentType);
-    }
-    return retval;
+    nsAutoCString actualAlignmentType;
+    rv = cmdParams->GetCStringValue("state_attribute", actualAlignmentType);
+    return !rv.Failed() && !actualAlignmentType.IsEmpty() &&
+           paramToCheck == actualAlignmentType;
   }
 
   // If command does not have a state_all value, this call fails and sets
   // retval to false.  This is fine -- we want to return false in that case
   // anyway (bug 738385), so we just succeed and return false regardless.
   bool retval = false;
   cmdParams->GetBooleanValue("state_all", &retval);
   return retval;
@@ -3259,46 +3253,45 @@ nsHTMLDocument::QueryCommandValue(const 
 
   // this is a special command since we are calling DoCommand rather than
   // GetCommandState like the other commands
   if (cmdToDispatch.EqualsLiteral("cmd_getContents")) {
     rv = cmdParams->SetBooleanValue("selection_only", true);
     if (rv.Failed()) {
       return;
     }
-    rv = cmdParams->SetCStringValue("format", "text/html");
+    rv = cmdParams->SetCStringValue("format", NS_LITERAL_CSTRING("text/html"));
     if (rv.Failed()) {
       return;
     }
     rv = cmdMgr->DoCommand(cmdToDispatch.get(), cmdParams, window);
     if (rv.Failed()) {
       return;
     }
     rv = cmdParams->GetStringValue("result", aValue);
     return;
   }
 
-  rv = cmdParams->SetCStringValue("state_attribute", paramStr.get());
+  rv = cmdParams->SetCStringValue("state_attribute", paramStr);
   if (rv.Failed()) {
     return;
   }
 
   rv = cmdMgr->GetCommandState(cmdToDispatch.get(), window, cmdParams);
   if (rv.Failed()) {
     return;
   }
 
   // If command does not have a state_attribute value, this call fails, and
   // aValue will wind up being the empty string.  This is fine -- we want to
   // return "" in that case anyway (bug 738385), so we just return NS_OK
   // regardless.
-  nsCString cStringResult;
-  cmdParams->GetCStringValue("state_attribute",
-                             getter_Copies(cStringResult));
-  CopyUTF8toUTF16(cStringResult, aValue);
+  nsAutoCString result;
+  cmdParams->GetCStringValue("state_attribute", result);
+  CopyUTF8toUTF16(result, aValue);
 }
 
 nsresult
 nsHTMLDocument::Clone(mozilla::dom::NodeInfo *aNodeInfo, nsINode **aResult,
                       bool aPreallocateChildren) const
 {
   NS_ASSERTION(aNodeInfo->NodeInfoManager() == mNodeInfoManager,
                "Can't import this document into another document!");
--- a/editor/libeditor/HTMLEditorCommands.cpp
+++ b/editor/libeditor/HTMLEditorCommands.cpp
@@ -622,26 +622,27 @@ MultiStateCommandBase::DoCommandParams(c
   if (!editor) {
     return NS_OK;
   }
   mozilla::HTMLEditor* htmlEditor = editor->AsHTMLEditor();
   if (NS_WARN_IF(!htmlEditor)) {
     return NS_ERROR_FAILURE;
   }
 
-  nsAutoString tString;
+  nsAutoString attribute;
   if (aParams) {
-    nsCString s;
-    nsresult rv = aParams->GetCStringValue(STATE_ATTRIBUTE, getter_Copies(s));
-    if (NS_SUCCEEDED(rv))
-      CopyASCIItoUTF16(s, tString);
-    else
-      aParams->GetStringValue(STATE_ATTRIBUTE, tString);
+    nsAutoCString asciiAttribute;
+    nsresult rv = aParams->GetCStringValue(STATE_ATTRIBUTE, asciiAttribute);
+    if (NS_SUCCEEDED(rv)) {
+      CopyASCIItoUTF16(asciiAttribute, attribute);
+    } else {
+      aParams->GetStringValue(STATE_ATTRIBUTE, attribute);
+    }
   }
-  return SetState(htmlEditor, tString);
+  return SetState(htmlEditor, attribute);
 }
 
 NS_IMETHODIMP
 MultiStateCommandBase::GetCommandStateParams(const char* aCommandName,
                                              nsICommandParams* aParams,
                                              nsISupports* refCon)
 {
   nsCOMPtr<nsIEditor> editor = do_QueryInterface(refCon);
@@ -670,17 +671,17 @@ ParagraphStateCommand::GetCurrentState(H
 
   bool outMixed;
   nsAutoString outStateString;
   nsresult rv = aHTMLEditor->GetParagraphState(&outMixed, outStateString);
   if (NS_SUCCEEDED(rv)) {
     nsAutoCString tOutStateString;
     LossyCopyUTF16toASCII(outStateString, tOutStateString);
     aParams->SetBooleanValue(STATE_MIXED,outMixed);
-    aParams->SetCStringValue(STATE_ATTRIBUTE, tOutStateString.get());
+    aParams->SetCStringValue(STATE_ATTRIBUTE, tOutStateString);
   }
   return rv;
 }
 
 nsresult
 ParagraphStateCommand::SetState(HTMLEditor* aHTMLEditor,
                                 const nsString& newState)
 {
@@ -703,17 +704,18 @@ FontFaceStateCommand::GetCurrentState(HT
     return NS_ERROR_INVALID_ARG;
   }
 
   nsAutoString outStateString;
   bool outMixed;
   nsresult rv = aHTMLEditor->GetFontFaceState(&outMixed, outStateString);
   if (NS_SUCCEEDED(rv)) {
     aParams->SetBooleanValue(STATE_MIXED,outMixed);
-    aParams->SetCStringValue(STATE_ATTRIBUTE, NS_ConvertUTF16toUTF8(outStateString).get());
+    aParams->SetCStringValue(STATE_ATTRIBUTE,
+                             NS_ConvertUTF16toUTF8(outStateString));
   }
   return rv;
 }
 
 nsresult
 FontFaceStateCommand::SetState(HTMLEditor* aHTMLEditor,
                                const nsString& newState)
 {
@@ -763,17 +765,17 @@ FontSizeStateCommand::GetCurrentState(HT
                                EmptyString(),
                                &firstHas, &anyHas, &allHas,
                                outStateString);
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsAutoCString tOutStateString;
   LossyCopyUTF16toASCII(outStateString, tOutStateString);
   aParams->SetBooleanValue(STATE_MIXED, anyHas && !allHas);
-  aParams->SetCStringValue(STATE_ATTRIBUTE, tOutStateString.get());
+  aParams->SetCStringValue(STATE_ATTRIBUTE, tOutStateString);
   aParams->SetBooleanValue(STATE_ENABLED, true);
 
   return rv;
 }
 
 
 // acceptable values for "newState" are:
 //   -2
@@ -826,17 +828,17 @@ FontColorStateCommand::GetCurrentState(H
   bool outMixed;
   nsAutoString outStateString;
   nsresult rv = aHTMLEditor->GetFontColorState(&outMixed, outStateString);
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsAutoCString tOutStateString;
   LossyCopyUTF16toASCII(outStateString, tOutStateString);
   aParams->SetBooleanValue(STATE_MIXED, outMixed);
-  aParams->SetCStringValue(STATE_ATTRIBUTE, tOutStateString.get());
+  aParams->SetCStringValue(STATE_ATTRIBUTE, tOutStateString);
   return NS_OK;
 }
 
 nsresult
 FontColorStateCommand::SetState(HTMLEditor* aHTMLEditor,
                                 const nsString& newState)
 {
   if (NS_WARN_IF(!aHTMLEditor)) {
@@ -868,17 +870,17 @@ HighlightColorStateCommand::GetCurrentSt
   bool outMixed;
   nsAutoString outStateString;
   nsresult rv = aHTMLEditor->GetHighlightColorState(&outMixed, outStateString);
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsAutoCString tOutStateString;
   LossyCopyUTF16toASCII(outStateString, tOutStateString);
   aParams->SetBooleanValue(STATE_MIXED, outMixed);
-  aParams->SetCStringValue(STATE_ATTRIBUTE, tOutStateString.get());
+  aParams->SetCStringValue(STATE_ATTRIBUTE, tOutStateString);
   return NS_OK;
 }
 
 nsresult
 HighlightColorStateCommand::SetState(HTMLEditor* aHTMLEditor,
                                      const nsString& newState)
 {
   if (NS_WARN_IF(!aHTMLEditor)) {
@@ -927,17 +929,17 @@ BackgroundColorStateCommand::GetCurrentS
   bool outMixed;
   nsAutoString outStateString;
   nsresult rv = aHTMLEditor->GetBackgroundColorState(&outMixed, outStateString);
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsAutoCString tOutStateString;
   LossyCopyUTF16toASCII(outStateString, tOutStateString);
   aParams->SetBooleanValue(STATE_MIXED, outMixed);
-  aParams->SetCStringValue(STATE_ATTRIBUTE, tOutStateString.get());
+  aParams->SetCStringValue(STATE_ATTRIBUTE, tOutStateString);
   return NS_OK;
 }
 
 nsresult
 BackgroundColorStateCommand::SetState(HTMLEditor* aHTMLEditor,
                                       const nsString& newState)
 {
   if (NS_WARN_IF(!aHTMLEditor)) {
@@ -982,17 +984,17 @@ AlignCommand::GetCurrentState(HTMLEditor
 
     case nsIHTMLEditor::eJustify:
       outStateString.AssignLiteral("justify");
       break;
   }
   nsAutoCString tOutStateString;
   LossyCopyUTF16toASCII(outStateString, tOutStateString);
   aParams->SetBooleanValue(STATE_MIXED,outMixed);
-  aParams->SetCStringValue(STATE_ATTRIBUTE, tOutStateString.get());
+  aParams->SetCStringValue(STATE_ATTRIBUTE, tOutStateString);
   return NS_OK;
 }
 
 nsresult
 AlignCommand::SetState(HTMLEditor* aHTMLEditor,
                        const nsString& newState)
 {
   if (NS_WARN_IF(!aHTMLEditor)) {
@@ -1034,24 +1036,26 @@ AbsolutePositioningCommand::GetCurrentSt
 {
   if (NS_WARN_IF(!aHTMLEditor)) {
     return NS_ERROR_INVALID_ARG;
   }
 
   bool isEnabled = aHTMLEditor->AbsolutePositioningEnabled();
   if (!isEnabled) {
     aParams->SetBooleanValue(STATE_MIXED,false);
-    aParams->SetCStringValue(STATE_ATTRIBUTE, "");
+    aParams->SetCStringValue(STATE_ATTRIBUTE, EmptyCString());
     return NS_OK;
   }
 
   RefPtr<Element> container =
     aHTMLEditor->GetAbsolutelyPositionedSelectionContainer();
   aParams->SetBooleanValue(STATE_MIXED,  false);
-  aParams->SetCStringValue(STATE_ATTRIBUTE, container ? "absolute" : "");
+  aParams->SetCStringValue(STATE_ATTRIBUTE,
+                           container ? NS_LITERAL_CSTRING("absolute") :
+                                       EmptyCString());
   return NS_OK;
 }
 
 nsresult
 AbsolutePositioningCommand::ToggleState(HTMLEditor* aHTMLEditor)
 {
   if (NS_WARN_IF(!aHTMLEditor)) {
     return NS_ERROR_INVALID_ARG;
@@ -1484,24 +1488,27 @@ InsertTagCommand::DoCommandParams(const 
     return NS_ERROR_FAILURE;
   }
   mozilla::HTMLEditor* htmlEditor = editor->AsHTMLEditor();
   if (NS_WARN_IF(!htmlEditor)) {
     return NS_ERROR_FAILURE;
   }
 
   // do we have an href to use for creating link?
-  nsCString s;
-  nsresult rv = aParams->GetCStringValue(STATE_ATTRIBUTE, getter_Copies(s));
-  NS_ENSURE_SUCCESS(rv, rv);
-  nsAutoString attrib;
-  CopyASCIItoUTF16(s, attrib);
+  nsAutoCString asciiAttribute;
+  nsresult rv = aParams->GetCStringValue(STATE_ATTRIBUTE, asciiAttribute);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+  nsAutoString attribute;
+  CopyASCIItoUTF16(asciiAttribute, attribute);
 
-  if (attrib.IsEmpty())
+  if (attribute.IsEmpty()) {
     return NS_ERROR_INVALID_ARG;
+  }
 
   // filter out tags we don't know how to insert
   nsAutoString attributeType;
   if (mTagName == nsGkAtoms::a) {
     attributeType.AssignLiteral("href");
   } else if (mTagName == nsGkAtoms::img) {
     attributeType.AssignLiteral("src");
   } else {
@@ -1509,17 +1516,17 @@ InsertTagCommand::DoCommandParams(const 
   }
 
   RefPtr<Element> elem;
   rv = htmlEditor->CreateElementWithDefaults(nsDependentAtomString(mTagName),
                                              getter_AddRefs(elem));
   NS_ENSURE_SUCCESS(rv, rv);
 
   ErrorResult err;
-  elem->SetAttribute(attributeType, attrib, err);
+  elem->SetAttribute(attributeType, attribute, err);
   if (NS_WARN_IF(err.Failed())) {
     return err.StealNSResult();
   }
 
   // do actual insertion
   if (mTagName == nsGkAtoms::a) {
     return htmlEditor->InsertLinkAroundSelection(elem);
   }
--- a/editor/libeditor/HTMLEditorDocumentCommands.cpp
+++ b/editor/libeditor/HTMLEditorDocumentCommands.cpp
@@ -251,19 +251,18 @@ SetDocumentStateCommand::DoCommandParams
     if (NS_WARN_IF(!aParams)) {
       return NS_ERROR_NULL_POINTER;
     }
     HTMLEditor* htmlEditor = textEditor->AsHTMLEditor();
     if (NS_WARN_IF(!htmlEditor)) {
       return NS_ERROR_INVALID_ARG;
     }
 
-    nsCString newValue;
-    nsresult rv = aParams->GetCStringValue(STATE_ATTRIBUTE,
-                                           getter_Copies(newValue));
+    nsAutoCString newValue;
+    nsresult rv = aParams->GetCStringValue(STATE_ATTRIBUTE, newValue);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
 
     if (newValue.LowerCaseEqualsLiteral("div")) {
       htmlEditor->SetDefaultParagraphSeparator(ParagraphSeparator::div);
       return NS_OK;
     }
@@ -378,25 +377,25 @@ SetDocumentStateCommand::GetCommandState
     }
     HTMLEditor* htmlEditor = textEditor->AsHTMLEditor();
     if (NS_WARN_IF(!htmlEditor)) {
       return NS_ERROR_INVALID_ARG;
     }
 
     switch (htmlEditor->GetDefaultParagraphSeparator()) {
       case ParagraphSeparator::div:
-        aParams->SetCStringValue(STATE_ATTRIBUTE, "div");
+        aParams->SetCStringValue(STATE_ATTRIBUTE, NS_LITERAL_CSTRING("div"));
         return NS_OK;
 
       case ParagraphSeparator::p:
-        aParams->SetCStringValue(STATE_ATTRIBUTE, "p");
+        aParams->SetCStringValue(STATE_ATTRIBUTE, NS_LITERAL_CSTRING("p"));
         return NS_OK;
 
       case ParagraphSeparator::br:
-        aParams->SetCStringValue(STATE_ATTRIBUTE, "br");
+        aParams->SetCStringValue(STATE_ATTRIBUTE, NS_LITERAL_CSTRING("br"));
         return NS_OK;
 
       default:
         MOZ_ASSERT_UNREACHABLE("Invalid paragraph separator value");
         return NS_ERROR_UNEXPECTED;
     }
   }