Make ProfileBuilder responsible for module deduplication

This CL introduces new structs InternalFrame and InternalModule for sampling
data transfer from NativeStackSampler to ProfileBuilder. After the transfer,
ProfileBuilder converts InternalFrame/InternalModule to Frame/Module, and
deduplicates modules.

Bug: 851163
Change-Id: I550a80d2528688ebef95066d9781104976063950
Reviewed-on: https://chromium-review.googlesource.com/1108957
Commit-Queue: Xi Cheng <chengx@chromium.org>
Reviewed-by: Mike Wittman <wittman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569827}

base/profiler/native_stack_sampler.h

@@ -59,19 +59,14 @@ class NativeStackSampler {
   // The following functions are all called on the SamplingThread (not the
   // thread being sampled).
 
-  // Notifies the sampler that we're starting to record a new profile. Modules
-  // shared across samples in the profile should be recorded in |modules|.
-  virtual void ProfileRecordingStarting(
-      std::vector<StackSamplingProfiler::Module>* modules) = 0;
+  // Notifies the sampler that we're starting to record a new profile.
+  virtual void ProfileRecordingStarting() = 0;
 
   // Records a set of stack frames and returns them.
-  virtual std::vector<StackSamplingProfiler::Frame> RecordStackFrames(
+  virtual std::vector<StackSamplingProfiler::InternalFrame> RecordStackFrames(
       StackBuffer* stackbuffer,
       StackSamplingProfiler::SamplingProfileBuilder* profile_builder) = 0;
 
-  // Notifies the sampler that we've stopped recording the current profile.
-  virtual void ProfileRecordingStopped() = 0;
-
  protected:
   NativeStackSampler();
 

base/profiler/native_stack_sampler_win.cc

@@ -33,7 +33,9 @@
 namespace base {
 
 using Frame = StackSamplingProfiler::Frame;
+using InternalFrame = StackSamplingProfiler::InternalFrame;
 using Module = StackSamplingProfiler::Module;
+using InternalModule = StackSamplingProfiler::InternalModule;
 using SamplingProfileBuilder = StackSamplingProfiler::SamplingProfileBuilder;
 
 // Stack recording functions --------------------------------------------------
@@ -389,27 +391,20 @@ class NativeStackSamplerWin : public NativeStackSampler {
   ~NativeStackSamplerWin() override;
 
   // StackSamplingProfiler::NativeStackSampler:
-  void ProfileRecordingStarting(std::vector<Module>* modules) override;
-  std::vector<Frame> RecordStackFrames(
+  void ProfileRecordingStarting() override;
+  std::vector<InternalFrame> RecordStackFrames(
       StackBuffer* stack_buffer,
       SamplingProfileBuilder* profile_builder) override;
-  void ProfileRecordingStopped() override;
 
  private:
   // Attempts to query the module filename, base address, and id for
-  // |module_handle|, and store them in |module|. Returns true if it succeeded.
-  static bool GetModuleForHandle(HMODULE module_handle, Module* module);
+  // |module_handle|, and returns them in an InternalModule object.
+  static InternalModule GetModuleForHandle(HMODULE module_handle);
 
-  // Gets the index for the Module corresponding to |module_handle| in
-  // |modules|, adding it if it's not already present. Returns
-  // StackSamplingProfiler::Frame::kUnknownModuleIndex if no Module can be
-  // determined for |module|.
-  size_t GetModuleIndex(HMODULE module_handle, std::vector<Module>* modules);
-
-  // Creates a set of frames with the information represented by |stack| and
-  // |modules|.
-  std::vector<Frame> CreateFrames(const std::vector<RecordedFrame>& stack,
-                                  std::vector<Module>* modules);
+  // Creates a set of internal frames with the information represented by
+  // |stack|.
+  std::vector<InternalFrame> CreateInternalFrames(
+      const std::vector<RecordedFrame>& stack);
 
   win::ScopedHandle thread_handle_;
 
@@ -418,13 +413,8 @@ class NativeStackSamplerWin : public NativeStackSampler {
   // The stack base address corresponding to |thread_handle_|.
   const void* const thread_stack_base_address_;
 
-  // Weak. Points to the modules associated with the profile being recorded
-  // between ProfileRecordingStarting() and ProfileRecordingStopped().
-  std::vector<Module>* current_modules_;
-
-  // Maps a module handle to the corresponding Module's index within
-  // current_modules_.
-  std::map<HMODULE, size_t> profile_module_index_;
+  // Maps a module handle to a StackSamplingProfiler::InternalModule object.
+  std::map<HMODULE, InternalModule> module_cache_;
 
   DISALLOW_COPY_AND_ASSIGN(NativeStackSamplerWin);
 };
@@ -437,78 +427,70 @@ NativeStackSamplerWin::NativeStackSamplerWin(
       thread_stack_base_address_(
           GetThreadEnvironmentBlock(thread_handle_.Get())->Tib.StackBase) {}
 
-NativeStackSamplerWin::~NativeStackSamplerWin() {
-}
+NativeStackSamplerWin::~NativeStackSamplerWin() {}
 
-void NativeStackSamplerWin::ProfileRecordingStarting(
-    std::vector<Module>* modules) {
-  current_modules_ = modules;
-  profile_module_index_.clear();
+void NativeStackSamplerWin::ProfileRecordingStarting() {
+  module_cache_.clear();
 }
 
-std::vector<Frame> NativeStackSamplerWin::RecordStackFrames(
+std::vector<InternalFrame> NativeStackSamplerWin::RecordStackFrames(
     StackBuffer* stack_buffer,
     SamplingProfileBuilder* profile_builder) {
   DCHECK(stack_buffer);
-  DCHECK(current_modules_);
 
   std::vector<RecordedFrame> stack;
   SuspendThreadAndRecordStack(thread_handle_.Get(), thread_stack_base_address_,
                               stack_buffer->buffer(), stack_buffer->size(),
                               &stack, profile_builder, test_delegate_);
 
-  return CreateFrames(stack, current_modules_);
-}
-
-void NativeStackSamplerWin::ProfileRecordingStopped() {
-  current_modules_ = nullptr;
+  return CreateInternalFrames(stack);
 }
 
 // static
-bool NativeStackSamplerWin::GetModuleForHandle(HMODULE module_handle,
-                                               Module* module) {
+InternalModule NativeStackSamplerWin::GetModuleForHandle(
+    HMODULE module_handle) {
   wchar_t module_name[MAX_PATH];
   DWORD result_length =
       ::GetModuleFileName(module_handle, module_name, size(module_name));
   if (result_length == 0)
-    return false;
-
-  module->filename = FilePath(module_name);
-  module->base_address = reinterpret_cast<uintptr_t>(module_handle);
-  module->id = GetBuildIDForModule(module_handle);
-  return !module->id.empty();
-}
+    return InternalModule();
 
-size_t NativeStackSamplerWin::GetModuleIndex(HMODULE module_handle,
-                                             std::vector<Module>* modules) {
-  if (!module_handle)
-    return Frame::kUnknownModuleIndex;
-
-  auto loc = profile_module_index_.find(module_handle);
-  if (loc == profile_module_index_.end()) {
-    Module module;
-    if (!GetModuleForHandle(module_handle, &module))
-      return Frame::kUnknownModuleIndex;
-    modules->push_back(module);
-    loc = profile_module_index_.insert(std::make_pair(
-        module_handle, modules->size() - 1)).first;
-  }
+  const std::string& module_id = GetBuildIDForModule(module_handle);
+  if (module_id.empty())
+    return InternalModule();
 
-  return loc->second;
+  return InternalModule(reinterpret_cast<uintptr_t>(module_handle), module_id,
+                        FilePath(module_name));
 }
 
-std::vector<Frame> NativeStackSamplerWin::CreateFrames(
-    const std::vector<RecordedFrame>& stack,
-    std::vector<Module>* modules) {
-  std::vector<Frame> frames;
-  frames.reserve(stack.size());
+std::vector<InternalFrame> NativeStackSamplerWin::CreateInternalFrames(
+    const std::vector<RecordedFrame>& stack) {
+  std::vector<InternalFrame> internal_frames;
+  internal_frames.reserve(stack.size());
 
   for (const auto& frame : stack) {
-    frames.emplace_back(reinterpret_cast<uintptr_t>(frame.instruction_pointer),
-                        GetModuleIndex(frame.module.Get(), modules));
+    auto frame_ip = reinterpret_cast<uintptr_t>(frame.instruction_pointer);
+
+    HMODULE module_handle = frame.module.Get();
+    if (!module_handle) {
+      internal_frames.emplace_back(frame_ip, InternalModule());
+      continue;
+    }
+
+    auto loc = module_cache_.find(module_handle);
+    if (loc != module_cache_.end()) {
+      internal_frames.emplace_back(frame_ip, loc->second);
+      continue;
+    }
+
+    InternalModule internal_module = GetModuleForHandle(module_handle);
+    if (internal_module.is_valid)
+      module_cache_.insert(std::make_pair(module_handle, internal_module));
+
+    internal_frames.emplace_back(frame_ip, std::move(internal_module));
   }
 
-  return frames;
+  return internal_frames;
 }
 
 }  // namespace

base/profiler/stack_sampling_profiler.cc

@@ -5,7 +5,6 @@
 #include "base/profiler/stack_sampling_profiler.h"
 
 #include <algorithm>
-#include <map>
 #include <utility>
 
 #include "base/atomic_sequence_num.h"
@@ -26,6 +25,8 @@
 
 namespace base {
 
+const size_t kUnknownModuleIndex = static_cast<size_t>(-1);
+
 namespace {
 
 // This value is used to initialize the WaitableEvent object. This MUST BE set
@@ -59,6 +60,7 @@ void ChangeAtomicFlags(subtle::Atomic32* flags,
 // StackSamplingProfiler::Module ----------------------------------------------
 
 StackSamplingProfiler::Module::Module() : base_address(0u) {}
+
 StackSamplingProfiler::Module::Module(uintptr_t base_address,
                                       const std::string& id,
                                       const FilePath& filename)
@@ -66,6 +68,17 @@ StackSamplingProfiler::Module::Module(uintptr_t base_address,
 
 StackSamplingProfiler::Module::~Module() = default;
 
+// StackSamplingProfiler::InternalModule --------------------------------------
+
+StackSamplingProfiler::InternalModule::InternalModule() : is_valid(false) {}
+
+StackSamplingProfiler::InternalModule::InternalModule(uintptr_t base_address,
+                                                      const std::string& id,
+                                                      const FilePath& filename)
+    : base_address(base_address), id(id), filename(filename), is_valid(true) {}
+
+StackSamplingProfiler::InternalModule::~InternalModule() = default;
+
 // StackSamplingProfiler::Frame -----------------------------------------------
 
 StackSamplingProfiler::Frame::Frame(uintptr_t instruction_pointer,
@@ -77,6 +90,16 @@ StackSamplingProfiler::Frame::~Frame() = default;
 StackSamplingProfiler::Frame::Frame()
     : instruction_pointer(0), module_index(kUnknownModuleIndex) {}
 
+// StackSamplingProfiler::InternalFrame -------------------------------------
+
+StackSamplingProfiler::InternalFrame::InternalFrame(
+    uintptr_t instruction_pointer,
+    InternalModule internal_module)
+    : instruction_pointer(instruction_pointer),
+      internal_module(std::move(internal_module)) {}
+
+StackSamplingProfiler::InternalFrame::~InternalFrame() = default;
+
 // StackSamplingProfiler::Sample ----------------------------------------------
 
 StackSamplingProfiler::Sample::Sample() = default;
@@ -140,9 +163,30 @@ void StackSamplingProfiler::SamplingProfileBuilder::OnProfileCompleted(
 }
 
 void StackSamplingProfiler::SamplingProfileBuilder::OnSampleCompleted(
-    std::vector<Frame> frames) {
+    std::vector<InternalFrame> internal_frames) {
   DCHECK_EQ(sample_.frames.size(), 0u);
-  sample_.frames = std::move(frames);
+
+  // Dedup modules and convert InternalFrames to Frames.
+  for (const auto& internal_frame : internal_frames) {
+    const InternalModule& module(internal_frame.internal_module);
+    if (!module.is_valid) {
+      sample_.frames.emplace_back(internal_frame.instruction_pointer,
+                                  kUnknownModuleIndex);
+      continue;
+    }
+
+    auto loc = module_index_.find(module.base_address);
+    if (loc == module_index_.end()) {
+      profile_.modules.emplace_back(module.base_address, module.id,
+                                    module.filename);
+      size_t index = profile_.modules.size() - 1;
+      loc = module_index_.insert(std::make_pair(module.base_address, index))
+                .first;
+    }
+    sample_.frames.emplace_back(internal_frame.instruction_pointer,
+                                loc->second);
+  }
+
   profile_.samples.push_back(std::move(sample_));
   sample_ = Sample();
 }
@@ -523,8 +567,6 @@ void StackSamplingProfiler::SamplingThread::FinishCollection(
   TimeDelta profile_duration = Time::Now() - collection->profile_start_time +
                                collection->params.sampling_interval;
 
-  collection->native_sampler->ProfileRecordingStopped();
-
   collection->profile_builder->OnProfileCompleted(
       profile_duration, collection->params.sampling_interval);
 
@@ -611,8 +653,7 @@ void StackSamplingProfiler::SamplingThread::RecordSampleTask(
   if (collection->sample_count == 0) {
     collection->profile_start_time = Time::Now();
     collection->next_sample_time = Time::Now();
-    collection->native_sampler->ProfileRecordingStarting(
-        collection->profile_builder->modules());
+    collection->native_sampler->ProfileRecordingStarting();
   }
 
   // Record a single sample.

base/profiler/stack_sampling_profiler.h

@@ -7,6 +7,7 @@
 
 #include <stddef.h>
 
+#include <map>
 #include <memory>
 #include <string>
 #include <vector>
@@ -23,6 +24,9 @@
 
 namespace base {
 
+// Identifies an unknown module.
+BASE_EXPORT extern const size_t kUnknownModuleIndex;
+
 class NativeStackSamplerTestDelegate;
 
 // StackSamplingProfiler periodically stops a thread to sample its stack, for
@@ -88,11 +92,39 @@ class BASE_EXPORT StackSamplingProfiler {
     FilePath filename;
   };
 
+  // InternalModule represents the module (DLL or exe) and its validness state.
+  // Different from Module, it has an additional field "is_valid".
+  //
+  // This struct is only used for sampling data transfer from NativeStackSampler
+  // to SamplingProfileBuilder.
+  struct BASE_EXPORT InternalModule {
+    InternalModule();
+    InternalModule(uintptr_t base_address,
+                   const std::string& id,
+                   const FilePath& filename);
+    ~InternalModule();
+
+    // Points to the base address of the module.
+    uintptr_t base_address;
+
+    // An opaque binary string that uniquely identifies a particular program
+    // version with high probability. This is parsed from headers of the loaded
+    // module.
+    // For binaries generated by GNU tools:
+    //   Contents of the .note.gnu.build-id field.
+    // On Windows:
+    //   GUID + AGE in the debug image headers of a module.
+    std::string id;
+
+    // The filename of the module.
+    FilePath filename;
+
+    // The validness of the module.
+    bool is_valid;
+  };
+
   // Frame represents an individual sampled stack frame with module information.
   struct BASE_EXPORT Frame {
-    // Identifies an unknown module.
-    static const size_t kUnknownModuleIndex = static_cast<size_t>(-1);
-
     Frame(uintptr_t instruction_pointer, size_t module_index);
     ~Frame();
 
@@ -107,6 +139,23 @@ class BASE_EXPORT StackSamplingProfiler {
     size_t module_index;
   };
 
+  // InternalFrame represents an individual sampled stack frame with full module
+  // information. This is different from Frame which only contains module index.
+  //
+  // This struct is only used for sampling data transfer from NativeStackSampler
+  // to SamplingProfileBuilder.
+  struct BASE_EXPORT InternalFrame {
+    InternalFrame(uintptr_t instruction_pointer,
+                  InternalModule internal_module);
+    ~InternalFrame();
+
+    // The sampled instruction pointer within the function.
+    uintptr_t instruction_pointer;
+
+    // The module information.
+    InternalModule internal_module;
+  };
+
   // Sample represents a set of stack frames with some extra information.
   struct BASE_EXPORT Sample {
     Sample();
@@ -232,10 +281,7 @@ class BASE_EXPORT StackSamplingProfiler {
 
     // Records a new set of frames to sample_. Invoked when sampling a Sample
     // completes.
-    void OnSampleCompleted(std::vector<Frame> frames);
-
-    // Returns the address of modules in profile_.
-    std::vector<Module>* modules() { return &profile_.modules; }
+    void OnSampleCompleted(std::vector<InternalFrame> internal_frames);
 
    private:
     // The collected stack samples.
@@ -244,6 +290,9 @@ class BASE_EXPORT StackSamplingProfiler {
     // The current sample being recorded.
     Sample sample_;
 
+    // Maps Module's base_address to index.
+    std::map<uintptr_t, size_t> module_index_;
+
     // Callback made when sampling a profile completes.
     const CompletedCallback callback_;
 

components/metrics/call_stack_profile_metrics_provider.cc

@@ -249,7 +249,7 @@ void CopySampleToProto(
     // A frame may not have a valid module. If so, we can't compute the
     // instruction pointer offset, and we don't want to send bare pointers, so
     // leave call_stack_entry empty.
-    if (frame.module_index == StackSamplingProfiler::Frame::kUnknownModuleIndex)
+    if (frame.module_index == base::kUnknownModuleIndex)
       continue;
     int64_t module_offset =
         reinterpret_cast<const char*>(frame.instruction_pointer) -

components/metrics/call_stack_profile_metrics_provider_unittest.cc

@@ -361,7 +361,7 @@ TEST_F(CallStackProfileMetricsProviderTest, RepeatedStacksOrdered) {
 TEST_F(CallStackProfileMetricsProviderTest, UnknownModule) {
   Profile profile = ProfileFactory(100, 10)
                         .NewSample()
-                        .AddFrame(Frame::kUnknownModuleIndex, 0x1234)
+                        .AddFrame(base::kUnknownModuleIndex, 0x1234)
                         .Build();
 
   const ExpectedProtoEntry expected_proto_entries[] = {
@@ -404,7 +404,7 @@ TEST_F(CallStackProfileMetricsProviderTest, ProfileProvidedOnlyOnce) {
         // Use the sampling period to distinguish the two profiles.
         ProfileFactory(100, r)
             .NewSample()
-            .AddFrame(Frame::kUnknownModuleIndex, 0x1234)
+            .AddFrame(base::kUnknownModuleIndex, 0x1234)
             .Build();
 
     CallStackProfileMetricsProvider provider;
@@ -433,7 +433,7 @@ TEST_F(CallStackProfileMetricsProviderTest,
        ProfileProvidedWhenCollectedBeforeInstantiation) {
   Profile profile = ProfileFactory(100, 10)
                         .NewSample()
-                        .AddFrame(Frame::kUnknownModuleIndex, 0x1234)
+                        .AddFrame(base::kUnknownModuleIndex, 0x1234)
                         .Build();
 
   CallStackProfileParams params(CallStackProfileParams::BROWSER_PROCESS,
@@ -455,7 +455,7 @@ TEST_F(CallStackProfileMetricsProviderTest,
 TEST_F(CallStackProfileMetricsProviderTest, ProfileNotProvidedWhileDisabled) {
   Profile profile = ProfileFactory(100, 10)
                         .NewSample()
-                        .AddFrame(Frame::kUnknownModuleIndex, 0x1234)
+                        .AddFrame(base::kUnknownModuleIndex, 0x1234)
                         .Build();
 
   CallStackProfileMetricsProvider provider;
@@ -488,7 +488,7 @@ TEST_F(CallStackProfileMetricsProviderTest,
 
   Profile profile = ProfileFactory(100, 10)
                         .NewSample()
-                        .AddFrame(Frame::kUnknownModuleIndex, 0x1234)
+                        .AddFrame(base::kUnknownModuleIndex, 0x1234)
                         .Build();
   callback.Run(std::move(profile));
   ChromeUserMetricsExtension uma_proto;
@@ -515,7 +515,7 @@ TEST_F(CallStackProfileMetricsProviderTest,
 
   Profile profile = ProfileFactory(100, 10)
                         .NewSample()
-                        .AddFrame(Frame::kUnknownModuleIndex, 0x1234)
+                        .AddFrame(base::kUnknownModuleIndex, 0x1234)
                         .Build();
   callback.Run(std::move(profile));
   ChromeUserMetricsExtension uma_proto;
@@ -541,7 +541,7 @@ TEST_F(CallStackProfileMetricsProviderTest,
 
   Profile profile = ProfileFactory(100, 10)
                         .NewSample()
-                        .AddFrame(Frame::kUnknownModuleIndex, 0x1234)
+                        .AddFrame(base::kUnknownModuleIndex, 0x1234)
                         .Build();
   callback.Run(std::move(profile));
   ChromeUserMetricsExtension uma_proto;

components/metrics/public/cpp/call_stack_profile_struct_traits.h

@@ -56,17 +56,16 @@ struct StructTraits<metrics::mojom::CallStackFrameDataView,
   }
   static uint64_t module_index(
       const base::StackSamplingProfiler::Frame& frame) {
-    return frame.module_index ==
-        base::StackSamplingProfiler::Frame::kUnknownModuleIndex ?
-        static_cast<uint64_t>(-1) :
-        frame.module_index;
+    return frame.module_index == base::kUnknownModuleIndex
+               ? static_cast<uint64_t>(-1)
+               : frame.module_index;
   }
 
   static bool Read(metrics::mojom::CallStackFrameDataView data,
                    base::StackSamplingProfiler::Frame* out) {
-    size_t module_index = data.module_index() == static_cast<uint64_t>(-1) ?
-        base::StackSamplingProfiler::Frame::kUnknownModuleIndex :
-        data.module_index();
+    size_t module_index = data.module_index() == static_cast<uint64_t>(-1)
+                              ? base::kUnknownModuleIndex
+                              : data.module_index();
 
     // We can't know whether the module_index field is valid at this point since
     // we don't have access to the number of modules here. This will be checked
@@ -128,8 +127,7 @@ struct StructTraits<metrics::mojom::CallStackProfileDataView,
     for (const base::StackSamplingProfiler::Sample& sample : samples) {
       for (const base::StackSamplingProfiler::Frame& frame : sample.frames) {
         if (frame.module_index >= module_count &&
-            frame.module_index !=
-                base::StackSamplingProfiler::Frame::kUnknownModuleIndex)
+            frame.module_index != base::kUnknownModuleIndex)
           return false;
       }
     }

components/metrics/public/cpp/call_stack_profile_struct_traits_unittest.cc

@@ -151,20 +151,20 @@ TEST_F(CallStackProfileStructTraitsTest, Frame) {
   using Frame = base::StackSamplingProfiler::Frame;
 
   const Frame serialize_cases[] = {
-    // Null instruction pointer.
-    Frame(0x0, 10),
-    // Non-null instruction pointer.
-    Frame(0x10, 10),
-    // Instruction pointer with a bit set beyond 32 bits, when built for x64.
-    Frame(1ULL << (sizeof(uintptr_t) * 8) * 3 / 4, 10),
-    // Zero module index.
-    Frame(0xabcd, 0),
-    // Non-zero module index.
-    Frame(0xabcd, 1),
-    // Non-zero module index.
-    Frame(0xabcd, 10),
-    // Unknown module index.
-    Frame(0xabcd, Frame::kUnknownModuleIndex),
+      // Null instruction pointer.
+      Frame(0x0, 10),
+      // Non-null instruction pointer.
+      Frame(0x10, 10),
+      // Instruction pointer with a bit set beyond 32 bits, when built for x64.
+      Frame(1ULL << (sizeof(uintptr_t) * 8) * 3 / 4, 10),
+      // Zero module index.
+      Frame(0xabcd, 0),
+      // Non-zero module index.
+      Frame(0xabcd, 1),
+      // Non-zero module index.
+      Frame(0xabcd, 10),
+      // Unknown module index.
+      Frame(0xabcd, base::kUnknownModuleIndex),
   };
 
   for (const Frame& input : serialize_cases) {
@@ -179,10 +179,11 @@ TEST_F(CallStackProfileStructTraitsTest, Frame) {
 // Checks serialization/deserialization of Profile fields, including validation
 // of the Frame module_index field.
 TEST_F(CallStackProfileStructTraitsTest, Profile) {
-  using Module = base::StackSamplingProfiler::Module;
-  using Frame = base::StackSamplingProfiler::Frame;
-  using Sample = base::StackSamplingProfiler::Sample;
-  using Profile = base::StackSamplingProfiler::CallStackProfile;
+  using base::StackSamplingProfiler;
+  using Module = StackSamplingProfiler::Module;
+  using Frame = StackSamplingProfiler::Frame;
+  using Sample = StackSamplingProfiler::Sample;
+  using Profile = StackSamplingProfiler::CallStackProfile;
 
   struct SerializeCase {
     Profile profile;
@@ -190,60 +191,49 @@ TEST_F(CallStackProfileStructTraitsTest, Profile) {
   };
 
   const SerializeCase serialize_cases[] = {
-    // Empty modules and samples.
-    {
-      CreateProfile(std::vector<Module>(), std::vector<Sample>(),
-                    base::TimeDelta::FromSeconds(1),
-                    base::TimeDelta::FromSeconds(2)),
-      true
-    },
-    // Non-empty modules and empty samples.
-    {
-      CreateProfile({ Module(0x4000, "a", base::FilePath()) },
-                    std::vector<Sample>(),
-                    base::TimeDelta::FromSeconds(1),
-                    base::TimeDelta::FromSeconds(2)),
-      true
-    },
-    // Valid values for modules and samples.
-    {
-      CreateProfile({
-                      Module(0x4000, "a", base::FilePath()),
-                      Module(0x4100, "b", base::FilePath()),
-                    },
-                    {
-                      Sample({
-                          Frame(0x4010, 0),
-                          Frame(0x4110, 1),
-                          Frame(0x4110, Frame::kUnknownModuleIndex),
-                      }),
-                    },
-                    base::TimeDelta::FromSeconds(1),
-                    base::TimeDelta::FromSeconds(2)),
-      true
-    },
-    // Valid values for modules, but an out of range module index in the second
-    // sample.
-    {
-      CreateProfile({
-                      Module(0x4000, "a", base::FilePath()),
-                      Module(0x4100, "b", base::FilePath()),
-                    },
-                    {
-                      Sample({
-                          Frame(0x4010, 0),
-                          Frame(0x4110, 1),
-                          Frame(0x4110, Frame::kUnknownModuleIndex),
-                      }),
-                      Sample({
-                          Frame(0x4010, 0),
-                          Frame(0x4110, 2),
-                      }),
-                    },
-                    base::TimeDelta::FromSeconds(1),
-                    base::TimeDelta::FromSeconds(2)),
-      false
-    },
+      // Empty modules and samples.
+      {CreateProfile(std::vector<Module>(), std::vector<Sample>(),
+                     base::TimeDelta::FromSeconds(1),
+                     base::TimeDelta::FromSeconds(2)),
+       true},
+      // Non-empty modules and empty samples.
+      {CreateProfile({Module(0x4000, "a", base::FilePath())},
+                     std::vector<Sample>(), base::TimeDelta::FromSeconds(1),
+                     base::TimeDelta::FromSeconds(2)),
+       true},
+      // Valid values for modules and samples.
+      {CreateProfile(
+           {
+               Module(0x4000, "a", base::FilePath()),
+               Module(0x4100, "b", base::FilePath()),
+           },
+           {
+               Sample({
+                   Frame(0x4010, 0), Frame(0x4110, 1),
+                   Frame(0x4110, base::kUnknownModuleIndex),
+               }),
+           },
+           base::TimeDelta::FromSeconds(1), base::TimeDelta::FromSeconds(2)),
+       true},
+      // Valid values for modules, but an out of range module index in the
+      // second
+      // sample.
+      {CreateProfile(
+           {
+               Module(0x4000, "a", base::FilePath()),
+               Module(0x4100, "b", base::FilePath()),
+           },
+           {
+               Sample({
+                   Frame(0x4010, 0), Frame(0x4110, 1),
+                   Frame(0x4110, base::kUnknownModuleIndex),
+               }),
+               Sample({
+                   Frame(0x4010, 0), Frame(0x4110, 2),
+               }),
+           },
+           base::TimeDelta::FromSeconds(1), base::TimeDelta::FromSeconds(2)),
+       false},
   };
 
   for (const SerializeCase& input : serialize_cases) {