tracing v2: Introduce handles for safe usage of ProtoZeroMessage

components/tracing/core/proto_zero_message_unittest.cc

 // 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.
 
 #include "components/tracing/core/proto_zero_message.h"
 
 #include <limits>
 #include <memory>
 #include <vector>
 
 #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"
 
 namespace tracing {
 namespace v2 {
 
 const size_t kChunkSize = 16;
 const uint8_t kTestBytes[] = {0, 0, 0, 0, 0x42, 1, 0x42, 0xff, 0x42, 0};
 const char kStartWatermark[] = {'a', 'b', 'c', 'd', '1', '2', '3', '\0'};
 const char kEndWatermark[] = {'9', '8', '7', '6', 'z', 'w', 'y', '\0'};
@@ skipping 46 matching lines @@
     return buffer_->chunks().size() * kChunkSize -
            stream_writer_->bytes_available();
   }
 
   std::string GetNextSerializedBytes(size_t num_bytes) {
     size_t old_readback_pos = readback_pos_;
     readback_pos_ += num_bytes;
     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));
 
     if (depth < ProtoZeroMessage::kMaxNestingDepth) {
       auto* child_msg = msg->BeginNestedMessage<FakeMessage>(1 + depth * 10);
       BuildNestedMessages(depth + 1, child_msg);
     }
 
     for (uint32_t i = 129; i <= 256; ++i)
@@ skipping 74 matching lines @@
   ASSERT_EQ("8A0882808000", GetNextSerializedBytes(6));
   ASSERT_EQ("2002", GetNextSerializedBytes(2));
 
   ASSERT_EQ("2803", GetNextSerializedBytes(2));
 }
 
 // Checks that the size field of root and nested messages is properly written
 // 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);
 
   FakeMessage* nested_msg_1 = root_msg->BeginNestedMessage<FakeMessage>(2);
   nested_msg_1->AppendVarIntU32(3, 0x43);
 
   FakeMessage* nested_msg_2 = nested_msg_1->BeginNestedMessage<FakeMessage>(4);
   uint8_t buf200[200];
   memset(buf200, 0x42, sizeof(buf200));
@@ skipping 35 matching lines @@
 
   // The main point of this test is to stress the code paths and test for
   // unexpected crashes of the production code. The actual serialization is
   // already covered in the other text fixtures. Keeping just a final smoke test
   // here on the full buffer hash.
   std::string full_buf = GetNextSerializedBytes(GetNumSerializedBytes());
   uint32_t buf_hash = base::SuperFastHash(full_buf.data(), full_buf.size());
   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