Bug 1450882 - part 2: Make nsICommandParams::GetCStringValue() and nsICommandParams::SetCStringValue() treat nsACString instead of char r=Ehsan
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 10 Jul 2018 18:04:46 +0900
changeset 426440 cc00b4d8d55784d343ae3779ad7ad4223543b0c9
parent 426439 1a9c6868329019bd23b15c4eafb44cc00edf225e
child 426441 da27408845b366b1547d27539f23cd25936c4980
push id34272
push userebalazs@mozilla.com
push dateFri, 13 Jul 2018 08:51:04 +0000
treeherdermozilla-central@254564563107 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersEhsan
bugs1450882
milestone63.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 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
@@ -3015,17 +3015,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();
@@ -3176,26 +3176,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;
@@ -3273,46 +3267,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;
     }
   }