Change CryptoFramer to return a QuicData* instead of returning a bool and setting an out-param to the new QuicData*. This simplifies the API for callers.

Merge internal change 35500299


Review URL: https://chromiumcodereview.appspot.com/11184057

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@163203 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 23 insertions, 34 deletions

diff --git a/net/quic/crypto/crypto_framer.cc b/net/quic/crypto/crypto_framer.cc
index 9a47951..31c26cf 100644
--- a/net/quic/crypto/crypto_framer.cc
+++ b/net/quic/crypto/crypto_framer.cc
@@ -108,11 +108,10 @@ bool CryptoFramer::ProcessInput(StringPiece input) {
   return true;
 }
 
-bool CryptoFramer::ConstructHandshakeMessage(
-    const CryptoHandshakeMessage& message,
-    QuicData** packet) {
+QuicData* CryptoFramer::ConstructHandshakeMessage(
+    const CryptoHandshakeMessage& message) {
   if (message.tag_value_map.size() > kMaxEntries) {
-    return false;
+    return NULL;
   }
   size_t len = sizeof(uint32);  // message tag
   len += sizeof(uint16);  // number of map entries
@@ -122,7 +121,7 @@ bool CryptoFramer::ConstructHandshakeMessage(
     len += sizeof(uint16);  // value len
     len += it->second.length(); // value
     if (it->second.length() == 0) {
-      return false;
+      return NULL;
     }
     ++it;
   }
@@ -133,18 +132,18 @@ bool CryptoFramer::ConstructHandshakeMessage(
   QuicDataWriter writer(len);
   if (!writer.WriteUInt32(message.tag)) {
     DCHECK(false) << "Failed to write message tag.";
-    return false;
+    return NULL;
   }
   if (!writer.WriteUInt16(message.tag_value_map.size())) {
     DCHECK(false) << "Failed to write size.";
-    return false;
+    return NULL;
   }
   // Tags
   for (it = message.tag_value_map.begin();
        it != message.tag_value_map.end(); ++it) {
     if (!writer.WriteUInt32(it->first)) {
       DCHECK(false) << "Failed to write tag.";
-      return false;
+      return NULL;
     }
   }
   // Lengths
@@ -152,14 +151,14 @@ bool CryptoFramer::ConstructHandshakeMessage(
        it != message.tag_value_map.end(); ++it) {
     if (!writer.WriteUInt16(it->second.length())) {
       DCHECK(false) << "Failed to write length.";
-      return false;
+      return NULL;
     }
   }
   // Possible padding
   if (message.tag_value_map.size() % 2 == 1) {
     if (!writer.WriteUInt16(0xABAB)) {
       DCHECK(false) << "Failed to write padding.";
-      return false;
+      return NULL;
     }
   }
   // Values
@@ -167,11 +166,10 @@ bool CryptoFramer::ConstructHandshakeMessage(
        it != message.tag_value_map.end(); ++it) {
     if (!writer.WriteBytes(it->second.data(), it->second.length())) {
       DCHECK(false) << "Failed to write value.";
-      return false;
+      return NULL;
     }
   }
-  *packet = new QuicData(writer.take(), len, true);
-  return true;
+  return new QuicData(writer.take(), len, true);
 }
 
 void CryptoFramer::Clear() {
diff --git a/net/quic/crypto/crypto_framer.h b/net/quic/crypto/crypto_framer.h
index f1e6885..cf61a9e 100644
--- a/net/quic/crypto/crypto_framer.h
+++ b/net/quic/crypto/crypto_framer.h
@@ -57,10 +57,9 @@ class NET_EXPORT_PRIVATE CryptoFramer {
   // false if there was an error, and true otherwise.
   bool ProcessInput(base::StringPiece input);
 
-  // Serializes |message| into |packet|.  Returns false if there was an
-  // error, and true otherwise.
-  bool ConstructHandshakeMessage(const CryptoHandshakeMessage& message,
-                                 QuicData** packet);
+  // Returns a new QuicData owned by the caller that contains a serialized
+  // |message|, or NULL if there was an error.
+  QuicData* ConstructHandshakeMessage(const CryptoHandshakeMessage& message);
 
  private:
   // Clears per-message state.  Does not clear the visitor.
diff --git a/net/quic/crypto/crypto_framer_test.cc b/net/quic/crypto/crypto_framer_test.cc
index 5d79eed..eed175f 100644
--- a/net/quic/crypto/crypto_framer_test.cc
+++ b/net/quic/crypto/crypto_framer_test.cc
@@ -100,10 +100,8 @@ TEST(CryptoFramerTest, ConstructHandshakeMessage) {
   };
 
   CryptoFramer framer;
-  QuicData* data;
-  EXPECT_TRUE(framer.ConstructHandshakeMessage(message, &data));
-  scoped_ptr<QuicData> scoped_data(data);
-
+  scoped_ptr<QuicData> data(framer.ConstructHandshakeMessage(message));
+  ASSERT_TRUE(data.get() != NULL);
   test::CompareCharArraysWithHexError("constructed packet",
                                       data->data(), data->length(),
                                       AsChars(packet), arraysize(packet));
@@ -137,9 +135,8 @@ TEST(CryptoFramerTest, ConstructHandshakeMessageWithTwoKeys) {
   };
 
   CryptoFramer framer;
-  QuicData* data;
-  EXPECT_TRUE(framer.ConstructHandshakeMessage(message, &data));
-  scoped_ptr<QuicData> scoped_data(data);
+  scoped_ptr<QuicData> data(framer.ConstructHandshakeMessage(message));
+  ASSERT_TRUE(data.get() != NULL);
 
   test::CompareCharArraysWithHexError("constructed packet",
                                       data->data(), data->length(),
@@ -154,10 +151,8 @@ TEST(CryptoFramerTest, ConstructHandshakeMessageTooManyEntries) {
   }
 
   CryptoFramer framer;
-
-  QuicData* dummy = NULL;
-  EXPECT_FALSE(framer.ConstructHandshakeMessage(message, &dummy));
-  scoped_ptr<QuicData> scoped_data(dummy);
+  scoped_ptr<QuicData> data(framer.ConstructHandshakeMessage(message));
+  EXPECT_TRUE(data.get() == NULL);
 }
 
 
@@ -167,9 +162,8 @@ TEST(CryptoFramerTest, ConstructHandshakeMessageInvalidLength) {
   message.tag_value_map[0x12345678] = "";
 
   CryptoFramer framer;
-  QuicData* dummy = NULL;
-  EXPECT_FALSE(framer.ConstructHandshakeMessage(message, &dummy));
-  scoped_ptr<QuicData> scoped_data(dummy);
+  scoped_ptr<QuicData> data(framer.ConstructHandshakeMessage(message));
+  EXPECT_TRUE(data.get() == NULL);
 }
 
 TEST(CryptoFramerTest, ProcessInput) {
diff --git a/net/quic/test_tools/quic_test_utils.cc b/net/quic/test_tools/quic_test_utils.cc
index a4c71bd..5f93597 100644
--- a/net/quic/test_tools/quic_test_utils.cc
+++ b/net/quic/test_tools/quic_test_utils.cc
@@ -110,8 +110,7 @@ QuicPacket* ConstructHandshakePacket(QuicGuid guid, CryptoTag tag) {
   CryptoHandshakeMessage message;
   message.tag = tag;
   CryptoFramer crypto_framer;
-  QuicData* data;
-  crypto_framer.ConstructHandshakeMessage(message, &data);
+  scoped_ptr<QuicData> data(crypto_framer.ConstructHandshakeMessage(message));
   QuicFramer quic_framer(QuicDecrypter::Create(kNULL),
                          QuicEncrypter::Create(kNULL));
 
@@ -131,7 +130,6 @@ QuicPacket* ConstructHandshakePacket(QuicGuid guid, CryptoTag tag) {
   fragments.push_back(fragment);
   QuicPacket* packet;
   quic_framer.ConstructFragementDataPacket(header, fragments, &packet);
-  delete data;
   return packet;
 }