From d55b448a0cd367f7e4cf6596bd01db7e34ae73db Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Wed, 29 Apr 2020 20:11:30 +0000
Subject: [PATCH] Bug 1633675 - Avoid various string copies in FluentBundle
 constructor. r=zbraniecki

We were copying the string in C++, then again in Rust...

Differential Revision: https://phabricator.services.mozilla.com/D73034
---
 intl/l10n/FluentBundle.cpp              | 13 +++---
 intl/l10n/rust/fluent-ffi/src/bundle.rs | 54 ++++++++++++++++++++-----
 2 files changed, 52 insertions(+), 15 deletions(-)

diff --git a/intl/l10n/FluentBundle.cpp b/intl/l10n/FluentBundle.cpp
index 71d26d711243..d5c3dfe1cf69 100644
--- a/intl/l10n/FluentBundle.cpp
+++ b/intl/l10n/FluentBundle.cpp
@@ -7,6 +7,8 @@
 #include "FluentBundle.h"
 #include "nsContentUtils.h"
 #include "mozilla/dom/UnionTypes.h"
+#include "nsStringFwd.h"
+#include "nsTArray.h"
 #include "unicode/numberformatter.h"
 #include "unicode/datefmt.h"
 
@@ -70,12 +72,13 @@ already_AddRefed<FluentBundle> FluentBundle::Constructor(
   UniquePtr<ffi::FluentBundleRc> raw;
 
   if (aLocales.IsUTF8String()) {
-    nsTArray<nsCString> locales;
-    locales.AppendElement(aLocales.GetAsUTF8String());
-    raw.reset(ffi::fluent_bundle_new(&locales, useIsolating, &pseudoStrategy));
+    const nsACString& locale = aLocales.GetAsUTF8String();
+    raw.reset(
+        ffi::fluent_bundle_new_single(&locale, useIsolating, &pseudoStrategy));
   } else {
-    nsTArray<nsCString> locales(aLocales.GetAsUTF8StringSequence());
-    raw.reset(ffi::fluent_bundle_new(&locales, useIsolating, &pseudoStrategy));
+    const auto& locales = aLocales.GetAsUTF8StringSequence();
+    raw.reset(ffi::fluent_bundle_new(locales.Elements(), locales.Length(),
+                                     useIsolating, &pseudoStrategy));
   }
 
   if (!raw) {
diff --git a/intl/l10n/rust/fluent-ffi/src/bundle.rs b/intl/l10n/rust/fluent-ffi/src/bundle.rs
index dfc09067975f..9e1102ff31ed 100644
--- a/intl/l10n/rust/fluent-ffi/src/bundle.rs
+++ b/intl/l10n/rust/fluent-ffi/src/bundle.rs
@@ -68,21 +68,55 @@ fn format_numbers(num: &FluentValue, intls: &IntlLangMemoizer) -> Option<String>
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn fluent_bundle_new(
-    locales: &ThinVec<nsCString>,
+pub unsafe extern "C" fn fluent_bundle_new_single(
+    locale: &nsACString,
     use_isolating: bool,
     pseudo_strategy: &nsACString,
 ) -> *mut FluentBundleRc {
-    let mut langids = Vec::with_capacity(locales.len());
+    // We can use as_str_unchecked because this string comes from WebIDL and is
+    // guaranteed utf-8.
+    let id = match locale.as_str_unchecked().parse::<LanguageIdentifier>() {
+        Ok(id) => id,
+        Err(..) => return std::ptr::null_mut(),
+    };
 
+    Box::into_raw(fluent_bundle_new_internal(
+        &[id],
+        use_isolating,
+        pseudo_strategy,
+    ))
+}
+
+#[no_mangle]
+pub unsafe extern "C" fn fluent_bundle_new(
+    locales: *const nsCString,
+    locale_count: usize,
+    use_isolating: bool,
+    pseudo_strategy: &nsACString,
+) -> *mut FluentBundleRc {
+    let mut langids = Vec::with_capacity(locale_count);
+    let locales = std::slice::from_raw_parts(locales, locale_count);
     for locale in locales {
-        let langid: Result<LanguageIdentifier, _> = locale.to_string().parse();
-        match langid {
-            Ok(langid) => langids.push(langid),
-            Err(_) => return std::ptr::null_mut(),
-        }
+        let id = match locale.as_str_unchecked().parse::<LanguageIdentifier>() {
+            Ok(id) => id,
+            Err(..) => return std::ptr::null_mut(),
+        };
+        langids.push(id);
     }
-    let mut bundle = FluentBundle::new(&langids);
+
+    Box::into_raw(fluent_bundle_new_internal(
+        &langids,
+        use_isolating,
+        pseudo_strategy,
+    ))
+}
+
+fn fluent_bundle_new_internal(
+    langids: &[LanguageIdentifier],
+    use_isolating: bool,
+    pseudo_strategy: &nsACString,
+) -> Box<FluentBundleRc> {
+    let mut bundle = FluentBundle::new(langids.iter());
     bundle.set_use_isolating(use_isolating);
 
     bundle.set_formatter(Some(format_numbers));
@@ -132,7 +166,7 @@ pub unsafe extern "C" fn fluent_bundle_new(
             _ => bundle.set_transform(None),
         }
     }
-    Box::into_raw(Box::new(bundle.into()))
+    Box::new(bundle.into())
 }
 
 #[no_mangle]