Merge "metrics_collector: add libmetricscollectorservice binder service"

metricsd/Android.mk

@@ -28,6 +28,7 @@ metrics_collector_common := \
   collectors/cpu_usage_collector.cc \
   collectors/disk_usage_collector.cc \
   metrics_collector.cc \
+  metrics_collector_service_trampoline.cc \
   persistent_integer.cc
 
 metricsd_common := \
@@ -69,6 +70,7 @@ metrics_includes := external/gtest/include \
   $(LOCAL_PATH)/include
 libmetrics_shared_libraries := libchrome libbinder libbrillo libutils
 metrics_collector_shared_libraries := $(libmetrics_shared_libraries) \
+  libbrillo-binder \
   libbrillo-dbus \
   libbrillo-http \
   libchrome-dbus \
@@ -77,6 +79,8 @@ metrics_collector_shared_libraries := $(libmetrics_shared_libraries) \
   librootdev \
   libweaved
 
+metrics_collector_static_libraries := libmetricscollectorservice
+
 metricsd_shared_libraries := \
   libbinder \
   libbrillo \
@@ -86,7 +90,7 @@ metricsd_shared_libraries := \
   libupdate_engine_client \
   libutils
 
-# Static proxy library for the binder interface.
+# Static proxy library for the metricsd binder interface.
 # ========================================================
 include $(CLEAR_VARS)
 LOCAL_MODULE := metricsd_binder_proxy
@@ -94,6 +98,21 @@ LOCAL_SHARED_LIBRARIES := libbinder libutils
 LOCAL_SRC_FILES := aidl/android/brillo/metrics/IMetricsd.aidl
 include $(BUILD_STATIC_LIBRARY)
 
+# Static library for the metrics_collector binder interface.
+# ==========================================================
+include $(CLEAR_VARS)
+LOCAL_MODULE := libmetricscollectorservice
+LOCAL_SHARED_LIBRARIES := libbinder libbrillo-binder libchrome libutils
+LOCAL_CPP_EXTENSION := $(metrics_cpp_extension)
+LOCAL_C_INCLUDES := $(LOCAL_PATH)/include
+LOCAL_EXPORT_C_INCLUDE_DIRS := $(LOCAL_PATH)/include
+LOCAL_SRC_FILES := \
+  aidl/android/brillo/metrics/IMetricsCollectorService.aidl \
+  metrics_collector_service_impl.cc \
+  metrics_collector_service_client.cc
+LOCAL_RTTI_FLAG := -fno-rtti
+include $(BUILD_STATIC_LIBRARY)
+
 # Shared library for metrics.
 # ========================================================
 include $(CLEAR_VARS)
@@ -151,7 +170,8 @@ LOCAL_RTTI_FLAG := -frtti
 LOCAL_SHARED_LIBRARIES := $(metrics_collector_shared_libraries)
 LOCAL_SRC_FILES := $(metrics_collector_common) \
   metrics_collector_main.cc
-LOCAL_STATIC_LIBRARIES := metricsd_binder_proxy
+LOCAL_STATIC_LIBRARIES := metricsd_binder_proxy \
+  $(metrics_collector_static_libraries)
 include $(BUILD_EXECUTABLE)
 
 # metricsd daemon.
@@ -197,7 +217,8 @@ LOCAL_RTTI_FLAG := -frtti
 LOCAL_SHARED_LIBRARIES := $(metrics_collector_shared_libraries)
 LOCAL_SRC_FILES := $(metrics_collector_tests_sources) \
   $(metrics_collector_common)
-LOCAL_STATIC_LIBRARIES := libBionicGtestMain libgmock metricsd_binder_proxy
+LOCAL_STATIC_LIBRARIES := libBionicGtestMain libgmock metricsd_binder_proxy \
+  $(metrics_collector_static_libraries)
 include $(BUILD_NATIVE_TEST)
 
 # Weave schema files

metricsd/aidl/android/brillo/metrics/IMetricsCollectorService.aidl

