Do minor string conversion cleanups

A general ban of conversions can't be done, as for both wide strings
and Mac strings the pattern of passing a const char* into the
base::Sys conversion functions is unavoidable. However, some cleanup
can be done as well as adding compiler indications to the functions.

Bug: 1440840
Change-Id: I7d00c65410122df1dfcced72317f78b97d704c99
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4903845
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Marlon Facey <mfacey@chromium.org>
Auto-Submit: Avi Drissman <avi@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1205997}

base/strings/sys_string_conversions_apple_unittest.mm

@@ -23,15 +23,15 @@ TEST(SysStrings, ConversionsFromNSString) {
 
 std::vector<std::string> GetRoundTripStrings() {
   return {
-      "Hello, World!",  // ASCII / ISO8859 string (also valid UTF-8)
+      "Hello, World!",  // ASCII / ISO-8859 string (also valid UTF-8)
       "a\0b",           // UTF-8 with embedded NUL byte
       "λf",             // lowercase lambda + 'f'
       "χρώμιο",         // "chromium" in greek
       "כרום",           // "chromium" in hebrew
       "クロム",         // "chromium" in japanese
 
-      // Tarot card symbol "the morning", which does not fit in one UTF-16
-      // character.
+      // Tarot card symbol "the morning", which is outside of the BMP and is not
+      // representable with one UTF-16 code unit.
       "🃦",
   };
 }

base/strings/sys_string_conversions_unittest.cc

@@ -25,7 +25,8 @@ TEST(SysStrings, SysWideToUTF8) {
   EXPECT_EQ("Hello, world", SysWideToUTF8(L"Hello, world"));
   EXPECT_EQ("\xe4\xbd\xa0\xe5\xa5\xbd", SysWideToUTF8(L"\x4f60\x597d"));
 
-  // >16 bits
+  // A value outside of the BMP and therefore not representable with one UTF-16
+  // code unit.
   EXPECT_EQ("\xF0\x90\x8C\x80", SysWideToUTF8(kSysWideOldItalicLetterA));
 
   // Error case. When Windows finds a UTF-16 character going off the end of
@@ -52,7 +53,9 @@ TEST(SysStrings, SysWideToUTF8) {
 TEST(SysStrings, SysUTF8ToWide) {
   EXPECT_EQ(L"Hello, world", SysUTF8ToWide("Hello, world"));
   EXPECT_EQ(L"\x4f60\x597d", SysUTF8ToWide("\xe4\xbd\xa0\xe5\xa5\xbd"));
-  // >16 bits
+
+  // A value outside of the BMP and therefore not representable with one UTF-16
+  // code unit.
   EXPECT_EQ(kSysWideOldItalicLetterA, SysUTF8ToWide("\xF0\x90\x8C\x80"));
 
   // Error case. When Windows finds an invalid UTF-8 character, it just skips
@@ -83,7 +86,8 @@ TEST(SysStrings, SysWideToNativeMB) {
   EXPECT_EQ("Hello, world", SysWideToNativeMB(L"Hello, world"));
   EXPECT_EQ("\xe4\xbd\xa0\xe5\xa5\xbd", SysWideToNativeMB(L"\x4f60\x597d"));
 
-  // >16 bits
+  // A value outside of the BMP and therefore not representable with one UTF-16
+  // code unit.
   EXPECT_EQ("\xF0\x90\x8C\x80", SysWideToNativeMB(kSysWideOldItalicLetterA));
 
   // Error case. When Windows finds a UTF-16 character going off the end of
@@ -114,7 +118,9 @@ TEST(SysStrings, SysNativeMBToWide) {
 #endif
   EXPECT_EQ(L"Hello, world", SysNativeMBToWide("Hello, world"));
   EXPECT_EQ(L"\x4f60\x597d", SysNativeMBToWide("\xe4\xbd\xa0\xe5\xa5\xbd"));
-  // >16 bits
+
+  // A value outside of the BMP and therefore not representable with one UTF-16
+  // code unit.
   EXPECT_EQ(kSysWideOldItalicLetterA, SysNativeMBToWide("\xF0\x90\x8C\x80"));
 
   // Error case. When Windows finds an invalid UTF-8 character, it just skips

base/strings/utf_string_conversions.h

@@ -67,27 +67,24 @@ BASE_EXPORT bool UTF16ToUTF8(const char16_t* src,
 
 // The conversion functions in this file should not be used to convert string
 // literals. Instead, the corresponding prefixes (e.g. u"" for UTF16 or L"" for
-// Wide) should be used. Deleting the overloads here catches these cases at
-// compile time.
+// Wide) should be used. Catch those cases with overloads that assert at compile
+// time.
 template <size_t N>
-std::u16string WideToUTF16(const wchar_t (&str)[N]) {
+[[noreturn]] std::u16string WideToUTF16(const wchar_t (&str)[N]) {
   static_assert(AlwaysFalse<decltype(N)>,
-                "Error: Use the u\"...\" prefix instead.");
-  return std::u16string();
+                "Error: Use u\"...\" to create a std::u16string literal.");
 }
 
 template <size_t N>
-std::u16string UTF8ToUTF16(const char (&str)[N]) {
+[[noreturn]] std::u16string UTF8ToUTF16(const char (&str)[N]) {
   static_assert(AlwaysFalse<decltype(N)>,
-                "Error: Use the u\"...\" prefix instead.");
-  return std::u16string();
+                "Error: Use u\"...\" to create a std::u16string literal.");
 }
 
 template <size_t N>
-std::u16string ASCIIToUTF16(const char (&str)[N]) {
+[[noreturn]] std::u16string ASCIIToUTF16(const char (&str)[N]) {
   static_assert(AlwaysFalse<decltype(N)>,
-                "Error: Use the u\"...\" prefix instead.");
-  return std::u16string();
+                "Error: Use u\"...\" to create a std::u16string literal.");
 }
 
 // Mutable character arrays are usually only populated during runtime. Continue

chrome/browser/enterprise/connectors/device_trust/key_management/core/mac/secure_enclave_client_unittest.mm

@@ -69,8 +69,7 @@ class SecureEnclaveClientTest : public testing::Test {
         CFDictionaryCreateMutable(kCFAllocatorDefault, 0,
                                   &kCFTypeDictionaryKeyCallBacks,
                                   &kCFTypeDictionaryValueCallBacks));
-    CFDictionarySetValue(test_attributes, kSecAttrLabel,
-                         base::SysUTF8ToCFStringRef("fake-label"));
+    CFDictionarySetValue(test_attributes, kSecAttrLabel, CFSTR("fake-label"));
     CFDictionarySetValue(test_attributes, kSecAttrKeyType,
                          kSecAttrKeyTypeECSECPrimeRandom);
     CFDictionarySetValue(test_attributes, kSecAttrKeySizeInBits,

components/policy/core/common/policy_loader_mac_unittest.cc

@@ -189,10 +189,9 @@ TEST_F(PolicyLoaderMacTest, Invalid) {
 TEST_F(PolicyLoaderMacTest, TestNonForcedValue) {
   ScopedCFTypeRef<CFStringRef> name(
       base::SysUTF8ToCFStringRef(test_keys::kKeyString));
-  ScopedCFTypeRef<CFPropertyListRef> test_value(
-      base::SysUTF8ToCFStringRef("string value"));
-  ASSERT_TRUE(test_value.get());
-  prefs_->AddTestItem(name, test_value.get(), /*is_forced=*/false,
+  CFPropertyListRef test_value = CFSTR("string value");
+  ASSERT_TRUE(test_value);
+  prefs_->AddTestItem(name, test_value, /*is_forced=*/false,
                       /*is_machine=*/true);
 
   // Make the provider read the updated |prefs_|.
@@ -208,10 +207,9 @@ TEST_F(PolicyLoaderMacTest, TestNonForcedValue) {
 TEST_F(PolicyLoaderMacTest, TestUserScopeValue) {
   ScopedCFTypeRef<CFStringRef> name(
       base::SysUTF8ToCFStringRef(test_keys::kKeyString));
-  ScopedCFTypeRef<CFPropertyListRef> test_value(
-      base::SysUTF8ToCFStringRef("string value"));
-  ASSERT_TRUE(test_value.get());
-  prefs_->AddTestItem(name, test_value.get(), /*is_forced=*/true,
+  CFPropertyListRef test_value = CFSTR("string value");
+  ASSERT_TRUE(test_value);
+  prefs_->AddTestItem(name, test_value, /*is_forced=*/true,
                       /*is_machine=*/false);
 
   // Make the provider read the updated |prefs_|.

ios/chrome/browser/ui/omnibox/popup/omnibox_popup_egtest.mm

@@ -576,7 +576,7 @@ std::unique_ptr<net::test_server::HttpResponse> StandardResponse(
   [ChromeEarlGrey loadURL:GURL("about:blank")];
 
   // Clears the url and replace it with local url host.
-  [ChromeEarlGreyUI focusOmniboxAndType:base::SysUTF8ToNSString("abc")];
+  [ChromeEarlGreyUI focusOmniboxAndType:@"abc"];
 
   // Wait for the suggestions to show.
   [ChromeEarlGrey waitForUIElementToAppearWithMatcher:
@@ -633,7 +633,7 @@ std::unique_ptr<net::test_server::HttpResponse> StandardResponse(
 - (void)testUpDownArrowAutocomplete {
   // Focus omnibox from Web.
   [ChromeEarlGrey loadURL:GURL("about:blank")];
-  [ChromeEarlGreyUI focusOmniboxAndType:base::SysUTF8ToNSString("testupdown")];
+  [ChromeEarlGreyUI focusOmniboxAndType:@"testupdown"];
 
   // Matcher for the first autocomplete suggestions.
   id<GREYMatcher> testupDownAutocomplete1 =
@@ -681,8 +681,7 @@ std::unique_ptr<net::test_server::HttpResponse> StandardResponse(
 
   // Typing the title of page1.
   [[EarlGrey selectElementWithMatcher:chrome_test_util::Omnibox()]
-      performAction:grey_replaceText(
-                        base::SysUTF8ToNSString(std::string(kPage1Title)))];
+      performAction:grey_replaceText(base::SysUTF8ToNSString(kPage1Title))];
 
   // Wait for suggestions to show.
   [ChromeEarlGrey