[MD settings] stream i18n replacement without large accumulation buffer

content/browser/webui/i18n_source_stream.cc

Index: content/browser/webui/i18n_source_stream.cc
diff --git a/content/browser/webui/i18n_source_stream.cc b/content/browser/webui/i18n_source_stream.cc
index 1ff237e26b82946452dbea00cc74eaadafbd42ab..1b443ad86863abbe1723256aa3ad9c32abb0b38b 100644
--- a/content/browser/webui/i18n_source_stream.cc
+++ b/content/browser/webui/i18n_source_stream.cc
@@ -29,7 +29,6 @@ I18nSourceStream::I18nSourceStream(std::unique_ptr<SourceStream> upstream,
                                    SourceStream::SourceType type,
                                    const ui::TemplateReplacements* replacements)
     : FilterSourceStream(type, std::move(upstream)),
-      drain_offset_(0),
       replacements_(replacements) {}
 
 std::string I18nSourceStream::GetTypeAsString() const {
@@ -42,24 +41,28 @@ int I18nSourceStream::FilterData(net::IOBuffer* output_buffer,
                                  int input_buffer_size,
                                  int* consumed_bytes,
                                  bool upstream_end_reached) {
-  DCHECK(output_.empty() || (upstream_end_reached && input_buffer_size == 0));
-  *consumed_bytes = input_buffer_size;
-  // TODO(dschuyler): Perform replacements without accumulation.
-  // Accumulate.
+  // |input_| is often empty (or it may have something from the prior call).
   input_.append(input_buffer->data(), input_buffer_size);
-  if (upstream_end_reached && !drain_offset_ && output_.empty()) {
-    // Process.
-    output_ = ui::ReplaceTemplateExpressions(input_, *replacements_);
-  }
+  *consumed_bytes = input_buffer_size;
 
-  if (drain_offset_ == output_.size())
-    return 0;
+  size_t pos = input_.find_last_of("$}");

[mmenke, 2017/01/03 16:47:16]

optional:  Maybe toss in "\r\n"?  Not sure how common $'s are that aren't part
of $i18n's are, but may make sense to handle them a bit better. 
I18nSourceStream::FilterData doesn't explicitly disallow \r or \n in names, but
it assumes keys are shorter than a line, which seems to imply they don't include
CRs/LFs.

[dschuyler, 2017/01/03 20:37:34]

Done.

+  std::string to_process;
+  if (!upstream_end_reached && pos != std::string::npos && input_[pos] == '$') {

[mmenke, 2017/01/03 16:47:16]

A string ending in "$" now has special case code.  Should have a test for it.

[dschuyler, 2017/01/03 20:37:34]

Done.

+    // If there is a trailing '$' then split the |input_| at that point. Process
+    // the first part; save the second part for the next call to FilterData().
+    to_process.assign(input_, 0, pos);
+    input_.assign(input_, pos, std::string::npos);

[mmenke, 2017/01/03 16:47:16]

input_.erase(0, pos);?  Should do the same thing, but may open up more space for
compiler optimizations, and while std::string self-assignment may work this way,
it seems a bit ugly.

[dschuyler, 2017/01/03 20:37:34]

Done.

+  } else {
+    // There is no risk of a split key, process the whole input.
+    to_process = input_;
+    input_.clear();

[mmenke, 2017/01/03 16:47:16]

to_process.swap(input_);  accomplishes what both these lines do without the
additional copy.  If also has the advantage of actually "freeing" the memory
owned by input_, since clear() generally doesn't free memory.

Alternatively, should make to_process a StringPiece, and have "input_.erase(0,
to_process.size());" after the output_.append call (i.e., just have a shared
method to truncate append on both paths).

[dschuyler, 2017/01/03 20:37:34]

Done.

+  }
 
-  // Drain.
-  int bytes_out = std::min(output_.size() - drain_offset_,
+  output_.append(ui::ReplaceTemplateExpressions(to_process, *replacements_));
+  int bytes_out = std::min(output_.size(),
                            static_cast<size_t>(output_buffer_size));
-  output_.copy(output_buffer->data(), bytes_out, drain_offset_);
-  drain_offset_ += bytes_out;
+  output_.copy(output_buffer->data(), bytes_out);
+  output_.assign(output_, bytes_out, std::string::npos);

[mmenke, 2017/01/03 16:47:16]

ouput_.erase(0, bytes_out);

[dschuyler, 2017/01/03 20:37:34]

Done.

   return bytes_out;
 }