tracing v2: Introduce handles for safe usage of ProtoZeroMessage

components/tracing/core/proto_zero_message_unittest.cc

Index: components/tracing/core/proto_zero_message_unittest.cc
diff --git a/components/tracing/core/proto_zero_message_unittest.cc b/components/tracing/core/proto_zero_message_unittest.cc
index e95db77319254ab9537b93b011557f0ea212ac72..898d1f2bc7bcb6d7a05f5a682d71cd2d577d7bd8 100644
--- a/components/tracing/core/proto_zero_message_unittest.cc
+++ b/components/tracing/core/proto_zero_message_unittest.cc
@@ -10,6 +10,7 @@
 
 #include "base/hash.h"
 #include "components/tracing/core/proto_utils.h"
+#include "components/tracing/core/proto_zero_message_handle.h"
 #include "components/tracing/test/fake_scattered_buffer.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -76,6 +77,10 @@ class ProtoZeroMessageTest : public ::testing::Test {
     return buffer_->GetBytesAsString(old_readback_pos, num_bytes);
   }
 
+  std::string GetSerializedBytes(size_t start, size_t num_bytes) {
+    return buffer_->GetBytesAsString(start, num_bytes);
+  }
+
   static void BuildNestedMessages(uint32_t depth, ProtoZeroMessage* msg) {
     for (uint32_t i = 1; i <= 128; ++i)
       msg->AppendBytes(i, kTestBytes, sizeof(kTestBytes));
@@ -170,7 +175,7 @@ TEST_F(ProtoZeroMessageTest, NestedMessagesSimple) {
 // on finalization.
 TEST_F(ProtoZeroMessageTest, BackfillSizeOnFinalization) {
   ProtoZeroMessage* root_msg = NewMessage();
-  uint8_t root_msg_size[proto::kMessageLengthFieldSize];
+  uint8_t root_msg_size[proto::kMessageLengthFieldSize] = {};
   root_msg->set_size_field(
       {&root_msg_size[0], &root_msg_size[proto::kMessageLengthFieldSize]});
   root_msg->AppendVarIntU32(1, 0x42);
@@ -226,5 +231,96 @@ TEST_F(ProtoZeroMessageTest, StressTest) {
   EXPECT_EQ(0x14BC1BA3u, buf_hash);
 }
 
+TEST_F(ProtoZeroMessageTest, MessageHandle) {
+  ProtoZeroMessage* msg1 = NewMessage();
+  ProtoZeroMessage* msg2 = NewMessage();
+  ProtoZeroMessage* msg3 = NewMessage();
+  ProtoZeroMessage* ignored_msg = NewMessage();
+  uint8_t msg1_size[proto::kMessageLengthFieldSize] = {};
+  uint8_t msg2_size[proto::kMessageLengthFieldSize] = {};
+  uint8_t msg3_size[proto::kMessageLengthFieldSize] = {};
+  msg1->set_size_field(
+      {&msg1_size[0], &msg1_size[proto::kMessageLengthFieldSize]});
+  msg2->set_size_field(
+      {&msg2_size[0], &msg2_size[proto::kMessageLengthFieldSize]});
+  msg3->set_size_field(
+      {&msg3_size[0], &msg3_size[proto::kMessageLengthFieldSize]});
+
+  // Test that the handle going out of scope causes the finalization of the
+  // target message.
+  {
+    ProtoZeroMessageHandleBase handle1(msg1);
+    handle1->AppendBytes(1 /* field_id */, kTestBytes, 1 /* size */);
+    ASSERT_EQ(0u, msg1_size[0]);
+  }
+  ASSERT_EQ(0x83u, msg1_size[0]);
+
+  // Test that the handle can be late initialized.
+  ProtoZeroMessageHandleBase handle2(ignored_msg);
+  handle2 = ProtoZeroMessageHandleBase(msg2);
+  handle2->AppendBytes(1 /* field_id */, kTestBytes, 2 /* size */);
+  ASSERT_EQ(0u, msg2_size[0]);  // |msg2| should not be finalized yet.
+
+  // Test that std::move works and does NOT cause finalization of the moved
+  // message.
+  ProtoZeroMessageHandleBase handle_swp(ignored_msg);
+  handle_swp = std::move(handle2);
+  ASSERT_EQ(0u, msg2_size[0]);  // msg2 should be NOT finalized yet.
+  handle_swp->AppendBytes(2 /* field_id */, kTestBytes, 3 /* size */);
+
+  ProtoZeroMessageHandleBase handle3(msg3);
+  handle3->AppendBytes(1 /* field_id */, kTestBytes, 4 /* size */);
+  ASSERT_EQ(0u, msg3_size[0]);  // msg2 should be NOT finalized yet.
+
+  // Both |handle3| and |handle_swp| point to a valid message (respectively,
+  // |msg3| and |msg2|). Now move |handle3| into |handle_swp|.
+  handle_swp = std::move(handle3);
+  ASSERT_EQ(0x89u, msg2_size[0]);  // |msg2| should be finalized at this point.
+
+  // At this point writing into handle_swp should actually write into |msg3|.
+  ASSERT_EQ(msg3, &*handle_swp);
+  handle_swp->AppendBytes(2 /* field_id */, kTestBytes, 8 /* size */);
+  ProtoZeroMessageHandleBase another_handle(ignored_msg);
+  handle_swp = std::move(another_handle);
+  ASSERT_EQ(0x90u, msg3_size[0]);  // |msg3| should be finalized at this point.
+
+#if DCHECK_IS_ON()
+  // In developer builds w/ DCHECK on a finalized message should invalidate the
+  // handle, in order to early catch bugs in the client code.
+  ProtoZeroMessage* msg4 = NewMessage();
+  ProtoZeroMessageHandleBase handle4(msg4);
+  ASSERT_EQ(msg4, &*handle4);
+  msg4->Finalize();
+  ASSERT_EQ(nullptr, &*handle4);
+#endif
+
+  // Test also the behavior of handle with non-root (nested) messages.
+
+  ContiguousMemoryRange size_msg_2;
+  {
+    auto* child_msg_1 = NewMessage()->BeginNestedMessage<FakeMessage>(3);
+    ProtoZeroMessageHandle<FakeMessage> child_handle_1(child_msg_1);
+    ContiguousMemoryRange size_msg_1 = child_msg_1->size_field();
+    memset(size_msg_1.begin, 0, size_msg_1.size());
+    child_handle_1->AppendVarIntU32(1, 0x11);
+
+    auto* child_msg_2 = NewMessage()->BeginNestedMessage<FakeMessage>(2);
+    size_msg_2 = child_msg_2->size_field();
+    memset(size_msg_2.begin, 0, size_msg_2.size());
+    ProtoZeroMessageHandle<FakeMessage> child_handle_2(child_msg_2);
+    child_handle_2->AppendVarIntU32(2, 0xFF);
+
+    // |child_msg_1| should not be finalized yet.
+    ASSERT_EQ(0u, size_msg_1.begin[0]);
+
+    // should stay NOT finalized, until end of the current scope.
+    child_handle_1 = std::move(child_handle_2);
+    // This move should cause |child_msg_1| to be finalized, while |child_msg_2|

[alph, 2016/07/20 18:35:00]

s/while/but not/

[Primiano Tucci (use gerrit), 2016/07/21 10:50:03]

Oh I must have accidentally swapped the lines. This comment was meant to be
shifted up two lines :)
Reput in place and reworded.

+    ASSERT_EQ(0x82u, size_msg_1.begin[0]);
+    ASSERT_EQ(0u, size_msg_2.begin[0]);
+  }
+  ASSERT_EQ(0x83u, size_msg_2.begin[0]);
+}
+
 }  // namespace v2
 }  // namespace tracing