Bug 1344629 - Part 6: Rewrite unnecessary uses of nsLiteralString. r=…

…dbaron

There's an antipattern where nsLiteralString is used as an unnecessary intermediary in converting from CharT* to CharT*,
e.g. CallAFunctionThatTakesACharPointer(NS_LITERAL_CSTRING("foo").get());
or
NS_NAMED_LITERAL_STRING(foo, "abc");
CallAFunctionThatTakesACharPointer(foo.get());

This patch rewrites the callsites that can be trivially changed to use char*/char16_t*.

I'd somewhat like to remove nsTLiteralString::get() altogether, but in code that's less straightforward than these examples, get() is useful enough to keep.

MozReview-Commit-ID: Kh1rUziVllo

browser/components/shell/nsWindowsShellService.cpp

@@ -166,14 +166,14 @@ nsWindowsShellService::ShortcutMaintenance()
   if (NS_FAILED(taskbarInfo->GetDefaultGroupId(appId)))
     return NS_ERROR_UNEXPECTED;
 
-  NS_NAMED_LITERAL_CSTRING(prefName, "browser.taskbar.lastgroupid");
+  const char* prefName = "browser.taskbar.lastgroupid";
   nsCOMPtr<nsIPrefBranch> prefs =
     do_GetService(NS_PREFSERVICE_CONTRACTID);
   if (!prefs)
     return NS_ERROR_UNEXPECTED;
 
   nsCOMPtr<nsISupportsString> prefString;