@@ -0,0 +1,21 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.brillo.metrics;
+
+interface IMetricsCollectorService {
+  oneway void notifyUserCrash();
+}

metricsd/include/metrics/metrics_collector_service_client.h

@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Client interface to IMetricsCollectorService.
+
+#ifndef METRICS_METRICS_COLLECTOR_SERVICE_CLIENT_H_
+#define METRICS_METRICS_COLLECTOR_SERVICE_CLIENT_H_
+
+#include "android/brillo/metrics/IMetricsCollectorService.h"
+
+class MetricsCollectorServiceClient {
+ public:
+  MetricsCollectorServiceClient() = default;
+  ~MetricsCollectorServiceClient() = default;
+
+  // Initialize.  Returns true if OK, or false if IMetricsCollectorService
+  // is not registered.
+  bool Init();
+
+  // Called by crash_reporter to report a userspace crash event.  Returns
+  // true if successfully called the IMetricsCollectorService method of the
+  // same name, or false if the service was not registered at Init() time.
+  bool notifyUserCrash();
+
+ private:
+  // IMetricsCollectorService binder proxy
+  android::sp<android::brillo::metrics::IMetricsCollectorService>
+      metrics_collector_service_;
+};
+
+#endif  // METRICS_METRICS_COLLECTOR_SERVICE_CLIENT_H_

metricsd/metrics_collector.cc

@@ -33,6 +33,7 @@
 #include <dbus/message.h>
 
 #include "constants.h"
+#include "metrics_collector_service_trampoline.h"
 
 using base::FilePath;
 using base::StringPrintf;
@@ -46,11 +47,6 @@ using std::vector;
 
 namespace {
 
-const char kCrashReporterInterface[] = "org.chromium.CrashReporter";
-const char kCrashReporterUserCrashSignal[] = "UserCrash";
-const char kCrashReporterMatchRule[] =
-    "type='signal',interface='%s',path='/',member='%s'";
-
 const int kSecondsPerMinute = 60;
 const int kMinutesPerHour = 60;
 const int kHoursPerDay = 24;
@@ -130,6 +126,10 @@ int MetricsCollector::Run() {
     version_cumulative_cpu_use_->Set(0);
   }
 
+  // Start metricscollectorservice via trampoline
+  MetricsCollectorServiceTrampoline metricscollectorservice_trampoline(this);
+  metricscollectorservice_trampoline.Run();
+
   return brillo::DBusDaemon::Run();
 }
 
@@ -223,28 +223,6 @@ int MetricsCollector::OnInit() {
   bus_->AssertOnDBusThread();
   CHECK(bus_->SetUpAsyncOperations());
 
-  if (bus_->is_connected()) {
-    const std::string match_rule =
-        base::StringPrintf(kCrashReporterMatchRule,
-                           kCrashReporterInterface,
-                           kCrashReporterUserCrashSignal);
-
-    bus_->AddFilterFunction(&MetricsCollector::MessageFilter, this);
-
-    DBusError error;
-    dbus_error_init(&error);
-    bus_->AddMatch(match_rule, &error);
-
-    if (dbus_error_is_set(&error)) {
-      LOG(ERROR) << "Failed to add match rule \"" << match_rule << "\". Got "
-          << error.name << ": " << error.message;
-      return EX_SOFTWARE;
-    }
-  } else {
-    LOG(ERROR) << "DBus isn't connected.";
-    return EX_UNAVAILABLE;
-  }
-
   device_ = weaved::Device::CreateInstance(
       bus_,
       base::Bind(&MetricsCollector::UpdateWeaveState, base::Unretained(this)));
@@ -268,23 +246,6 @@ int MetricsCollector::OnInit() {
 }
 
 void MetricsCollector::OnShutdown(int* return_code) {
-  if (!testing_ && bus_->is_connected()) {
-    const std::string match_rule =
-        base::StringPrintf(kCrashReporterMatchRule,
-                           kCrashReporterInterface,
-                           kCrashReporterUserCrashSignal);
-
-    bus_->RemoveFilterFunction(&MetricsCollector::MessageFilter, this);
-
-    DBusError error;
-    dbus_error_init(&error);
-    bus_->RemoveMatch(match_rule, &error);
-
-    if (dbus_error_is_set(&error)) {
-      LOG(ERROR) << "Failed to remove match rule \"" << match_rule << "\". Got "
-          << error.name << ": " << error.message;
-    }
-  }
   brillo::DBusDaemon::OnShutdown(return_code);
 }
 
@@ -340,36 +301,6 @@ void MetricsCollector::UpdateWeaveState() {
   }
 }
 
