Clean up more minor differences between google3 and Chromium version of HTTP/2

R=bnc@chromium.org

Change-Id: I4645a737b8554c86ab4f21361e44cadae84f2d4b
Reviewed-on: https://chromium-review.googlesource.com/c/1347213Reviewed-by: Ryan Hamilton <rch@chromium.org>
Commit-Queue: Victor Vasiliev <vasilvv@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610997}

net/third_party/http2/decoder/decode_buffer_test.cc

@@ -44,9 +44,6 @@ struct TestStruct {
 };
 
 class DecodeBufferTest : public ::testing::Test {
- public:
-  DecodeBufferTest() = default;
-
  protected:
   Http2Random random_;
   uint32_t decode_offset_;

net/third_party/http2/decoder/decode_http2_structures_test.cc

@@ -44,9 +44,7 @@ class StructureDecoderTest : public ::testing::Test {
  protected:
   typedef S Structure;
 
-  StructureDecoderTest() : random_decode_count_(100) {
-    CHECK_LE(random_decode_count_, 1000u * 1000u) << "That should be plenty!";
-  }
+  StructureDecoderTest() : random_(), random_decode_count_(100) {}
 
   // Set the fields of |*p| to random values.
   void Randomize(S* p) { ::http2::test::Randomize(p, &random_); }
@@ -79,7 +77,6 @@ class StructureDecoderTest : public ::testing::Test {
 
   // Generate
   void TestDecodingRandomizedStructures(size_t count) {
-    EXPECT_LT(count, 1000u * 1000u) << "That should be plenty!";
     for (size_t i = 0; i < count && !HasFailure(); ++i) {
       Structure input;
       Randomize(&input);

net/third_party/http2/decoder/payload_decoders/payload_decoder_base_test_util.h

@@ -191,7 +191,7 @@ class AbstractPayloadDecoderTest : public PayloadDecoderBaseTest {
   AssertionResult DecodePayloadAndValidateSeveralWays(
       Http2StringPiece payload,
       const FrameParts& expected) {
-    NoArgValidator validator = [&expected, this]() -> AssertionResult {
+    auto validator = [&expected, this]() -> AssertionResult {
       VERIFY_FALSE(listener_.IsInProgress());
       VERIFY_EQ(1u, listener_.size());
       VERIFY_AND_RETURN_SUCCESS(expected.VerifyEquals(*listener_.frame(0)));
@@ -306,7 +306,7 @@ class AbstractPayloadDecoderTest : public PayloadDecoderBaseTest {
   ::testing::AssertionResult VerifyDetectsFrameSizeError(
       uint8_t required_flags,
       Http2StringPiece unpadded_payload,
-      ApproveSize approve_size) {
+      const ApproveSize& approve_size) {
     Http2FrameType frame_type = DecoderPeer::FrameType();
     uint8_t known_flags = KnownFlagsMaskForFrameType(frame_type);
     VERIFY_EQ(0, known_flags & Http2FrameFlag::PADDED);

net/third_party/http2/hpack/decoder/hpack_block_collector.h

@@ -30,15 +30,16 @@ namespace test {
 
 class HpackBlockCollector : public HpackEntryDecoderListener {
  public:
+  HpackBlockCollector();
+  HpackBlockCollector(const HpackBlockCollector& other);
+  ~HpackBlockCollector() override;
+
   // Implementations of HpackEntryDecoderListener, forwarding to pending_entry_,
   // an HpackEntryCollector for the "in-progress" HPACK entry. OnIndexedHeader
   // and OnDynamicTableSizeUpdate are pending only for that one call, while
   // OnStartLiteralHeader is followed by many calls, ending with OnValueEnd.
   // Once all the calls for one HPACK entry have been received, PushPendingEntry
   // is used to append the pending_entry_ entry to the collected entries_.
-  HpackBlockCollector();
-  HpackBlockCollector(const HpackBlockCollector& other);
-  ~HpackBlockCollector() override;
   void OnIndexedHeader(size_t index) override;
   void OnDynamicTableSizeUpdate(size_t size) override;
   void OnStartLiteralHeader(HpackEntryType header_type,

net/third_party/http2/hpack/decoder/hpack_decoder.h

@@ -20,7 +20,8 @@
 // spend time to do that?
 
 #include <stddef.h>
-#include <stdint.h>
+
+#include <cstdint>
 
 #include "net/third_party/http2/decoder/decode_buffer.h"
 #include "net/third_party/http2/hpack/decoder/hpack_block_decoder.h"

net/third_party/http2/hpack/decoder/hpack_decoder_state.h

@@ -13,7 +13,8 @@
 #define NET_THIRD_PARTY_HTTP2_HPACK_DECODER_HPACK_DECODER_STATE_H_
 
 #include <stddef.h>
-#include <stdint.h>
+
+#include <cstdint>
 
 #include "net/third_party/http2/hpack/decoder/hpack_decoder_listener.h"
 #include "net/third_party/http2/hpack/decoder/hpack_decoder_string_buffer.h"

net/third_party/http2/hpack/decoder/hpack_decoder_tables.h

@@ -23,7 +23,7 @@
 
 #include "base/containers/circular_deque.h"
 #include "net/third_party/http2/hpack/hpack_string.h"
-//#include "net/third_party/http2/http2_constants.h"
+#include "net/third_party/http2/http2_constants.h"
 #include "net/third_party/http2/platform/api/http2_export.h"
 
 namespace http2 {
@@ -134,7 +134,7 @@ class HTTP2_EXPORT_PRIVATE HpackDecoderDynamicTable {
 
   // The last received DynamicTableSizeUpdate value, initialized to
   // SETTINGS_HEADER_TABLE_SIZE.
-  size_t size_limit_ = 4096;  // Http2SettingsInfo::DefaultHeaderTableSize();
+  size_t size_limit_ = Http2SettingsInfo::DefaultHeaderTableSize();
 
   size_t current_size_ = 0;
 

net/third_party/http2/hpack/decoder/hpack_entry_collector.cc

@@ -26,13 +26,8 @@ HpackEntryCollector::HpackEntryCollector() {
   Clear();
 }
 
-HpackEntryCollector::HpackEntryCollector(const HpackEntryCollector& other)
-    : header_type_(other.header_type_),
-      index_(other.index_),
-      name_(other.name_),
-      value_(other.value_),
-      started_(other.started_),
-      ended_(other.ended_) {}
+HpackEntryCollector::HpackEntryCollector(const HpackEntryCollector& other) =
+    default;
 
 HpackEntryCollector::HpackEntryCollector(HpackEntryType type,
                                          size_t index_or_size)

net/third_party/http2/hpack/decoder/hpack_string_decoder_test.cc

@@ -51,7 +51,6 @@ class HpackStringDecoderTest : public RandomDecoderTest {
   // expected_str is a Http2String rather than a const Http2String& or
   // Http2StringPiece so that the lambda makes a copy of the string, and thus
   // the string to be passed to Collected outlives the call to MakeValidator.
-
   Validator MakeValidator(const Http2String& expected_str,
                           bool expected_huffman) {
     return

net/third_party/http2/hpack/decoder/hpack_whole_entry_buffer_test.cc

@@ -18,7 +18,6 @@ using ::testing::HasSubstr;
 using ::testing::InSequence;
 using ::testing::Property;
 using ::testing::StrictMock;
-using ::testing::_;
 
 namespace http2 {
 namespace test {

net/third_party/http2/hpack/hpack_string.cc

@@ -12,7 +12,7 @@
 namespace http2 {
 
 HpackString::HpackString(const char* data) : str_(data) {}
-HpackString::HpackString(Http2StringPiece str) : str_(str.as_string()) {}
+HpackString::HpackString(Http2StringPiece str) : str_(Http2String(str)) {}
 HpackString::HpackString(Http2String str) : str_(std::move(str)) {}
 HpackString::HpackString(const HpackString& other) = default;
 HpackString::~HpackString() = default;

net/third_party/http2/hpack/hpack_string.h

@@ -30,8 +30,6 @@ class HTTP2_EXPORT_PRIVATE HpackString {
   // Not sure yet whether this move ctor is required/sensible.
   HpackString(HpackString&& other) = default;
 
-  HpackString& operator=(const HpackString& other) = default;
-
   ~HpackString();
 
   size_t size() const { return str_.size(); }
@@ -65,8 +63,8 @@ struct HTTP2_EXPORT_PRIVATE HpackStringPair {
 
   Http2String DebugString() const;
 
-  HpackString name;
-  HpackString value;
+  const HpackString name;
+  const HpackString value;
 };
 
 HTTP2_EXPORT_PRIVATE std::ostream& operator<<(std::ostream& os,

net/third_party/http2/http2_constants.cc

@@ -4,9 +4,6 @@
 
 #include "net/third_party/http2/http2_constants.h"
 
-#include <ios>
-#include <sstream>
-
 #include "base/logging.h"
 #include "net/third_party/http2/platform/api/http2_string_piece.h"
 #include "net/third_party/http2/platform/api/http2_string_utils.h"

net/third_party/http2/http2_constants.h

@@ -7,8 +7,7 @@
 
 // Constants from the HTTP/2 spec, RFC 7540, and associated helper functions.
 
-#include <stdint.h>
-
+#include <cstdint>
 #include <iosfwd>
 #include <ostream>
 

net/third_party/http2/http2_constants_test.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "net/third_party/http2/http2_constants.h"
+
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace http2 {

net/third_party/http2/http2_constants_test_util.h

@@ -5,6 +5,7 @@
 #ifndef NET_THIRD_PARTY_HTTP2_HTTP2_CONSTANTS_TEST_UTIL_H_
 #define NET_THIRD_PARTY_HTTP2_HTTP2_CONSTANTS_TEST_UTIL_H_
 
+#include <cstdint>
 #include <vector>
 
 #include "net/third_party/http2/http2_constants.h"

net/third_party/http2/http2_structures.h

@@ -26,8 +26,8 @@
 // Http2PriorityFields.
 
 #include <stddef.h>
-#include <stdint.h>
 
+#include <cstdint>
 #include <ostream>
 
 #include "base/logging.h"

net/third_party/http2/http2_structures_test.cc

@@ -44,6 +44,36 @@ E IncrementEnum(E e) {
   return static_cast<E>(1 + static_cast<I>(e));
 }
 
+template <class T>
+AssertionResult VerifyRandomCalls() {
+  T t1;
+  Http2Random seq1;
+  Randomize(&t1, &seq1);
+
+  T t2;
+  Http2Random seq2(seq1.Key());
+  Randomize(&t2, &seq2);
+
+  // The two Randomize calls should have made the same number of calls into
+  // the Http2Random implementations.
+  VERIFY_EQ(seq1.Rand64(), seq2.Rand64());
+
+  // And because Http2Random implementation is returning the same sequence, and
+  // Randomize should have been consistent in applying those results, the two
+  // Ts should have the same value.
+  VERIFY_EQ(t1, t2);
+
+  Randomize(&t2, &seq2);
+  VERIFY_NE(t1, t2);
+
+  Randomize(&t1, &seq1);
+  VERIFY_EQ(t1, t2);
+
+  VERIFY_EQ(seq1.Rand64(), seq2.Rand64());
+
+  return AssertionSuccess();
+}
+
 #if GTEST_HAS_DEATH_TEST && !defined(NDEBUG)
 std::vector<Http2FrameType> ValidFrameTypes() {
   std::vector<Http2FrameType> valid_types{Http2FrameType::DATA};
@@ -111,6 +141,8 @@ TEST(Http2FrameHeaderTest, Eq) {
   EXPECT_TRUE(v == u);
   EXPECT_FALSE(u != v);
   EXPECT_FALSE(v != u);
+
+  EXPECT_TRUE(VerifyRandomCalls<Http2FrameHeader>());
 }
 
 #if GTEST_HAS_DEATH_TEST && !defined(NDEBUG)
@@ -345,6 +377,8 @@ TEST(Http2PriorityFieldsTest, Constructor) {
   EXPECT_DEBUG_DEATH(
       Http2PriorityFields(stream_dependency, weight + 256, is_exclusive),
       "too large");
+
+  EXPECT_TRUE(VerifyRandomCalls<Http2PriorityFields>());
 }
 #endif  // GTEST_HAS_DEATH_TEST && !defined(NDEBUG)
 
@@ -354,6 +388,8 @@ TEST(Http2RstStreamFieldsTest, IsSupported) {
 
   Http2RstStreamFields u{static_cast<Http2ErrorCode>(~0)};
   EXPECT_FALSE(u.IsSupportedErrorCode()) << v;
+
+  EXPECT_TRUE(VerifyRandomCalls<Http2RstStreamFields>());
 }
 
 TEST(Http2SettingFieldsTest, Misc) {
@@ -388,6 +424,8 @@ TEST(Http2SettingFieldsTest, Misc) {
   std::stringstream s;
   s << x;
   EXPECT_EQ("parameter=MAX_FRAME_SIZE, value=123", s.str());
+
+  EXPECT_TRUE(VerifyRandomCalls<Http2SettingFields>());
 }
 
 TEST(Http2PushPromiseTest, Misc) {
@@ -410,6 +448,8 @@ TEST(Http2PushPromiseTest, Misc) {
 
   v.promised_stream_id = promised_stream_id;
   EXPECT_EQ(v, w);
+
+  EXPECT_TRUE(VerifyRandomCalls<Http2PushPromiseFields>());
 }
 
 TEST(Http2PingFieldsTest, Misc) {
@@ -417,6 +457,8 @@ TEST(Http2PingFieldsTest, Misc) {
   std::stringstream s;
   s << v;
   EXPECT_EQ("opaque_bytes=0x3820627974657300", s.str());
+
+  EXPECT_TRUE(VerifyRandomCalls<Http2PingFields>());
 }
 
 TEST(Http2GoAwayFieldsTest, Misc) {
@@ -439,6 +481,8 @@ TEST(Http2GoAwayFieldsTest, Misc) {
   EXPECT_NE(v, u);
   EXPECT_NE(v.last_stream_id, u.last_stream_id);
   EXPECT_EQ(v.error_code, u.error_code);
+
+  EXPECT_TRUE(VerifyRandomCalls<Http2GoAwayFields>());
 }
 
 TEST(Http2WindowUpdateTest, Misc) {
@@ -462,6 +506,8 @@ TEST(Http2WindowUpdateTest, Misc) {
 
   v.window_size_increment = window_size_increment;
   EXPECT_EQ(v, w);
+
+  EXPECT_TRUE(VerifyRandomCalls<Http2WindowUpdateFields>());
 }
 
 TEST(Http2AltSvcTest, Misc) {
@@ -482,6 +528,8 @@ TEST(Http2AltSvcTest, Misc) {
 
   v.origin_length = w.origin_length;
   EXPECT_EQ(v, w);
+
+  EXPECT_TRUE(VerifyRandomCalls<Http2AltSvcFields>());
 }
 
 }  // namespace

net/third_party/http2/test_tools/frame_parts.cc

@@ -77,7 +77,7 @@ AssertionResult FrameParts::VerifyEquals(const FrameParts& that) const {
   VERIFY_EQ(padding_, that.padding_) << COMMON_MESSAGE;
   VERIFY_EQ(altsvc_origin_, that.altsvc_origin_) << COMMON_MESSAGE;
   VERIFY_EQ(altsvc_value_, that.altsvc_value_) << COMMON_MESSAGE;
-  VERIFY_EQ(settings_, that.settings_) << COMMON_MESSAGE;
+  VERIFY_THAT(settings_, ContainerEq(that.settings_)) << COMMON_MESSAGE;
 
 #define VERIFY_OPTIONAL_FIELD(field_name) \
   VERIFY_SUCCESS(VerifyOptionalEq(field_name, that.field_name))

net/third_party/http2/tools/http2_frame_builder.cc

@@ -65,7 +65,7 @@ void Http2FrameBuilder::AppendUInt31(uint32_t value) {
   // If you want to test the high-bit being set, call AppendUInt32 instead.
   uint32_t tmp = value & StreamIdMask();
   EXPECT_EQ(value, value & StreamIdMask())
-      << "High-bit of uint32 should be clear.";
+      << "High-bit of uint32_t should be clear.";
   value = htonl(tmp);
   AppendBytes(&value, 4);
 }

net/third_party/http2/tools/random_decoder_test.cc

@@ -10,7 +10,6 @@
 #include <memory>
 
 #include "base/logging.h"
-#include "base/strings/string_number_conversions.h"
 #include "net/third_party/http2/decoder/decode_buffer.h"
 #include "net/third_party/http2/decoder/decode_status.h"
 #include "net/third_party/http2/http2_constants.h"

net/third_party/http2/tools/random_decoder_test.h

@@ -209,7 +209,8 @@ class RandomDecoderTest : public ::testing::Test {
   // TODO(jamessynge): Replace this overload with the next, as using this method
   // usually means that the wrapped function doesn't need to be passed the
   // DecodeBuffer nor the DecodeStatus.
-  static Validator ValidateDoneAndOffset(uint32_t offset, Validator wrapped) {
+  static Validator ValidateDoneAndOffset(uint32_t offset,
+                                         const Validator& wrapped) {
     return [wrapped, offset](const DecodeBuffer& input,
                              DecodeStatus status) -> AssertionResult {
       VERIFY_EQ(status, DecodeStatus::kDecodeDone);