arc: enable Android tracing in verified-boot mode

components/arc/trace/arc_trace_bridge.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/arc/trace/arc_trace_bridge.h"
+
+#include <string>
+#include <utility>
+
+#include "base/logging.h"
+#include "base/trace_event/trace_event.h"
+#include "components/arc/arc_bridge_service.h"
+
+namespace arc {
+
+namespace {
+// This class is owned by ArcServiceManager so that it is safe to use this raw
+// pointer as the singleton reference.
+ArcTraceBridge* g_arc_trace_bridge = nullptr;
+
+// The prefix of the categories to be shown on the trace selection UI.
+const std::string g_category_prefix(

[Luis Héctor Chávez, 2016/10/11 03:38:49]

nit: constexpr char kCategoryPrefix[] = ...;

[shunhsingou, 2016/10/11 07:22:05]

Done.

+    TRACE_DISABLED_BY_DEFAULT("android.trace."));
+
+}  // namcespace
+
+ArcTraceBridge* ArcTraceBridge::Get() {
+  DCHECK(g_arc_trace_bridge);
+  return g_arc_trace_bridge;
+}
+
+ArcTraceBridge::ArcTraceBridge(ArcBridgeService* bridge_service)
+    : ArcService(bridge_service), binding_(this), weak_ptr_factory_(this) {
+  arc_bridge_service()->trace()->AddObserver(this);
+  g_arc_trace_bridge = this;
+}
+
+ArcTraceBridge::~ArcTraceBridge() {
+  arc_bridge_service()->trace()->RemoveObserver(this);
+  g_arc_trace_bridge = nullptr;
+}
+
+void ArcTraceBridge::OnInstanceReady() {
+  auto* trace_instance = arc_bridge_service()->trace()->GetInstanceForMethod(
+      "QueryAvailableCategories");
+  if (!trace_instance) {
+    LOG(ERROR) << "OnTraceInstanceReady called, "
+               << "but no trace instance found";

[Luis Héctor Chávez, 2016/10/11 03:38:49]

GetInstanceForMethod already does logging, so this is superfluous. In fact, you
can probably just call QueryAvailableCategories().

[shunhsingou, 2016/10/11 07:22:05]

Done.

+    return;
+  }
+  QueryAvailableCategories();
+}
+
+void ArcTraceBridge::QueryAvailableCategories() {
+  auto* trace_instance = arc_bridge_service()->trace()->GetInstanceForMethod(
+      "QueryAvailableCategories");
+  if (trace_instance) {

[Luis Héctor Chávez, 2016/10/11 03:38:49]

nit:

if (!trace_instance)
  return;

[shunhsingou, 2016/10/11 07:22:05]

Done.

+    trace_instance->QueryAvailableCategories(
+        base::Bind(&ArcTraceBridge::OnCategoriesReady,
+                   weak_ptr_factory_.GetWeakPtr()));
+  }
+}
+
+void ArcTraceBridge::OnCategoriesReady(mojo::Array<mojo::String> categories) {
+  categories_ = categories;

[Luis Héctor Chávez, 2016/10/11 03:38:49]

This needs thread-checking code, like
DCHECK(thread_checker_.CalledOnValidThread()); Same in all other methods that
use |categories_|.

[shunhsingou, 2016/10/11 07:22:05]

Done.

+  for (size_t i = 0; i < categories_.size(); ++i) {

[Luis Héctor Chávez, 2016/10/11 03:38:49]

You can use

for (const auto& category : categories_) {

Same below.

[shunhsingou, 2016/10/11 07:22:05]

Done.

+    std::string full_category_name =
+        g_category_prefix + std::string(categories_.at(i));
+
+    // Shows the category name in the selection UI.
+    base::trace_event::TraceLog::GetCategoryGroupEnabled(
+        full_category_name.c_str());
+  }
+}
+
+void ArcTraceBridge::StartTracing(
+    const base::trace_event::TraceConfig& trace_config) {
+
+  mojo::Array<mojo::String> categories;
+
+  for (size_t i = 0; i < categories_.size(); ++i) {
+    std::string category_name = categories_.at(i);
+    std::string full_category_name = g_category_prefix + category_name;

[Luis Héctor Chávez, 2016/10/11 03:38:49]

Should you store the full name in addition to the short name to avoid performing
recalculation every time you do this?

[shunhsingou, 2016/10/11 07:22:05]

Done.

+    if (trace_config.IsCategoryGroupEnabled(full_category_name.c_str())) {
+      categories.push_back(category_name);
+    }
+  }
+
+  auto* trace_instance =
+      arc_bridge_service()->trace()->GetInstanceForMethod("StartTracing");
+  if (trace_instance) trace_instance->StartTracing(std::move(categories));

[Luis Héctor Chávez, 2016/10/11 03:38:49]

This should be at the beginning of the function, using the

if (!trace_instance)
  return;

pattern.

[shunhsingou, 2016/10/11 07:22:05]

Done.

+}
+
+void ArcTraceBridge::StopTracing() {
+  auto* trace_instance =
+      arc_bridge_service()->trace()->GetInstanceForMethod("StopTracing");
+  if (trace_instance) trace_instance->StopTracing();
+}
+
+}  // namespace arc