Add net::SdchSourceStream and net::SdchPolicyDelegate

net/filter/sdch_source_stream.h

+// Copyright 2016 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.
+
+#ifndef NET_FILTER_SDCH_SOURCE_STREAM_H_
+#define NET_FILTER_SDCH_SOURCE_STREAM_H_
+
+#include <memory>
+
+#include "base/macros.h"
+#include "net/base/net_export.h"
+#include "net/base/sdch_dictionary.h"
+#include "net/base/sdch_manager.h"
+#include "net/filter/filter_source_stream.h"
+
+namespace open_vcdiff {
+
+class VCDiffStreamingDecoder;
+
+}  // namespace open_vcdiff
+
+namespace net {
+
+class IOBuffer;
+
+// SdchSourceStream applies open_vcdiff content decoding to a datastream.
+// This decoding uses a pre-cached dictionary of text fragments to decode
+// (expand) the stream back to its original contents.
+//
+// This SdchSourceStream internally uses open_vcdiff/vcdec library to do
+// decoding.
+class NET_EXPORT_PRIVATE SdchSourceStream : public FilterSourceStream {
+ public:
+  // The Delegate interface is responsible for error recovery and stats
+  // gathering. See the methods below for descriptions of which errors the
+  // delegate is expected to handle and what it can do to repair them.
+  class NET_EXPORT_PRIVATE Delegate {
+   public:
+    virtual ~Delegate(){};
+
+    // Called by the SdchSourceStream if an error occurs while parsing the
+    // server-sent dictionary ID, or if the specified dictionary can't be loaded
+    // (i.e., GetDictionary returned false). This method is expected to correct
+    // the error condition by either:
+    //   a) returning true and calling ReplaceOutput or Passthrough, or

[Randy Smith (Not in Mondays), 2016/09/27 20:09:13]

nit: I think that RepalceOutput() or Passthrough() must be called *before* the
function returns true; reasonable for the documentation to have this ordering?

[xunjieli, 2016/09/30 15:32:18]

Done.

+    //   b) returning false, in which case the SourceStream will return an error
+    //      to its caller
+    // Conceptually, the return value is true if the error has been
+    // handled/recovered from and false if not.
+    // This method must not retain a reference to |source| after returning.
+    virtual bool OnDictionaryError(SdchSourceStream* source) = 0;

[Randy Smith (Not in Mondays), 2016/09/27 20:09:13]

Suggestion: So I flip-flopped a bit on this in thinking about it, but my
personal preference would be to have the StopDecoding/ReplaceOutput()
possibilities for handling the error be embedded in this functions signature
(e.g. to return an enum indicating the error state transition and to have a
writeable string output parameter for replacing the output if that's needed). 
It makes these signatures more complicated and messy, but it eliminates the
method signatures below and means that the state transition logic is inline in
the case statement in the .cc file, as opposed to being magically invoked by the
delegate from outside.  If you really prefer it this way, I'm ok with it, but if
I were doing it I'd probably just hack up these signatures and get rid of
StopDecoding/ReplaceOutput.

[xunjieli, 2016/09/30 15:32:18]

Done.

+
+    // Called by the SdchSourceStream if an error occurs while decoding the
+    // vcdiff-compressed data stream. Semantics are similar to
+    // OnDictionaryError above. This method must not retain a reference to
+    // |source| after returning.
+    virtual bool OnDecodingError(SdchSourceStream* source) = 0;
+
+    // Called by the SdchSourceStream to request the text of the specified
+    // dictionary. This method must either:
+    //   * Fill in |*text| and return true, or
+    //   * Leave |*text| untouched and return false.
+    // The return value is true if the named dictionary could be found and false
+    // otherwise.
+    //
+    // The |server_id| string is guaranteed to be a syntactically valid SDCH
+    // server-id.
+    virtual bool OnGetDictionary(const std::string& server_id,
+                                 const std::string** text) = 0;

[Randy Smith (Not in Mondays), 2016/09/27 20:09:13]

Would you do me a favor and put O(5m) of thought into what the code would have
to look like if this interface was async (you may totally have done this
already), make sure this design doesn't horribly get in the way of whatever that
is, and put in a todo sketching out how what changes would have to be made here
for this to be async?  I suspect you can figure out why I'm asking :-}.

[xunjieli, 2016/09/30 15:32:18]

Done.

+  };
+
+  SdchSourceStream(std::unique_ptr<SourceStream> previous, Delegate* delegate);
+  ~SdchSourceStream() override;
+
+  // For use by SdchSourceStreamDelegate. Switches this stream source into
+  // "passthrough" mode, where it will not decode any further output with
+  // vcdiff. This method can only be called during an invocation of the
+  // delegate's OnDictionaryError or OnDecodingError functions.
+  virtual void StopDecoding();
+
+  // For use by SdchSourceStreamDelegate. Replaces the entire output of this
+  // stream with the supplied |data|, which is of length |size|. Note that
+  // calling this method also stops vcdiff decoding as a side-effect - |data|
+  // should not point to vcdiff-encoded data. This method can only be called
+  // during an invocation of the delegate's OnDictionaryError or
+  // OnDecodingError functions.
+  virtual void ReplaceOutput(const char* data, size_t size);

[Randy Smith (Not in Mondays), 2016/09/27 20:09:13]

nit, suggestion: If the above two are only called by the
SdchSourceStreamDelegate, it should be possible to move them into the private:
section.  (This may require indirecting the calls through protected methods on
the delegate class, since it's that class that has the private: privilege on
SdchSourceStream.)  Feel free to decide the plumbing's not worth it for the
encapsulation gain.

[xunjieli, 2016/09/30 15:32:18]

Acknowledged. These two methods are removed.

+
+ private:
+  enum InputState {
+    STATE_LOAD_DICTIONARY,
+    STATE_FLUSH_INTERNAL_BUFFER,
+    STATE_PASS_THROUGH,
+    STATE_HANDLE_ERROR,
+    STATE_DECODE,
+  };
+
+  // SourceStream implementation:
+  std::string GetTypeAsString() const override;
+  int FilterData(IOBuffer* output_buffer,
+                 int output_buffer_size,
+                 IOBuffer* input_buffer,
+                 int input_buffer_size,
+                 int* consumed_bytes,
+                 bool upstream_end_reached) override;
+
+  bool LoadDictionary(DrainableIOBuffer* input_buffer);
+
+  // Returns whether |id| looks like a dictionary ID, meaning 8 characters of
+  // base64url followed by a null character.
+  bool CouldBeDictionaryId(const std::string& id) const;
+
+  bool AskDelegateToHandleDictionaryError();
+  bool AskDelegateToHandleDecodingError();
+
+  std::unique_ptr<open_vcdiff::VCDiffStreamingDecoder> decoder_;
+  Delegate* delegate_;
+
+  // After the encoded response SDCH header is read, this variable contains
+  // the server hash with trailing null byte.
+  std::string dictionary_id_;

[Randy Smith (Not in Mondays), 2016/09/27 20:09:13]

nit, suggestion: I've had enough trouble with server vs. client id in the SDCH
code that I'd suggest naming this dictionary_server_id_ to make clear which id
is being discussed in the name.

[xunjieli, 2016/09/30 15:32:18]

Done.

+
+  // Since vcdiff may generate quite a bit of output at once, SdchSourceStream
+  // has to buffer excess output (more than requested by the caller) here to
+  // return later. This could maybe become quite large.

[Randy Smith (Not in Mondays), 2016/09/27 20:09:13]

nit: "could maybe" seems redundant to me; eliminate "maybe"?

[Randy Smith (Not in Mondays), 2016/09/27 20:09:13]

I find myself wanting to file a bug on upstream asking for control over the
amount returned; given that it's a compressed format, there's no reason we
couldn't buffer more efficiently on the input side.  But my understanding is
that we don't have a maintainer for vcdiff, and thus no effective bug tracker
:-J.  Maybe worthwhile filing a crbug.com and noting that it really refers to a
dependent package?  (Feel free to ignore this suggestion.)

[xunjieli, 2016/09/30 15:32:18]

Done.

[xunjieli, 2016/09/30 15:32:18]

Done.

+  std::string buffered_output_;
+
+  bool output_replaced_;
+  bool passthrough_;
+
+  bool in_delegate_handler_;
+
+  InputState input_state_;
+
+  DISALLOW_COPY_AND_ASSIGN(SdchSourceStream);
+};
+
+}  // namespace net
+
+#endif  // NET_FILTER_SDCH_SOURCE_STREAM_H_