Detect unreplaced placeholders in WebUI template replacements

Adds a DCHECK for "$1" style placeholders in WebUI template
replacements.

When WebUI is replacing $i18n{foo} with a string, the string should
already have its own "$1" style placeholders replaced at that point.
Otherwise, the resulting HTML will include those placeholders.

$i18nPolymer is treated as an exception, since those strings might be
used as inputs to computed bindings that will use JS substitutions.

Bug: 987877
Change-Id: Iae02b84775b8a767e905c7528dfc85da6cac20e0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1721237Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Auto-Submit: Michael Giuffrida <michaelpg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707515}

chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.html

@@ -241,7 +241,7 @@
       <div slot="dialog-title">[[dialogTitle]]</div>
       <div slot="dialog-body">
         <paper-spinner-lite active></paper-spinner-lite>
-        <div id="configuringMessage">$i18n{printerConfiguringMessage}</div>
+        <div id="configuringMessage"></div>
       </div>
       <div slot="dialog-buttons">
         <cr-button class="cancel-button" on-click="onCloseConfiguringTap_">

ui/base/BUILD.gn

@@ -4,6 +4,7 @@
 
 import("//build/buildflag_header.gni")
 import("//build/config/compiler/compiler.gni")
+import("//build/config/dcheck_always_on.gni")
 import("//build/config/jumbo.gni")
 import("//build/config/linux/pangocairo/pangocairo.gni")
 import("//build/config/sanitizers/sanitizers.gni")
@@ -424,6 +425,10 @@ jumbo_component("base") {
     "//url",
   ]
 
+  if (is_debug || dcheck_always_on) {
+    deps += [ "//third_party/re2" ]
+  }
+
   if (!is_ios) {
     # iOS does not use Chromium-specific code for event handling.
     public_deps += [

ui/base/DEPS

@@ -2,6 +2,7 @@ include_rules = [
   "+net",
   "+skia/ext",
   "+third_party/brotli",
+  "+third_party/re2",
   "+third_party/skia",
   "+third_party/zlib",
   "+ui/base/ui_base_jni_headers",

ui/base/template_expressions.cc

@@ -12,6 +12,10 @@
 #include "base/values.h"
 #include "net/base/escape.h"
 
+#if DCHECK_IS_ON()
+#include "third_party/re2/src/re2/re2.h"  // nogncheck
+#endif
+
 namespace {
 const char kLeader[] = "$i18n";
 const size_t kLeaderSize = base::size(kLeader) - 1;
@@ -141,6 +145,19 @@ bool EscapeForJS(const std::string& in_string,
   return true;
 }
 
+#if DCHECK_IS_ON()
+// Checks whether the replacement has an unsubstituted placeholder, e.g. "$1".
+bool HasUnexpectedPlaceholder(const std::string& key,
+                              const std::string& replacement) {
+  // TODO(crbug.com/988031): Fix display aria labels.
+#if defined(OS_CHROMEOS)
+  if (key == "displayResolutionText")
+    return false;
+#endif
+  return re2::RE2::PartialMatch(replacement, re2::RE2(R"(\$\d)"));
+}
+#endif  // DCHECK_IS_ON()
+
 bool ReplaceTemplateExpressionsInternal(
     base::StringPiece source,
     const ui::TemplateReplacements& replacements,
@@ -205,6 +222,15 @@ bool ReplaceTemplateExpressionsInternal(
       CHECK(false) << "Unknown context " << context;
     }
 
+#if DCHECK_IS_ON()
+    // Replacements in Polymer WebUI may invoke JavaScript to replace string
+    // placeholders. In other contexts, placeholders should already be replaced.
+    if (context != "Polymer") {
+      DCHECK(!HasUnexpectedPlaceholder(key, replacement))
+          << "Dangling placeholder found in " << key;
+    }
+#endif
+
     formatted->append(replacement);
 
     current_pos = key_end + sizeof(kKeyClose);

ui/login/account_picker/chromeos_user_pod_template.html

@@ -153,9 +153,7 @@
             <div class="action-box-remove-legacy-supervised-user-warning-text">
               $i18n{removeLegacySupervisedUserWarningText}
             </div>
-            <div class="action-box-remove-non-owner-user-warning-text">
-              $i18n{removeNonOwnerUserWarningText}
-            </div>
+            <div class="action-box-remove-non-owner-user-warning-text"></div>
             <!-- cr-button is imported inside user_manager.html -->
             <cr-button class="remove-warning-button action-button" noink>
               $i18n{removeUserWarningButtonTitle}

ui/login/account_picker/user_pod_template.html

@@ -143,9 +143,7 @@
       <div class="action-box-remove-legacy-supervised-user-warning-text">
         $i18n{removeLegacySupervisedUserWarningText}
       </div>
-      <div class="action-box-remove-non-owner-user-warning-text">
-        $i18n{removeNonOwnerUserWarningText}
-      </div>
+      <div class="action-box-remove-non-owner-user-warning-text"></div>
       <!-- cr-button is imported inside user_manager.html -->
       <cr-button class="remove-warning-button action-button">
         $i18n{removeUserWarningButtonTitle}