Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 6e139522 authored by Steve Fung's avatar Steve Fung Committed by ChromeOS Commit Bot
Browse files

crash: Remove glib from crash_reporter 

As part of the minimization effort, refactor crash_reporter code to
not depend directly on glib.

BUG=brillo:87, brillo:88, chromium:435314
TEST=`FEATURES=test emerge-panther libchromeos debugd crash-reporter`
TEST=Enabled crash reports; Browsed to chrome://crash; crash files \
     generated; `FORCE_OFFICIAL=1 SECONDS_SEND_SPREAD=1 crash_sender` \
     /var/log/messages shows crash id, report shows all expected files
TEST=`cbuildbot --remote -p chromiumos/platform2 amd64-generic-full`
CQ-DEPEND=I00331e0bf29195b41cd84d4495ab47738a5a41de
CQ-DEPEND=I9df752d8995773adb56fab34dd97626f3ddf1765

Change-Id: I48b366198a7f89ca55259603cf8470e4d59321bf
Reviewed-on: https://chromium-review.googlesource.com/246441


Reviewed-by: default avatarDan Erat <derat@chromium.org>
Tested-by: default avatarSteve Fung <stevefung@chromium.org>
Commit-Queue: Steve Fung <stevefung@chromium.org>
parent efa91c05

crash_reporter/chrome_collector.cc

+41 −43
Original line number Diff line number Diff line
@@ -4,10 +4,10 @@ 


#include "crash-reporter/chrome_collector.h"



#include <glib.h>

#include <pcrecpp.h>

#include <stdint.h>



#include <map>

#include <string>

#include <vector>
@@ -15,6 +15,7 @@ 
#include <base/logging.h>

#include <base/strings/string_number_conversions.h>

#include <base/strings/string_util.h>

#include <chromeos/data_encoding.h>

#include <chromeos/dbus/dbus.h>

#include <chromeos/dbus/service_constants.h>

