Introduce mojo interface for collecting StackSamplingProfiler data

components/metrics/public/cpp/call_stack_profile_struct_traits.h

+// 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.
+
+// Defines StructTraits specializations for translating between mojo types and
+// base::StackSamplingProfiler types, with data validity checks.
+
+#ifndef COMPONENTS_METRICS_CALL_STACK_PROFILE_STRUCT_TRAITS_H_
+#define COMPONENTS_METRICS_CALL_STACK_PROFILE_STRUCT_TRAITS_H_
+
+#include <vector>
+
+#include "base/files/file_path.h"
+#include "base/profiler/stack_sampling_profiler.h"
+#include "components/metrics/public/interfaces/call_stack_profile_collector.mojom.h"
+
+namespace mojo {
+
+template <>
+struct StructTraits<metrics::mojom::CallStackModule,
+                    base::StackSamplingProfiler::Module> {
+  static uint64_t base_address(
+      const base::StackSamplingProfiler::Module& module) {
+    return module.base_address;
+  }
+  static const std::string& id(
+      const base::StackSamplingProfiler::Module& module) {
+    return module.id;
+  }
+  static const base::FilePath& filename(
+      const base::StackSamplingProfiler::Module& module) {
+    return module.filename;
+  }
+
+  static bool Read(metrics::mojom::CallStackModuleDataView data,
+                   base::StackSamplingProfiler::Module* out) {
+    std::string id;
+    base::FilePath filename;
+    if (!data.ReadId(&id) || !data.ReadFilename(&filename))

[Ilya Sherman, 2016/08/18 07:59:18]

Tom didn't comment on this, so it's probably not an issue, but: Is there any
need to check for malicious input, possibly in the form of very large string
lengths?  I know that this was a concern for the Autofill IPC code; I don't know
whether Mojo has some ... mojo ... to handle this at some higher level of
abstraction.

[Mike Wittman, 2016/08/18 17:49:19]

The only use made of the filename is to generate a hash value, so long strings
there shouldn't be an issue.

The id is used as an opaque value so there shouldn't be an issue there either.
The expected values are at most 40 bytes, however, so I added a check for that.

[Ilya Sherman, 2016/08/18 21:39:49]

My understanding is that it's undesirable to allow a potentially compromised
remote process result in an attacker controlling a relatively large block of
memory in the browser process, as this makes it easier to circumvent some types
of address randomization defenses.  But, I am not a security expert.

+      return false;
+
+    *out =
+        base::StackSamplingProfiler::Module(data.base_address(), id, filename);
+    return true;
+  }
+};
+
+template <>
+struct StructTraits<metrics::mojom::CallStackFrame,
+                    base::StackSamplingProfiler::Frame> {
+  static uint64_t instruction_pointer(
+      const base::StackSamplingProfiler::Frame& frame) {
+    return frame.instruction_pointer;
+  }
+  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;
+  }
+
+  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();
+
+    // The module_index field must be checked to be in the valid range when
+    // reading CallStackProfile.

[Ilya Sherman, 2016/08/18 07:59:18]

Sorry, I'm not following what this comment means.  Where must this check happen?
 Do you mean that it's somehow happening in the line below?

I *think* that this validation actually happens within ValidateSamples().  But,
I imagine that it's possible to call this function without reaching the
ValidateSamples() code below, right?  If so, it would be nice if this caveat
were more prominently documented somewhere.  Maybe there isn't a better spot for
it, though.  (Have I mentioned that I'm not very fluent in Mojo?)

[Mike Wittman, 2016/08/18 17:49:19]

Clarified the comment.

Frames are not meaningful outside the context of a CallStackProfile, so this
function would never be executed without the check below happening subsequently.

+    *out = base::StackSamplingProfiler::Frame(data.instruction_pointer(),
+                                              module_index);
+    return true;
+  }
+};
+
+template <>
+struct StructTraits<metrics::mojom::CallStackProfile,
+                    base::StackSamplingProfiler::CallStackProfile> {
+  static const std::vector<base::StackSamplingProfiler::Module>& modules(
+      const base::StackSamplingProfiler::CallStackProfile& profile) {
+    return profile.modules;
+  }
+  static const std::vector<base::StackSamplingProfiler::Sample>& samples(
+      const base::StackSamplingProfiler::CallStackProfile& profile) {
+    return profile.samples;
+  }
+  static const base::TimeDelta profile_duration(
+      const base::StackSamplingProfiler::CallStackProfile& profile) {
+    return profile.profile_duration;
+  }
+  static const base::TimeDelta sampling_period(
+      const base::StackSamplingProfiler::CallStackProfile& profile) {
+    return profile.sampling_period;
+  }
+
+  static bool ValidateSamples(
+      std::vector<base::StackSamplingProfiler::Sample> samples,
+      size_t module_count) {
+    for (const base::StackSamplingProfiler::Sample& sample : samples) {
+      for (const base::StackSamplingProfiler::Frame& frame : sample) {
+        if (frame.module_index >= module_count && frame.module_index !=

[Ilya Sherman, 2016/08/18 07:59:18]

nit: This is a weird spot to break the line.  Is this clang-format output?

[Mike Wittman, 2016/08/18 17:49:19]

Updated formatting to clang output.

+            base::StackSamplingProfiler::Frame::kUnknownModuleIndex)
+          return false;
+      }
+    }
+    return true;
+  }
+
+  static bool Read(metrics::mojom::CallStackProfileDataView data,
+                   base::StackSamplingProfiler::CallStackProfile* out) {
+    std::vector<base::StackSamplingProfiler::Module> modules;
+    std::vector<base::StackSamplingProfiler::Sample> samples;
+    base::TimeDelta profile_duration, sampling_period;
+    if (!data.ReadModules(&modules) || !data.ReadSamples(&samples) ||
+        !data.ReadProfileDuration(&profile_duration) ||
+        !data.ReadSamplingPeriod(&sampling_period) ||
+        !ValidateSamples(samples, modules.size()))
+      return false;
+
+    *out = base::StackSamplingProfiler::CallStackProfile();
+    out->modules = std::move(modules);
+    out->samples = std::move(samples);
+    out->profile_duration = profile_duration;
+    out->sampling_period = sampling_period;
+    return true;
+  }
+};
+
+}  // mojo
+
+#endif  // COMPONENTS_METRICS_CALL_STACK_PROFILE_STRUCT_TRAITS_H_