Bug 1483036 - Report meaningful Promise values from FluentDOM C++ bits r=Gijs,smaug
authorZibi Braniecki <zbraniecki@mozilla.com>
Tue, 26 Mar 2019 19:34:27 +0000
changeset 466182 5100b0065b01ac6ac19ae9246483fa0003a8c6a3
parent 466181 0bd696ed34eace2844a540509a5022dc57b48508
child 466183 498699c9be80bd118c48335abc8fa689d66179f5
push id35762
push usercsabou@mozilla.com
push dateWed, 27 Mar 2019 04:44:00 +0000
treeherdermozilla-central@bc572aee49b6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs, smaug
bugs1483036
milestone68.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 1483036 - Report meaningful Promise values from FluentDOM C++ bits r=Gijs,smaug Differential Revision: https://phabricator.services.mozilla.com/D24113
dom/base/nsINode.cpp
intl/l10n/DocumentL10n.cpp
intl/l10n/Localization.jsm
--- a/dom/base/nsINode.cpp
+++ b/dom/base/nsINode.cpp
@@ -2811,34 +2811,23 @@ class LocalizationHandler : public Promi
 
         if (!JS_DefineElement(aCx, untranslatedElements, i, wrappedElem,
                               JSPROP_ENUMERATE)) {
           mReturnValuePromise->MaybeRejectWithUndefined();
           return;
         }
       }
     }
-
-    JS::RootedObject sourceScope(aCx, JS::CurrentGlobalOrNull(aCx));
-
-    AutoEntryScript aes(mReturnValuePromise->GetParentObject(),
-                        "Promise resolution");
-    JSContext* cx = aes.cx();
-    JS::Rooted<JS::Value> result(cx, JS::ObjectValue(*untranslatedElements));
-
-    xpc::StackScopedCloneOptions options;
-    options.wrapReflectors = true;
-    StackScopedClone(cx, options, sourceScope, &result);
-
-    mReturnValuePromise->MaybeResolve(result);
+    JS::Rooted<JS::Value> result(aCx, JS::ObjectValue(*untranslatedElements));
+    mReturnValuePromise->MaybeResolveWithClone(aCx, result);
   }
 
   virtual void RejectedCallback(JSContext* aCx,
                                 JS::Handle<JS::Value> aValue) override {
-    mReturnValuePromise->MaybeRejectWithUndefined();
+    mReturnValuePromise->MaybeRejectWithClone(aCx, aValue);
   }
 
  private:
   ~LocalizationHandler() = default;
 
   nsTArray<nsCOMPtr<Element>> mElements;
   RefPtr<Promise> mReturnValuePromise;
 };
--- a/intl/l10n/DocumentL10n.cpp
+++ b/intl/l10n/DocumentL10n.cpp
@@ -364,22 +364,22 @@ class L10nReadyHandler final : public Pr
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_CLASS(L10nReadyHandler)
 
   explicit L10nReadyHandler(Promise* aPromise, DocumentL10n* aDocumentL10n)
       : mPromise(aPromise), mDocumentL10n(aDocumentL10n) {}
 
   void ResolvedCallback(JSContext* aCx, JS::Handle<JS::Value> aValue) override {
     mDocumentL10n->InitialDocumentTranslationCompleted();
-    mPromise->MaybeResolveWithUndefined();
+    mPromise->MaybeResolveWithClone(aCx, aValue);
   }
 
   void RejectedCallback(JSContext* aCx, JS::Handle<JS::Value> aValue) override {
     mDocumentL10n->InitialDocumentTranslationCompleted();
-    mPromise->MaybeRejectWithUndefined();
+    mPromise->MaybeRejectWithClone(aCx, aValue);
   }
 
  private:
   ~L10nReadyHandler() = default;
 
   RefPtr<Promise> mPromise;
   RefPtr<DocumentL10n> mDocumentL10n;
 };
--- a/intl/l10n/Localization.jsm
+++ b/intl/l10n/Localization.jsm
@@ -186,16 +186,28 @@ function defaultGenerateBundles(resource
   return L10nRegistry.generateBundles(appLocales, resourceIds);
 }
 
 function defaultGenerateBundlesSync(resourceIds) {
   const appLocales = Services.locale.appLocalesAsBCP47;
   return L10nRegistry.generateBundlesSync(appLocales, resourceIds);
 }
 