-// static
-DBusHandlerResult MetricsCollector::MessageFilter(DBusConnection* connection,
-                                                   DBusMessage* message,
-                                                   void* user_data) {
-  int message_type = dbus_message_get_type(message);
-  if (message_type != DBUS_MESSAGE_TYPE_SIGNAL) {
-    DLOG(WARNING) << "unexpected message type " << message_type;
-    return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
-  }
-
-  // Signal messages always have interfaces.
-  const std::string interface(dbus_message_get_interface(message));
-  const std::string member(dbus_message_get_member(message));
-  DLOG(INFO) << "Got " << interface << "." << member << " D-Bus signal";
-
-  MetricsCollector* daemon = static_cast<MetricsCollector*>(user_data);
-
-  DBusMessageIter iter;
-  dbus_message_iter_init(message, &iter);
-  if (interface == kCrashReporterInterface) {
-    CHECK_EQ(member, kCrashReporterUserCrashSignal);
-    daemon->ProcessUserCrash();
-  } else {
-    // Ignore messages from the bus itself.
-    return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
-  }
-
-  return DBUS_HANDLER_RESULT_HANDLED;
-}
-
 void MetricsCollector::ProcessUserCrash() {
   // Counts the active time up to now.
   UpdateStats(TimeTicks::Now(), Time::Now());

metricsd/metrics_collector.h

@@ -63,6 +63,10 @@ class MetricsCollector : public brillo::DBusDaemon {
   // Returns the active time since boot (uptime minus sleep time) in seconds.
   static double GetActiveTime();
 
+  // Updates the active use time and logs time between user-space
+  // process crashes.  Called via MetricsCollectorServiceTrampoline.
+  void ProcessUserCrash();
+
  protected:
   // Used also by the unit tests.
   static const char kComprDataSizeName[];
@@ -108,11 +112,6 @@ class MetricsCollector : public brillo::DBusDaemon {
     int value;               // value from /proc/meminfo
   };
 
-  // D-Bus filter callback.
-  static DBusHandlerResult MessageFilter(DBusConnection* connection,
-                                         DBusMessage* message,
-                                         void* user_data);
-
   // Enables metrics reporting.
   void OnEnableMetrics(const std::weak_ptr<weaved::Command>& cmd);
 
@@ -122,10 +121,6 @@ class MetricsCollector : public brillo::DBusDaemon {
   // Updates the weave device state.
   void UpdateWeaveState();
 
-  // Updates the active use time and logs time between user-space
-  // process crashes.
-  void ProcessUserCrash();
-
   // Updates the active use time and logs time between kernel crashes.
   void ProcessKernelCrash();
 

metricsd/metrics_collector_service_client.cc

@@ -0,0 +1,50 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Client interface to IMetricsCollectorService.
+
+#include "metrics/metrics_collector_service_client.h"
+
+#include <base/logging.h>
+#include <binder/IServiceManager.h>
+#include <utils/String16.h>
+
+#include "android/brillo/metrics/IMetricsCollectorService.h"
+
+namespace {
+const char kMetricsCollectorServiceName[] =
+    "android.brillo.metrics.IMetricsCollectorService";
+}
+
+bool MetricsCollectorServiceClient::Init() {
+  const android::String16 name(kMetricsCollectorServiceName);
+  metrics_collector_service_ = android::interface_cast<
+      android::brillo::metrics::IMetricsCollectorService>(
+      android::defaultServiceManager()->checkService(name));
+
+  if (metrics_collector_service_ == nullptr)
+    LOG(ERROR) << "Unable to lookup service " << kMetricsCollectorServiceName;
+
+  return metrics_collector_service_ != nullptr;
+}
+
+bool MetricsCollectorServiceClient::notifyUserCrash() {
+  if (metrics_collector_service_ == nullptr)
+    return false;
+
+  metrics_collector_service_->notifyUserCrash();
+  return true;
+}

metricsd/metrics_collector_service_impl.cc

@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "metrics_collector_service_impl.h"
+
+#include <binder/IServiceManager.h>
+#include <binder/Status.h>
+#include <brillo/binder_watcher.h>
+#include <utils/Errors.h>
+
+#include "metrics_collector_service_trampoline.h"
+
+using namespace android;
+
+BnMetricsCollectorServiceImpl::BnMetricsCollectorServiceImpl(
+    MetricsCollectorServiceTrampoline* metrics_collector_service_trampoline) {
+  metrics_collector_service_trampoline_ = metrics_collector_service_trampoline;
+}
+
+void BnMetricsCollectorServiceImpl::Run() {
+  status_t status =
+      defaultServiceManager()->addService(getInterfaceDescriptor(), this);
+  CHECK(status == OK) << "libmetricscollectorservice: failed to add service";
+  binder_watcher_.reset(new ::brillo::BinderWatcher);
+  CHECK(binder_watcher_->Init()) << "Binder FD watcher init failed";
+}
+
+android::binder::Status BnMetricsCollectorServiceImpl::notifyUserCrash() {
+  metrics_collector_service_trampoline_->ProcessUserCrash();
+  return android::binder::Status::ok();
+}

metricsd/metrics_collector_service_impl.h

@@ -0,0 +1,62 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef METRICSD_METRICS_COLLECTOR_SERVICE_IMPL_H_
+#define METRICSD_METRICS_COLLECTOR_SERVICE_IMPL_H_
+
+// metrics_collector binder service implementation.  Constructed by
+// MetricsCollectorServiceTrampoline, which we use to call back into
+// MetricsCollector.  The trampoline isolates us from the -frtti code of
+// metrics_collector / libbrillo.
+
+#include "android/brillo/metrics/BnMetricsCollectorService.h"
+
+#include <memory>
+
+#include <binder/Status.h>
+#include <brillo/binder_watcher.h>
+
+class MetricsCollectorServiceTrampoline;
+
+//#include "metrics_collector_service_trampoline.h"
+
+class BnMetricsCollectorServiceImpl
+    : public android::brillo::metrics::BnMetricsCollectorService {
+ public:
+  // Passed a this pointer from the MetricsCollectorServiceTrampoline
+  // object that constructs us.
+  explicit BnMetricsCollectorServiceImpl(
+      MetricsCollectorServiceTrampoline* metrics_collector_service_trampoline);
+
+  virtual ~BnMetricsCollectorServiceImpl() = default;
+
+  // Starts the binder main loop.
+  void Run();
+
+  // Called by crash_reporter to report a userspace crash event.  We relay
+  // this to MetricsCollector using the trampoline.
+  android::binder::Status notifyUserCrash();
+
+ private:
+  // Trampoline object that constructs us, we use this to call MetricsCollector
+  // methods via the trampoline.
+  MetricsCollectorServiceTrampoline* metrics_collector_service_trampoline_;
+
+  // BinderWatcher object we construct for handling Binder traffic
+  std::unique_ptr<brillo::BinderWatcher> binder_watcher_;
+};
+
+#endif  // METRICSD_METRICS_COLLECTOR_SERVICE_IMPL_H_

metricsd/metrics_collector_service_trampoline.cc

@@ -0,0 +1,34 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "metrics_collector_service_trampoline.h"
+#include "metrics_collector.h"
+#include "metrics_collector_service_impl.h"
+
+MetricsCollectorServiceTrampoline::MetricsCollectorServiceTrampoline(
+    MetricsCollector* metrics_collector) {
+  metrics_collector_ = metrics_collector;
+}
+
+void MetricsCollectorServiceTrampoline::Run() {
+  // Start metricscollectorservice binder service
+  metrics_collector_service.reset(new BnMetricsCollectorServiceImpl(this));
+  metrics_collector_service->Run();
+}
+
+void MetricsCollectorServiceTrampoline::ProcessUserCrash() {
+  metrics_collector_->ProcessUserCrash();
+}

metricsd/metrics_collector_service_trampoline.h

@@ -0,0 +1,57 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef METRICSD_METRICS_COLLECTOR_SERVICE_TRAMPOLINE_H_
+#define METRICSD_METRICS_COLLECTOR_SERVICE_TRAMPOLINE_H_
+
+// Trampoline between the -fno-rtti compile of libmetricsservice and the
+// -frtti compile of metrics_collector.  MetricsCollectorServiceTrampoline
+// is called from MetricsCollector to run the IMetricsCollectorService
+// server, and acts as a go-between for calls from server back to
+// MetricsCollector.
+
+#include <memory>
+
+#include "metrics_collector_service_impl.h"
+
+// Forward declaration of MetricsCollector.  Don't include the header file
+// for the class here, as it pulls in -frtti stuff.
+class MetricsCollector;
+
+class MetricsCollectorServiceTrampoline {
+ public:
+  // Constructor take a this pointer from the MetricsCollector class that
+  // constructs these objects.
+  explicit MetricsCollectorServiceTrampoline(
+      MetricsCollector* metrics_collector);
+
+  // Initialize and run the IMetricsCollectorService
+  void Run();
+
+  // Called from IMetricsCollectorService to trampoline into the
+  // MetricsCollector method of the same name.
+  void ProcessUserCrash();
+
+ private:
+  // The MetricsCollector object that constructs us, for which we act as
+  // the go-between for MetricsCollectorServiceImpl use.
+  MetricsCollector* metrics_collector_;
+
+  // The IMetricsCollectorService implementation we construct.
+  std::unique_ptr<BnMetricsCollectorServiceImpl> metrics_collector_service;
+};
+
+#endif  // METRICSD_METRICS_COLLECTOR_SERVICE_TRAMPOLINE_H_

metricsd/metrics_collector_test.cc

@@ -115,37 +115,6 @@ class MetricsCollectorTest : public testing::Test {
   StrictMock<MetricsLibraryMock> metrics_lib_;
 };
 
-TEST_F(MetricsCollectorTest, MessageFilter) {
-  // Ignore calls to SendToUMA.
-  EXPECT_CALL(metrics_lib_, SendToUMA(_, _, _, _, _)).Times(AnyNumber());
-
-  DBusMessage* msg = dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_CALL);
-  DBusHandlerResult res =
-      MetricsCollector::MessageFilter(/* connection */ nullptr, msg, &daemon_);
-  EXPECT_EQ(DBUS_HANDLER_RESULT_NOT_YET_HANDLED, res);
-  DeleteDBusMessage(msg);
-
-  vector<string> signal_args;
-  msg = NewDBusSignalString("/",
-                            "org.chromium.CrashReporter",
-                            "UserCrash",
-                            signal_args);
-  res = MetricsCollector::MessageFilter(/* connection */ nullptr, msg, &daemon_);
-  EXPECT_EQ(DBUS_HANDLER_RESULT_HANDLED, res);
-  DeleteDBusMessage(msg);
-
-  signal_args.clear();
-  signal_args.push_back("randomstate");
-  signal_args.push_back("bob");  // arbitrary username
-  msg = NewDBusSignalString("/",
-                            "org.chromium.UnknownService.Manager",
-                            "StateChanged",
-                            signal_args);
-  res = MetricsCollector::MessageFilter(/* connection */ nullptr, msg, &daemon_);
-  EXPECT_EQ(DBUS_HANDLER_RESULT_NOT_YET_HANDLED, res);
-  DeleteDBusMessage(msg);
-}
-
 TEST_F(MetricsCollectorTest, SendSample) {
   ExpectSample("Dummy.Metric", 3);
   daemon_.SendSample("Dummy.Metric", /* sample */ 3,