#include <chromeos/process.h>
@@ -51,39 +52,20 @@ bool GetDelimitedString(const std::string &str, char ch, size_t offset, 


// Gets the GPU's error state from debugd and writes it to |error_state_path|.

// Returns true on success.

bool GetDriErrorState(const FilePath &error_state_path) {

  chromeos::dbus::BusConnection dbus = chromeos::dbus::GetSystemBusConnection();

  if (!dbus.HasConnection()) {

    LOG(ERROR) << "Error connecting to system D-Bus";

    return false;

  }

bool GetDriErrorState(const FilePath &error_state_path,

                      org::chromium::debugdProxy *proxy) {

  chromeos::ErrorPtr error;

  std::string error_state_str;



  chromeos::dbus::Proxy proxy(dbus,

                              debugd::kDebugdServiceName,

                              debugd::kDebugdServicePath,

                              debugd::kDebugdInterface);

  if (!proxy) {

    LOG(ERROR) << "Error creating D-Bus proxy to interface "

               << "'" << debugd::kDebugdServiceName << "'";

    return false;

  }

  proxy->GetLog("i915_error_state", &error_state_str, &error);



  chromeos::glib::ScopedError error;

  gchar *error_state = nullptr;

  if (!dbus_g_proxy_call(proxy.gproxy(), debugd::kGetLog,

                         &chromeos::Resetter(&error).lvalue(),

                         G_TYPE_STRING, "i915_error_state", G_TYPE_INVALID,

                         G_TYPE_STRING, &error_state, G_TYPE_INVALID)) {

    LOG(ERROR) << "Error performing D-Bus proxy call "

               << "'" << debugd::kGetLog << "'"

               << ": " << (error ? error->message : "");

    g_free(error_state);

  if (error) {

    LOG(ERROR) << "Error calling D-Bus proxy call to interface "

               << "'" << proxy->GetObjectPath().value() << "':"

               << error->GetMessage();

    return false;

  }



  std::string error_state_str(error_state);

  g_free(error_state);



  if (error_state_str == "<empty>")

    return false;
@@ -94,17 +76,23 @@ bool GetDriErrorState(const FilePath &error_state_path) { 
    return false;

  }



  gsize len;

  guchar *decoded_error_state =

      g_base64_decode(error_state_str.c_str() + kBase64HeaderLength, &len);

  std::string decoded_error_state;



  int written =

      base::WriteFile(error_state_path,

                      reinterpret_cast<const char *>(decoded_error_state), len);

  g_free(decoded_error_state);

  if (!chromeos::data_encoding::Base64Decode(

      error_state_str.c_str() + kBase64HeaderLength,

      &decoded_error_state)) {

    LOG(ERROR) << "Could not decode i915_error_state";

    return false;

  }



  if (written < 0 || (gsize)written != len) {

    LOG(ERROR) << "Could not write file " << error_state_path.value();

  int written = base::WriteFile(error_state_path,

                                decoded_error_state.c_str(),

                                decoded_error_state.length());

  if (written < 0 ||

      static_cast<size_t>(written) != decoded_error_state.length()) {

    LOG(ERROR) << "Could not write file " << error_state_path.value()

               << " Written: " << written << " Len: "

               << decoded_error_state.length();

    base::DeleteFile(error_state_path, false);

    return false;

  }
@@ -208,6 +196,13 @@ bool ChromeCollector::HandleCrash(const FilePath &file_path, 
  return true;

}



void ChromeCollector::SetUpDBus() {

  CrashCollector::SetUpDBus();



  debugd_proxy_.reset(

      new org::chromium::debugdProxy(bus_, debugd::kDebugdServiceName));

}



bool ChromeCollector::ParseCrashLog(const std::string &data,

                                    const FilePath &dir,

                                    const FilePath &minidump,
@@ -325,11 +320,14 @@ std::map<std::string, base::FilePath> ChromeCollector::GetAdditionalLogs( 
      base::DeleteFile(chrome_log_path, false /* recursive */);

  }



  // Now get the GPU state from debugd.

  // For unit testing, debugd_proxy_ isn't initialized, so skip attempting to

  // get the GPU error state from debugd.

  if (debugd_proxy_) {

    const FilePath dri_error_state_path =

        GetCrashPath(dir, basename, kGpuStateFilename);

  if (GetDriErrorState(dri_error_state_path))

    if (GetDriErrorState(dri_error_state_path, debugd_proxy_.get()))

      logs[kGpuStateFilename] = dri_error_state_path;

  }



  return logs;

}

crash_reporter/chrome_collector.h

+8 −0
Original line number Diff line number Diff line
@@ -5,6 +5,7 @@ 
#ifndef CRASH_REPORTER_CHROME_COLLECTOR_H_

#define CRASH_REPORTER_CHROME_COLLECTOR_H_



#include <map>

#include <string>



#include <base/files/file_path.h>
@@ -12,6 +13,7 @@ 
#include <gtest/gtest_prod.h>  // for FRIEND_TEST



#include "crash-reporter/crash_collector.h"

#include "debugd/dbus-proxies.h"



class SystemLogging;
@@ -30,6 +32,9 @@ class ChromeCollector : public CrashCollector { 
                   const std::string &uid_string,

                   const std::string &exe_name);



 protected:

  void SetUpDBus() override;



 private:

  friend class ChromeCollectorTest;

  FRIEND_TEST(ChromeCollectorTest, GoodValues);
@@ -58,6 +63,9 @@ class ChromeCollector : public CrashCollector { 


  FILE *output_file_ptr_;



  // D-Bus proxy for debugd interface.  Unset in unit tests.

  std::unique_ptr<org::chromium::debugdProxy> debugd_proxy_;



  DISALLOW_COPY_AND_ASSIGN(ChromeCollector);

};

crash_reporter/chrome_collector_test.cc

+9 −1
Original line number Diff line number Diff line
@@ -10,6 +10,7 @@ 
#include <base/files/file_util.h>

#include <base/files/scoped_temp_dir.h>

#include <chromeos/syslog_logging.h>

#include <gmock/gmock.h>

#include <gtest/gtest.h>



using base::FilePath;
@@ -40,6 +41,11 @@ bool IsMetrics() { 


}  // namespace



class ChromeCollectorMock : public ChromeCollector {

 public:

  MOCK_METHOD0(SetUpDBus, void());

};



class ChromeCollectorTest : public ::testing::Test {

 protected:

  void ExpectFileEquals(const char *golden,
@@ -49,10 +55,12 @@ class ChromeCollectorTest : public ::testing::Test { 
    EXPECT_EQ(golden, contents);

  }



  ChromeCollector collector_;

  ChromeCollectorMock collector_;



 private:

  void SetUp() override {

    EXPECT_CALL(collector_, SetUpDBus()).WillRepeatedly(testing::Return());



    collector_.Initialize(CountCrash, IsMetrics);

    chromeos::ClearLog();

  }

crash_reporter/crash-reporter.gyp

+22 −0
Original line number Diff line number Diff line
@@ -39,6 +39,28 @@ 
        'unclean_shutdown_collector.cc',

        'user_collector.cc',

      ],

      'actions': [

        {

          'action_name': 'generate-session-manager-proxies',

          'variables': {

            'proxy_output_file': 'include/session_manager/dbus-proxies.h'

          },

          'sources': [

            '../login_manager/dbus_bindings/org.chromium.SessionManager.xml',

          ],

          'includes': ['../common-mk/generate-dbus-proxies.gypi'],

        },

        {

          'action_name': 'generate-debugd-proxies',

          'variables': {

            'proxy_output_file': 'include/debugd/dbus-proxies.h'

          },

          'sources': [

            '../debugd/share/org.chromium.debugd.xml',

          ],

          'includes': ['../common-mk/generate-dbus-proxies.gypi'],

        },

      ],

    },

    {

      'target_name': 'crash_reporter',

crash_reporter/crash_collector.cc

+32 −53
Original line number Diff line number Diff line
@@ -18,9 +18,6 @@ 
#include <utility>

#include <vector>



#include <dbus/dbus-glib-lowlevel.h>

#include <glib.h>



#include <base/files/file_util.h>

#include <base/logging.h>

#include <base/posix/eintr_wrapper.h>
@@ -92,6 +89,8 @@ CrashCollector::CrashCollector() 
}



CrashCollector::~CrashCollector() {

  if (bus_)

    bus_->ShutdownAndBlock();

}



void CrashCollector::Initialize(
@@ -102,6 +101,21 @@ void CrashCollector::Initialize( 


  count_crash_function_ = count_crash_function;

  is_feedback_allowed_function_ = is_feedback_allowed_function;



  SetUpDBus();

}



void CrashCollector::SetUpDBus() {

  dbus::Bus::Options options;

  options.bus_type = dbus::Bus::SYSTEM;



  bus_ = new dbus::Bus(options);

  CHECK(bus_->Connect());



  session_manager_proxy_.reset(

      new org::chromium::SessionManagerInterfaceProxy(

          bus_,

          login_manager::kSessionManagerInterface));

}



int CrashCollector::WriteNewFile(const FilePath &filename,
@@ -154,49 +168,19 @@ FilePath CrashCollector::GetCrashPath(const FilePath &crash_directory, 
                                             extension.c_str()));

}



namespace {



const char *GetGErrorMessage(const GError *error) {

  if (!error)

    return "Unknown error.";

  return error->message;

}



}



GHashTable *CrashCollector::GetActiveUserSessions() {

  GHashTable *active_sessions = nullptr;

bool CrashCollector::GetActiveUserSessions(

    std::map<std::string, std::string> *sessions) {

  chromeos::ErrorPtr error;

  session_manager_proxy_->RetrieveActiveSessions(sessions, &error);



  chromeos::dbus::BusConnection dbus = chromeos::dbus::GetSystemBusConnection();

  if (!dbus.HasConnection()) {

    LOG(ERROR) << "Error connecting to system D-Bus";

    return active_sessions;

  }

  chromeos::dbus::Proxy proxy(dbus,

                              login_manager::kSessionManagerServiceName,

                              login_manager::kSessionManagerServicePath,

                              login_manager::kSessionManagerInterface);

  if (!proxy) {

    LOG(ERROR) << "Error creating D-Bus proxy to interface "

               << "'" << login_manager::kSessionManagerServiceName << "'";

    return active_sessions;

  }



  // Request all the active sessions.

  GError *gerror = nullptr;

  if (!dbus_g_proxy_call(proxy.gproxy(),

                         login_manager::kSessionManagerRetrieveActiveSessions,

                         &gerror, G_TYPE_INVALID,

                         DBUS_TYPE_G_STRING_STRING_HASHTABLE, &active_sessions,

                         G_TYPE_INVALID)) {

    LOG(ERROR) << "Error performing D-Bus proxy call "

               << "'"

               << login_manager::kSessionManagerRetrieveActiveSessions << "'"

               << ": " << GetGErrorMessage(gerror);

    return active_sessions;

  if (error) {

    LOG(ERROR) << "Error calling D-Bus proxy call to interface "

               << "'" << session_manager_proxy_->GetObjectPath().value() << "':"

               << error->GetMessage();

    return false;

  }



  return active_sessions;

  return true;

}



FilePath CrashCollector::GetUserCrashPath() {
@@ -204,19 +188,14 @@ FilePath CrashCollector::GetUserCrashPath() { 
  // Ask the session manager for the active ones, then just run with the

  // first result we get back.

  FilePath user_path = FilePath(kFallbackUserCrashPath);

  GHashTable *active_sessions = GetActiveUserSessions();

  if (!active_sessions)

  std::map<std::string, std::string> active_sessions;

  if (!GetActiveUserSessions(&active_sessions)) {

    LOG(ERROR) << "Could not get active user sessions, using default.";

    return user_path;



  GList *list = g_hash_table_get_values(active_sessions);

  if (list) {

    const char *salted_path = static_cast<const char *>(list->data);

    user_path = chromeos::cryptohome::home::GetHashedUserPath(salted_path)

        .Append("crash");

    g_list_free(list);

  }



  g_hash_table_destroy(active_sessions);

  user_path = chromeos::cryptohome::home::GetHashedUserPath(

      active_sessions.begin()->second).Append("crash");



  return user_path;

}