-  rv = prefs->GetComplexValue(prefName.get(),
+  rv = prefs->GetComplexValue(prefName,
                               NS_GET_IID(nsISupportsString),
                               getter_AddRefs(prefString));
   if (NS_SUCCEEDED(rv)) {
@@ -191,7 +191,7 @@ nsWindowsShellService::ShortcutMaintenance()
     return rv;
 
   prefString->SetData(appId);
-  rv = prefs->SetComplexValue(prefName.get(),
+  rv = prefs->SetComplexValue(prefName,
                               NS_GET_IID(nsISupportsString),
                               prefString);
   if (NS_FAILED(rv)) {

dom/base/nsJSEnvironment.cpp

@@ -1632,11 +1632,11 @@ nsJSContext::EndCycleCollectionCallback(CycleCollectorResults &aResults)
       gcMsg.AssignLiteral(", forced a GC");
     }
 
-    NS_NAMED_LITERAL_STRING(kFmt,
+    const char16_t *kFmt =
       u"CC(T+%.1f)[%s-%i] max pause: %lums, total time: %lums, slices: %lu, suspected: %lu, visited: %lu RCed and %lu%s GCed, collected: %lu RCed and %lu GCed (%lu|%lu|%lu waiting for GC)%s\n"
-      u"ForgetSkippable %lu times before CC, min: %lu ms, max: %lu ms, avg: %lu ms, total: %lu ms, max sync: %lu ms, removed: %lu");
+      u"ForgetSkippable %lu times before CC, min: %lu ms, max: %lu ms, avg: %lu ms, total: %lu ms, max sync: %lu ms, removed: %lu";
     nsString msg;
-    msg.Adopt(nsTextFormatter::smprintf(kFmt.get(), double(delta) / PR_USEC_PER_SEC,
+    msg.Adopt(nsTextFormatter::smprintf(kFmt, double(delta) / PR_USEC_PER_SEC,
                                         ProcessNameForCollectorLog(), getpid(),
                                         gCCStats.mMaxSliceTime, gCCStats.mTotalSliceTime,
                                         aResults.mNumSlices, gCCStats.mSuspected,
@@ -1664,7 +1664,7 @@ nsJSContext::EndCycleCollectionCallback(CycleCollectorResults &aResults)
   }
 
   if (sPostGCEventsToObserver) {
-    NS_NAMED_LITERAL_STRING(kJSONFmt,
+    const char16_t* kJSONFmt =
        u"{ \"timestamp\": %llu, "
          u"\"duration\": %lu, "
          u"\"max_slice_pause\": %lu, "
@@ -1689,10 +1689,10 @@ nsJSContext::EndCycleCollectionCallback(CycleCollectorResults &aResults)
              u"\"avg\": %lu, "
              u"\"total\": %lu, "
              u"\"removed\": %lu } "
-       u"}");
+       u"}";
     nsString json;
 
-    json.Adopt(nsTextFormatter::smprintf(kJSONFmt.get(), PR_Now(), ccNowDuration,
+    json.Adopt(nsTextFormatter::smprintf(kJSONFmt, PR_Now(), ccNowDuration,
                                          gCCStats.mMaxSliceTime,
                                          gCCStats.mTotalSliceTime,
                                          gCCStats.mMaxGCDuration,
@@ -2139,10 +2139,9 @@ DOMGCSliceCallback(JSContext* aCx, JS::GCProgress aProgress, const JS::GCDescrip
       PRTime delta = GetCollectionTimeDelta();
 
       if (sPostGCEventsToConsole) {
-        NS_NAMED_LITERAL_STRING(kFmt, "GC(T+%.1f)[%s-%i] ");
         nsString prefix, gcstats;
         gcstats.Adopt(aDesc.formatSummaryMessage(aCx));
-        prefix.Adopt(nsTextFormatter::smprintf(kFmt.get(),
+        prefix.Adopt(nsTextFormatter::smprintf(u"GC(T+%.1f)[%s-%i] ",
                                                double(delta) / PR_USEC_PER_SEC,
                                                ProcessNameForCollectorLog(),
                                                getpid()));
@@ -2223,10 +2222,9 @@ DOMGCSliceCallback(JSContext* aCx, JS::GCProgress aProgress, const JS::GCDescrip
       }
 
       if (sPostGCEventsToConsole) {
-        NS_NAMED_LITERAL_STRING(kFmt, "[%s-%i] ");
         nsString prefix, gcstats;
         gcstats.Adopt(aDesc.formatSliceMessage(aCx));
-        prefix.Adopt(nsTextFormatter::smprintf(kFmt.get(),
+        prefix.Adopt(nsTextFormatter::smprintf(u"[%s-%i] ",
                                                ProcessNameForCollectorLog(),
                                                getpid()));
         nsString msg = prefix + gcstats;

dom/html/ImageDocument.cpp

@@ -585,8 +585,7 @@ ImageDocument::OnLoadComplete(imgIRequest* aRequest, nsresult aStatus)
     NS_ConvertUTF8toUTF16 srcString(src);
     const char16_t* formatString[] = { srcString.get() };
     nsXPIDLString errorMsg;
-    NS_NAMED_LITERAL_STRING(str, "InvalidImage");
-    mStringBundle->FormatStringFromName(str.get(), formatString, 1,
+    mStringBundle->FormatStringFromName(u"InvalidImage", formatString, 1,
                                         getter_Copies(errorMsg));
 
     mImageContent->SetAttr(kNameSpaceID_None, nsGkAtoms::alt, errorMsg, false);

dom/html/MediaDocument.cpp

@@ -418,8 +418,7 @@ MediaDocument::UpdateTitleAndCharset(const nsACString& aTypeStr,
     nsXPIDLString titleWithStatus;
     const nsPromiseFlatString& status = PromiseFlatString(aStatus);
     const char16_t *formatStrings[2] = {title.get(), status.get()};
-    NS_NAMED_LITERAL_STRING(fmtName, "TitleWithStatus");
-    mStringBundle->FormatStringFromName(fmtName.get(), formatStrings, 2,
+    mStringBundle->FormatStringFromName(u"TitleWithStatus", formatStrings, 2,
                                         getter_Copies(titleWithStatus));
     SetTitle(titleWithStatus);
   }

dom/power/PowerManagerService.cpp

@@ -124,12 +124,12 @@ PowerManagerService::SyncProfile()
 {
   nsCOMPtr<nsIObserverService> obsServ = services::GetObserverService();
   if (obsServ) {
-    NS_NAMED_LITERAL_STRING(context, "shutdown-persist");
-    obsServ->NotifyObservers(nullptr, "profile-change-net-teardown", context.get());
-    obsServ->NotifyObservers(nullptr, "profile-change-teardown", context.get());
-    obsServ->NotifyObservers(nullptr, "profile-before-change", context.get());
-    obsServ->NotifyObservers(nullptr, "profile-before-change-qm", context.get());
-    obsServ->NotifyObservers(nullptr, "profile-before-change-telemetry", context.get());
+    const char16_t* context = u"shutdown-persist";
+    obsServ->NotifyObservers(nullptr, "profile-change-net-teardown", context);
+    obsServ->NotifyObservers(nullptr, "profile-change-teardown", context);
+    obsServ->NotifyObservers(nullptr, "profile-before-change", context);
+    obsServ->NotifyObservers(nullptr, "profile-before-change-qm", context);
+    obsServ->NotifyObservers(nullptr, "profile-before-change-telemetry", context);
   }
 }
 

dom/security/nsCSPParser.cpp

@@ -1116,7 +1116,7 @@ nsCSPParser::directiveName()
 
   // if we have a frame-src, cache it so we can decide whether to use child-src
   if (CSP_IsDirective(mCurToken, nsIContentSecurityPolicy::FRAME_SRC_DIRECTIVE)) {
-    const char16_t* params[] = { mCurToken.get(), NS_LITERAL_STRING("child-src").get() };
+    const char16_t* params[] = { mCurToken.get(), u"child-src" };
     logWarningErrorToConsole(nsIScriptError::warningFlag, "deprecatedDirective",
                              params, ArrayLength(params));
     mFrameSrc = new nsCSPDirective(CSP_StringToCSPDirective(mCurToken));

dom/xhr/XMLHttpRequestMainThread.cpp

@@ -2588,11 +2588,10 @@ XMLHttpRequestMainThread::InitiateFetch(nsIInputStream* aUploadStream,
       nsCOMPtr<nsIConsoleService> consoleService =
         do_GetService(NS_CONSOLESERVICE_CONTRACTID);
       if (consoleService) {
-        consoleService->LogStringMessage(NS_LITERAL_STRING(
-          "Http channel implementation doesn't support nsIUploadChannel2. "
+        consoleService->LogStringMessage(
+          u"Http channel implementation doesn't support nsIUploadChannel2. "
           "An extension has supplied a non-functional http protocol handler. "
-          "This will break behavior and in future releases not work at all."
-        ).get());
+          "This will break behavior and in future releases not work at all.");
       }
     }
 

layout/generic/nsPageFrame.cpp

@@ -193,7 +193,7 @@ nsPageFrame::ProcessSpecialCodes(const nsString& aStr, nsString& aNewStr)
   // then subst in the current date/time
   NS_NAMED_LITERAL_STRING(kDate, "&D");
   if (aStr.Find(kDate) != kNotFound) {
-    aNewStr.ReplaceSubstring(kDate.get(), mPD->mDateTimeStr.get());
+    aNewStr.ReplaceSubstring(kDate, mPD->mDateTimeStr);
   }
 
   // NOTE: Must search for &PT before searching for &P
@@ -204,7 +204,7 @@ nsPageFrame::ProcessSpecialCodes(const nsString& aStr, nsString& aNewStr)
   NS_NAMED_LITERAL_STRING(kPageAndTotal, "&PT");
   if (aStr.Find(kPageAndTotal) != kNotFound) {
     char16_t * uStr = nsTextFormatter::smprintf(mPD->mPageNumAndTotalsFormat.get(), mPageNum, mTotNumPages);
-    aNewStr.ReplaceSubstring(kPageAndTotal.get(), uStr);
+    aNewStr.ReplaceSubstring(kPageAndTotal, nsDependentString(uStr));
     free(uStr);
   }
 
@@ -213,24 +213,24 @@ nsPageFrame::ProcessSpecialCodes(const nsString& aStr, nsString& aNewStr)
   NS_NAMED_LITERAL_STRING(kPage, "&P");
   if (aStr.Find(kPage) != kNotFound) {
     char16_t * uStr = nsTextFormatter::smprintf(mPD->mPageNumFormat.get(), mPageNum);
-    aNewStr.ReplaceSubstring(kPage.get(), uStr);
+    aNewStr.ReplaceSubstring(kPage, nsDependentString(uStr));
     free(uStr);
   }
 
   NS_NAMED_LITERAL_STRING(kTitle, "&T");
   if (aStr.Find(kTitle) != kNotFound) {
-    aNewStr.ReplaceSubstring(kTitle.get(), mPD->mDocTitle.get());
+    aNewStr.ReplaceSubstring(kTitle, mPD->mDocTitle);
   }
 
   NS_NAMED_LITERAL_STRING(kDocURL, "&U");
   if (aStr.Find(kDocURL) != kNotFound) {
-    aNewStr.ReplaceSubstring(kDocURL.get(), mPD->mDocURL.get());
+    aNewStr.ReplaceSubstring(kDocURL, mPD->mDocURL);
   }
 
   NS_NAMED_LITERAL_STRING(kPageTotal, "&L");
   if (aStr.Find(kPageTotal) != kNotFound) {
     char16_t * uStr = nsTextFormatter::smprintf(mPD->mPageNumFormat.get(), mTotNumPages);
-    aNewStr.ReplaceSubstring(kPageTotal.get(), uStr);
+    aNewStr.ReplaceSubstring(kPageTotal, nsDependentString(uStr));
     free(uStr);
   }
 }

media/webrtc/signaling/src/peerconnection/PeerConnectionCtx.cpp

@@ -85,12 +85,10 @@ class PeerConnectionCtxObserver : public nsIObserver
       PeerConnectionCtx::gPeerConnectionCtxObserver = nullptr;
     }
     if (strcmp(aTopic, NS_IOSERVICE_OFFLINE_STATUS_TOPIC) == 0) {
-      const nsLiteralString onlineString(u"" NS_IOSERVICE_ONLINE);
-      const nsLiteralString offlineString(u"" NS_IOSERVICE_OFFLINE);
-      if (NS_strcmp(aData, offlineString.get()) == 0) {
+      if (NS_strcmp(aData, u"" NS_IOSERVICE_OFFLINE) == 0) {
         CSFLogDebug(logTag, "Updating network state to offline");
         PeerConnectionCtx::UpdateNetworkState(false);
-      } else if(NS_strcmp(aData, onlineString.get()) == 0) {
+      } else if(NS_strcmp(aData, u"" NS_IOSERVICE_ONLINE) == 0) {
         CSFLogDebug(logTag, "Updating network state to online");
         PeerConnectionCtx::UpdateNetworkState(true);
       } else {

netwerk/base/nsIOService.cpp

@@ -834,9 +834,10 @@ nsIOService::NewChannelFromURIWithProxyFlagsInternal(nsIURI* aURI,
             nsCOMPtr<nsIConsoleService> consoleService =
                 do_GetService(NS_CONSOLESERVICE_CONTRACTID);
             if (consoleService) {
-                consoleService->LogStringMessage(NS_LITERAL_STRING(
-                    "Http channel implementation doesn't support nsIUploadChannel2. An extension has supplied a non-functional http protocol handler. This will break behavior and in future releases not work at all."
-                                                                   ).get());
+                consoleService->LogStringMessage(u"Http channel implementation "
+                    "doesn't support nsIUploadChannel2. An extension has "
+                    "supplied a non-functional http protocol handler. This will "
+                    "break behavior and in future releases not work at all.");
             }
             gHasWarnedUploadChannel2 = true;
         }
@@ -1062,14 +1063,13 @@ nsIOService::SetOffline(bool offline)
         offline = mSetOfflineValue;
 
         if (offline && !mOffline) {
-            NS_NAMED_LITERAL_STRING(offlineString, NS_IOSERVICE_OFFLINE);
             mOffline = true; // indicate we're trying to shutdown
 
             // don't care if notifications fail
             if (observerService)
                 observerService->NotifyObservers(subject,
                                                  NS_IOSERVICE_GOING_OFFLINE_TOPIC,
-                                                 offlineString.get());
+                                                 u"" NS_IOSERVICE_OFFLINE);
 
             if (mSocketTransportService)
                 mSocketTransportService->SetOffline(true);
@@ -1078,7 +1078,7 @@ nsIOService::SetOffline(bool offline)
             if (observerService)
                 observerService->NotifyObservers(subject,
                                                  NS_IOSERVICE_OFFLINE_STATUS_TOPIC,
-                                                 offlineString.get());
+                                                 u"" NS_IOSERVICE_OFFLINE);
         }
         else if (!offline && mOffline) {
             // go online
@@ -1193,13 +1193,12 @@ nsIOService::SetConnectivityInternal(bool aConnectivity)
     } else {
         // If we were previously online and lost connectivity
         // send the OFFLINE notification
-        const nsLiteralString offlineString(u"" NS_IOSERVICE_OFFLINE);
         observerService->NotifyObservers(static_cast<nsIIOService *>(this),
                                          NS_IOSERVICE_GOING_OFFLINE_TOPIC,
-                                         offlineString.get());
+                                         u"" NS_IOSERVICE_OFFLINE);
         observerService->NotifyObservers(static_cast<nsIIOService *>(this),
                                          NS_IOSERVICE_OFFLINE_STATUS_TOPIC,
-                                         offlineString.get());
+                                         u"" NS_IOSERVICE_OFFLINE);
     }
     return NS_OK;
 }

netwerk/protocol/ftp/nsFtpConnectionThread.cpp

@@ -946,10 +946,9 @@ nsFtpState::R_syst() {
 
             char16_t* ucs2Response = ToNewUnicode(mResponseMsg);
             const char16_t *formatStrings[1] = { ucs2Response };
-            NS_NAMED_LITERAL_STRING(name, "UnsupportedFTPServer");
 
             nsXPIDLString formattedString;
-            rv = bundle->FormatStringFromName(name.get(), formatStrings, 1,
+            rv = bundle->FormatStringFromName(u"UnsupportedFTPServer", formatStrings, 1,
                                               getter_Copies(formattedString));
             free(ucs2Response);
             if (NS_FAILED(rv))

security/manager/ssl/nsNSSComponent.cpp

@@ -1848,12 +1848,10 @@ nsNSSComponent::Init()
   // - wrong thread: 'NS_IsMainThread()' in nsIOService.cpp
   // when loading error strings on the SSL threads.
   {
-    NS_NAMED_LITERAL_STRING(dummy_name, "dummy");
+    const char16_t* dummy = u"dummy";
     nsXPIDLString result;
-    mPIPNSSBundle->GetStringFromName(dummy_name.get(),
-                                     getter_Copies(result));
-    mNSSErrorsBundle->GetStringFromName(dummy_name.get(),
-                                        getter_Copies(result));
+    mPIPNSSBundle->GetStringFromName(dummy, getter_Copies(result));
+    mNSSErrorsBundle->GetStringFromName(dummy, getter_Copies(result));
   }
 
 

security/manager/ssl/nsNSSU2FToken.cpp

@@ -29,7 +29,7 @@ const nsCString nsNSSU2FToken::mSecretNickname =
   NS_LITERAL_CSTRING("U2F_NSSTOKEN");
 const nsString nsNSSU2FToken::mVersion =
   NS_LITERAL_STRING("U2F_V2");
-NS_NAMED_LITERAL_CSTRING(kAttestCertSubjectName, "CN=Firefox U2F Soft Token");
+const char* kAttestCertSubjectName = "CN=Firefox U2F Soft Token";
 
 // This U2F-compatible soft token uses FIDO U2F-compatible ECDSA keypairs
 // on the SEC_OID_SECG_EC_SECP256R1 curve. When asked to Register, it will
@@ -256,7 +256,7 @@ GetAttestationCertificate(const UniquePK11SlotInfo& aSlot,
   }
 
   // Construct the Attestation Certificate itself
-  UniqueCERTName subjectName(CERT_AsciiToName(kAttestCertSubjectName.get()));
+  UniqueCERTName subjectName(CERT_AsciiToName(kAttestCertSubjectName));
   if (!subjectName) {
     MOZ_LOG(gNSSTokenLog, LogLevel::Warning,
             ("Failed to set subject name, NSS error #%d", PORT_GetError()));

storage/VacuumManager.cpp

@@ -29,8 +29,8 @@
 
 // Used to notify begin and end of a heavy IO task.
 #define OBSERVER_TOPIC_HEAVY_IO "heavy-io-task"
-#define OBSERVER_DATA_VACUUM_BEGIN NS_LITERAL_STRING("vacuum-begin")
-#define OBSERVER_DATA_VACUUM_END NS_LITERAL_STRING("vacuum-end")
+#define OBSERVER_DATA_VACUUM_BEGIN u"vacuum-begin"
+#define OBSERVER_DATA_VACUUM_END u"vacuum-end"
 
 // This preferences root will contain last vacuum timestamps (in seconds) for
 // each database.  The database filename is used as a key.
@@ -191,7 +191,7 @@ Vacuumer::execute()
   if (os) {
     rv =
       os->NotifyObservers(nullptr, OBSERVER_TOPIC_HEAVY_IO,
-                          OBSERVER_DATA_VACUUM_BEGIN.get());
+                          OBSERVER_DATA_VACUUM_BEGIN);
     MOZ_ASSERT(NS_SUCCEEDED(rv), "Should be able to notify");
   }
 
@@ -289,7 +289,7 @@ Vacuumer::notifyCompletion(bool aSucceeded)
   nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
   if (os) {
     os->NotifyObservers(nullptr, OBSERVER_TOPIC_HEAVY_IO,
-                        OBSERVER_DATA_VACUUM_END.get());
+                        OBSERVER_DATA_VACUUM_END);
   }
 
   nsresult rv = mParticipant->OnEndVacuum(aSucceeded);

toolkit/components/find/nsWebBrowserFind.cpp

@@ -92,11 +92,9 @@ nsWebBrowserFind::FindNext(bool* aResult)
     NS_ENSURE_SUCCESS(rv, rv);
     nsCOMPtr<nsISupports> searchWindowSupports = do_QueryInterface(rootFrame);
     windowSupportsData->SetData(searchWindowSupports);
-    NS_NAMED_LITERAL_STRING(dnStr, "down");
-    NS_NAMED_LITERAL_STRING(upStr, "up");
     observerSvc->NotifyObservers(windowSupportsData,
                                  "nsWebBrowserFind_FindAgain",
-                                 mFindBackwards ? upStr.get() : dnStr.get());
+                                 mFindBackwards ? u"up" : u"down");
     windowSupportsData->GetData(getter_AddRefs(searchWindowSupports));
     // findnext performed if search window data cleared out
     *aResult = searchWindowSupports == nullptr;

toolkit/components/startup/nsAppStartup.cpp

@@ -458,11 +458,8 @@ nsAppStartup::Quit(uint32_t aMode)
     // No chance of the shutdown being cancelled from here on; tell people
     // we're shutting down for sure while all services are still available.
     if (obsService) {
-      NS_NAMED_LITERAL_STRING(shutdownStr, "shutdown");
-      NS_NAMED_LITERAL_STRING(restartStr, "restart");
       obsService->NotifyObservers(nullptr, "quit-application",
-        (mRestart || mRestartNotSameProfile) ?
-         restartStr.get() : shutdownStr.get());
+        (mRestart || mRestartNotSameProfile) ? u"restart" : u"shutdown");
     }
 
     if (!mRunning) {

toolkit/xre/nsEmbedFunctions.cpp

@@ -583,7 +583,7 @@ XRE_InitChildProcess(int aArgc,
       nsString appId;
       appId.AssignWithConversion(nsDependentCString(appModelUserId));
       // The version string is encased in quotes
-      appId.Trim(NS_LITERAL_CSTRING("\"").get());
+      appId.Trim("\"");
       // Set the id
       SetTaskbarGroupId(appId);
     }

toolkit/xre/nsNativeAppSupportWin.cpp

@@ -953,7 +953,7 @@ nsNativeAppSupportWin::HandleDDENotification( UINT uType,       // transaction t
                     do {
                         // Get most recently used Nav window.
                         nsCOMPtr<mozIDOMWindowProxy> navWin;
-                        GetMostRecentWindow( NS_LITERAL_STRING( "navigator:browser" ).get(),
+                        GetMostRecentWindow( u"navigator:browser",
                                              getter_AddRefs( navWin ) );
                         nsCOMPtr<nsPIDOMWindowOuter> piNavWin = do_QueryInterface(navWin);
                         if ( !piNavWin ) {
@@ -1430,7 +1430,7 @@ nsNativeAppSupportWin::OpenBrowserWindow()
     // browser window.
 
     nsCOMPtr<mozIDOMWindowProxy> navWin;
-    GetMostRecentWindow( NS_LITERAL_STRING( "navigator:browser" ).get(), getter_AddRefs( navWin ) );
+    GetMostRecentWindow( u"navigator:browser", getter_AddRefs( navWin ) );
 
     // This isn't really a loop.  We just use "break" statements to fall
     // out to the OpenWindow call when things go awry.