[Template Expressions] changing ${ leader to $i18n{

ui/base/template_expressions.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/base/template_expressions.h"
 
 #include <stddef.h>
 
 #include "base/logging.h"
 
 namespace ui {
 
 std::string ReplaceTemplateExpressions(
     base::StringPiece format_string,
-    const std::map<base::StringPiece, std::string>& substitutions) {
+    const TemplateReplacements& substitutions) {
+  const char kLeader[] = "$i18n{";
+  const size_t kLeaderSize = arraysize(kLeader) - 1;

[Dan Beam, 2016/01/22 00:51:44]

wait, why "- 1"?  should we just go back to strlen() instead if that's the right
number without the "- 1"?

[Dan Beam, 2016/01/22 00:51:44]

if these are invariant, should they be static or in an anonymous namespace (or
both)?

[dschuyler, 2016/01/22 01:58:58]

arraysize allows for initializing the value statically, which is nice.  The -1
is for the null terminator in the character array (double quotes always add a
null terminator in the array).

[dschuyler, 2016/01/22 01:58:58]

Done.

   std::string formatted;
   const size_t kValueLengthGuess = 16;
   formatted.reserve(format_string.length() +
                     substitutions.size() * kValueLengthGuess);
   base::StringPiece::const_iterator i = format_string.begin();
   while (i < format_string.end()) {
-    if (*i == '$' && i + 2 < format_string.end() && i[1] == '{' &&
-        i[2] != '}') {
-      size_t key_start = i + strlen("${") - format_string.begin();
+    if (base::StringPiece(i).starts_with(kLeader)) {
+      size_t key_start = i + kLeaderSize - format_string.begin();
       size_t key_length = format_string.find('}', key_start);
       if (key_length == base::StringPiece::npos)
         NOTREACHED() << "TemplateExpression missing ending brace '}'";
       key_length -= key_start;
-      base::StringPiece key(format_string.begin() + key_start, key_length);
+      std::string key(format_string.begin() + key_start, key_length);

[Dan Beam, 2016/01/22 00:51:44]

why are you changing the type of |key|?

[Dan Beam, 2016/01/22 00:51:44]

DCHECK(!key.empty());

[dschuyler, 2016/01/22 01:58:58]

In a near-future CL this code will be run in a background thread where keys are
not made from StringPiece.  StringPiece holds pointers into memory owned by
something else.  That's generally safe within a single thread and less so across
threads.  This is prep for multi-threaded use.  

[dschuyler, 2016/01/22 01:58:58]

Done.

       const auto& replacement = substitutions.find(key);
       if (replacement != substitutions.end()) {
         formatted.append(replacement->second);
-        i += strlen("${") + key_length + strlen("}");
+        i += kLeaderSize + key_length + strlen("}");
         continue;
       } else {
         NOTREACHED() << "TemplateExpression key not found: " << key;
       }
     }
     formatted.push_back(*i);
     ++i;
   }
   return formatted;
 }
 
 }  // namespace ui