Improve documentation on Filter and fix pipeline overwrite.

net/filter/filter.cc

Index: net/filter/filter.cc
diff --git a/net/filter/filter.cc b/net/filter/filter.cc
index 7d3af0aebfb5a89b7c7063b02a258d0b45762beb..8626c5d5fcfced12f0b5cf36d4f23f42f8e01a96 100644
--- a/net/filter/filter.cc
+++ b/net/filter/filter.cc
@@ -2,6 +2,24 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// The basic usage of the Filter interface is described in the comment at
+// the beginning of filter.h.  Filters are designed so that they may be
+// connected together into an arbitrary chain; in such a case the first filter
+// in the chain proxies calls to ReadData() so that the input and output signals
+// apply to the entire chain.

[Bence, 2014/10/29 20:00:24]

Should the part of this paragraph about how to use Filters in a chain not be
added to the header file instead, leaving only implementation details here?

[Randy Smith (Not in Mondays), 2014/10/30 18:35:42]

I'm of two minds on this.  The issue is whether the filter chain functionality
is something that's exposed in the interface.  You can make an argument that
it's not; filter chains are setup by Filter::Factory and PrependNewFilter,
completely within this file.  On the other hand, we do pass the vector of
FilterTypes to Filter::Factory, and it's fairly clear that what it's doing is
creating a filter chain.  I still tend to think that the fact that it supports
chaining is more of an implementation detail than an interface detail--it's not
something callers worry about except for passing that vector of FilterTypes in. 
I've added some commentary to make that clear, but let me know if you disagree
with the basic judgement; as I say, I'm of two minds.

+//
+// In a filter chain, the data flows from first filter (held by the
+// caller) down the chain.  When ReadData() is called on any filter
+// except for the last filter, it proxies the call down the chain,
+// filling in the input buffers of subsequent filters if needed (==
+// that filter's last_status() value is FILTER_NEED_MORE_DATA) and
+// available (== the current filter has data it can output).  The last
+// Filter will then output data if possible, and return
+// FILTER_NEED_MORE_DATA if not.  Because the indirection pushes
+// data along the filter chain at each level if it's available and the
+// next filter needs it, a return value of FILTER_NEED_MORE_DATA from the
+// final filter will apply to the entire chain.
+
 #include "net/filter/filter.h"
 
 #include "base/files/file_path.h"
@@ -87,6 +105,9 @@ Filter::FilterStatus Filter::ReadData(char* dest_buffer, int* dest_len) {
     return last_status_;
   if (!next_filter_.get())
     return last_status_ = ReadFilteredData(dest_buffer, dest_len);
+
+  // This filter needs more data, but it's not clear that the rest of
+  // the chain does; delegate the actual status return to the next filter.
   if (last_status_ == FILTER_NEED_MORE_DATA && !stream_data_len())
     return next_filter_->ReadData(dest_buffer, dest_len);