+function maybeReportErrorToGecko(error) {
+  if (AppConstants.NIGHTLY_BUILD || Cu.isInAutomation) {
+    if (Cu.isInAutomation) {
+      // We throw a string, rather than Error
+      // to allow the C++ Promise handler
+      // to clone it
+      throw error;
+    }
+    console.warn(error);
+  }
+}
+
 /**
  * The `Localization` class is a central high-level API for vanilla
  * JavaScript use of Fluent.
  * It combines language negotiation, FluentBundle and I/O to
  * provide a scriptable API to format translations.
  */
 class Localization {
   /**
@@ -241,33 +253,34 @@ class Localization {
    * fallback chain.
    *
    * @param   {Array<Object>}         keys    - Translation keys to format.
    * @param   {Function}              method  - Formatting function.
    * @returns {Promise<Array<string|Object>>}
    * @private
    */
   async formatWithFallback(keys, method) {
-    const translations = [];
+    const translations = new Array(keys.length);
+    let hasAtLeastOneBundle = false;
 
     for await (const bundle of this.bundles) {
+      hasAtLeastOneBundle = true;
       const missingIds = keysFromBundle(method, bundle, keys, translations);
 
       if (missingIds.size === 0) {
         break;
       }
 
-      if (AppConstants.NIGHTLY_BUILD || Cu.isInAutomation) {
-        const locale = bundle.locales[0];
-        const ids = Array.from(missingIds).join(", ");
-        if (Cu.isInAutomation) {
-          throw new Error(`Missing translations in ${locale}: ${ids}`);
-        }
-        console.warn(`Missing translations in ${locale}: ${ids}`);
-      }
+      const locale = bundle.locales[0];
+      const ids = Array.from(missingIds).join(", ");
+      maybeReportErrorToGecko(`[fluent] Missing translations in ${locale}: ${ids}.`);
+    }
+
+    if (!hasAtLeastOneBundle) {
+      maybeReportErrorToGecko(`[fluent] Request for keys failed because no resource bundles got generated.\n keys: ${JSON.stringify(keys)}.\n resourceIds: ${JSON.stringify(this.resourceIds)}.`);
     }
 
     return translations;
   }
 
   /**
    * Format translations into {value, attributes} objects.
    *
@@ -411,33 +424,34 @@ class LocalizationSync extends Localizat
     super(resourceIds, generateBundles);
   }
 
   cached(iterable) {
     return CachedSyncIterable.from(iterable);
   }
 
   formatWithFallback(keys, method) {
-    const translations = [];
+    const translations = new Array(keys.length);
+    let hasAtLeastOneBundle = false;
 
     for (const bundle of this.bundles) {
+      hasAtLeastOneBundle = true;
       const missingIds = keysFromBundle(method, bundle, keys, translations);
 
       if (missingIds.size === 0) {
         break;
       }
 
-      if (AppConstants.NIGHTLY_BUILD || Cu.isInAutomation) {
-        const locale = bundle.locales[0];
-        const ids = Array.from(missingIds).join(", ");
-        if (Cu.isInAutomation) {
-          throw new Error(`Missing translations in ${locale}: ${ids}`);
-        }
-        console.warn(`Missing translations in ${locale}: ${ids}`);
-      }
+      const locale = bundle.locales[0];
+      const ids = Array.from(missingIds).join(", ");
+      maybeReportErrorToGecko(`[fluent] Missing translations in ${locale}: ${ids}.`);
+    }
+
+    if (!hasAtLeastOneBundle) {
+      maybeReportErrorToGecko(`[fluent] Request for keys failed because no resource bundles got generated.\n keys: ${JSON.stringify(keys)}.\n resourceIds: ${JSON.stringify(this.resourceIds)}.`);
     }
 
     return translations;
   }
 
   formatValue(id, args) {
     const [val] = this.formatValues([{id, args}]);
     return val;
@@ -549,20 +563,25 @@ function keysFromBundle(method, bundle, 
   keys.forEach(({id, args}, i) => {
     if (translations[i] !== undefined) {
       return;
     }
 
     if (bundle.hasMessage(id)) {
       messageErrors.length = 0;
       translations[i] = method(bundle, messageErrors, id, args);
-      // XXX: Report resolver errors
+      if (messageErrors.length > 0) {
+        const locale = bundle.locales[0];
+        const errors = messageErrors.join(", ");
+        maybeReportErrorToGecko(`[fluent][resolver] errors in ${locale}/${id}: ${errors}.`);
+      }
     } else {
       missingIds.add(id);
     }
   });
 
+
   return missingIds;
 }
 
 this.Localization = Localization;
 this.LocalizationSync = LocalizationSync;
 var EXPORTED_SYMBOLS = ["Localization", "LocalizationSync"];