New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 705051 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

CrOS: debugd: migrate from dbus-c++ to chrome dbus API

Project Member Reported by benchan@chromium.org, Mar 24 2017

Issue description

Migrate debugd to use Chrome DBus API and then get rid of its dependency on dbus-c++.
 
Cc: vapier@chromium.org derat@chromium.org
Status: Started (was: Assigned)
Cc: jorgelo@chromium.org

Comment 3 by derat@chromium.org, Mar 24 2017

Cc: ejcaruso@chromium.org
Eric is looking at this too. Are you and he aware of each other's work?
yep, ejcaruso and I synced. He'll take care of the main debugd DBus interface, and I'll handle the proxy code in those debugd helpers.
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/d85dd56bc5f2b11df70be319202d836a49196845

commit d85dd56bc5f2b11df70be319202d836a49196845
Author: Ben Chan <benchan@chromium.org>
Date: Fri Mar 31 04:21:53 2017

debugd: add helper class for talking to system services over DBus

This CL adds a helper class, SystemServiceProxy, for talking to system
services over DBus, which is used as a building block for migrating the
DBus proxy code in various debugd helpers to use the DBus API from
libchrome instead of dbus-c++.

BUG= chromium:705051 
TEST=Run unit tests.

Change-Id: I63015bfa820432db7e4a38fed5825577324bf2f8
Reviewed-on: https://chromium-review.googlesource.com/461209
Commit-Ready: Ben Chan <benchan@chromium.org>
Tested-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[add] https://crrev.com/d85dd56bc5f2b11df70be319202d836a49196845/debugd/src/helpers/system_service_proxy.h
[modify] https://crrev.com/d85dd56bc5f2b11df70be319202d836a49196845/debugd/debugd.gyp
[add] https://crrev.com/d85dd56bc5f2b11df70be319202d836a49196845/debugd/src/helpers/system_service_proxy.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/cb2a0429b7e56a452def00a32045b54dc5d0f875

commit cb2a0429b7e56a452def00a32045b54dc5d0f875
Author: Ben Chan <benchan@chromium.org>
Date: Fri Mar 31 04:21:53 2017

debugd: add helper class for talking to shill over DBus

This CL adds a helper class, ShillProxy, for talking to system services
over DBus, which is used as a building block for migrating the DBus
proxy code in various debugd helpers (e.g. network_status, netif) to use
the DBus API from libchrome instead of dbus-c++.

BUG= chromium:705051 
TEST=Run unit tests.

Change-Id: If602a50834fb61c71a01708e4d87fae2fc99c282
Reviewed-on: https://chromium-review.googlesource.com/461210
Commit-Ready: Ben Chan <benchan@chromium.org>
Tested-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[add] https://crrev.com/cb2a0429b7e56a452def00a32045b54dc5d0f875/debugd/src/helpers/shill_proxy.cc
[modify] https://crrev.com/cb2a0429b7e56a452def00a32045b54dc5d0f875/debugd/debugd.gyp
[add] https://crrev.com/cb2a0429b7e56a452def00a32045b54dc5d0f875/debugd/src/helpers/shill_proxy.h

Project Member

Comment 7 by bugdroid1@chromium.org, Mar 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/7a43e59d1c7f6ccae332f3758190554981a4a4ad

commit 7a43e59d1c7f6ccae332f3758190554981a4a4ad
Author: Ben Chan <benchan@chromium.org>
Date: Fri Mar 31 04:21:53 2017

debugd: migrate 'network_status' helper to use Chrome DBus API

This CL migrates the 'network_status' helper to use Chrome DBus API,
instead of dbus-c++, to query device and service information over
shill's DBus interfaces.

BUG= chromium:705051 
TEST=Run platform_DebugDaemonGetNetworkStatus autotest.
TEST=Examine the 'network-status' field reported by chrome://system.
TEST=Examine the output from the following command:

  dbus-send --system --print-reply --dest=org.chromium.debugd \
      /org/chromium/debugd org.chromium.debugd.GetNetworkStatus

Change-Id: Ia4dec1294604119554674140edcf57e77ee3a98c
Reviewed-on: https://chromium-review.googlesource.com/461211
Commit-Ready: Ben Chan <benchan@chromium.org>
Tested-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/7a43e59d1c7f6ccae332f3758190554981a4a4ad/debugd/debugd.gyp
[modify] https://crrev.com/7a43e59d1c7f6ccae332f3758190554981a4a4ad/debugd/src/helpers/network_status.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Mar 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/c721508071fc33d4ce5b5b34df1e79d139af5b6e

commit c721508071fc33d4ce5b5b34df1e79d139af5b6e
Author: Ben Chan <benchan@chromium.org>
Date: Fri Mar 31 04:21:53 2017

debugd: migrate 'netif' helper to use Chrome DBus API

This CL migrates the 'netif' helper to use Chrome DBus API,
instead of dbus-c++, to query device and service information over
shill's DBus interfaces.

BUG= chromium:705051 
TEST=Examine the output from the following command:

  dbus-send --system --print-reply --dest=org.chromium.debugd \
      /org/chromium/debugd org.chromium.debugd.GetInterfaces

Change-Id: If8714fd8e99b44c267cab123e7b861bc10e82ec0
Reviewed-on: https://chromium-review.googlesource.com/461212
Commit-Ready: Ben Chan <benchan@chromium.org>
Tested-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/c721508071fc33d4ce5b5b34df1e79d139af5b6e/debugd/debugd.gyp
[modify] https://crrev.com/c721508071fc33d4ce5b5b34df1e79d139af5b6e/debugd/src/helpers/netif.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Mar 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/2a44bf9e668eff5db48efae3f198419c6021b98f

commit 2a44bf9e668eff5db48efae3f198419c6021b98f
Author: Ben Chan <benchan@chromium.org>
Date: Fri Mar 31 04:21:53 2017

debugd: migrate 'wimax_status' helper to use Chrome DBus API

This CL migrates the 'wimax_status' helper to use Chrome DBus API,
instead of dbus-c++, to query device and service information over
shill's DBus interfaces.

BUG= chromium:705051 
TEST=Examine the 'wimax-status' field reported by chrome://system.
TEST=Examine the output from the following command:

  dbus-send --system --print-reply --dest=org.chromium.debugd \
      /org/chromium/debugd org.chromium.debugd.GetWiMaxStatus

Change-Id: Ia60eea2cabb887b6d7bb9c6b4d104a99ad742d50
Reviewed-on: https://chromium-review.googlesource.com/461213
Commit-Ready: Ben Chan <benchan@chromium.org>
Tested-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/2a44bf9e668eff5db48efae3f198419c6021b98f/debugd/src/helpers/wimax_status.cc
[modify] https://crrev.com/2a44bf9e668eff5db48efae3f198419c6021b98f/debugd/debugd.gyp

Project Member

Comment 10 by bugdroid1@chromium.org, Mar 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/9d80f9ff33b1a705c0447c9115c8f1422c7a0fb5

commit 9d80f9ff33b1a705c0447c9115c8f1422c7a0fb5
Author: Ben Chan <benchan@chromium.org>
Date: Fri Mar 31 04:21:54 2017

debugd: migrate 'modem_status' helper to use Chrome DBus API

This CL migrates the 'modem_status' helper to use Chrome DBus API,
instead of dbus-c++, to query device and service information over
shill's DBus interfaces.

BUG= chromium:705051 
TEST=Run platform_DebugDaemonGetModemStatus autotest.
TEST=Examine the 'modem-status' field reported by chrome://system.
TEST=Examine the output from the following command:

  dbus-send --system --print-reply --dest=org.chromium.debugd \
      /org/chromium/debugd org.chromium.debugd.GetModemStatus

Change-Id: Ie112b49c8e4cd0aaf0dfe49eb591568af05ffa5a
Reviewed-on: https://chromium-review.googlesource.com/461214
Commit-Ready: Ben Chan <benchan@chromium.org>
Tested-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/9d80f9ff33b1a705c0447c9115c8f1422c7a0fb5/debugd/debugd.gyp
[modify] https://crrev.com/9d80f9ff33b1a705c0447c9115c8f1422c7a0fb5/debugd/src/helpers/modem_status.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Mar 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/42aa30f570b5140838e58c00161b048bb032cfc9

commit 42aa30f570b5140838e58c00161b048bb032cfc9
Author: Ben Chan <benchan@chromium.org>
Date: Fri Mar 31 04:21:54 2017

debugd: remove deprecated and unused dbus_utils*

debugd helpers now use the DBus API from libchrome instead of dbus-c++.
The helper functions in dbus_utils are thus no longer needed.

BUG= chromium:705051 
TEST=Run unit tests.

Change-Id: I56a5ae8b5f41ffc607e25b2d1d9adf708a1e7e80
Reviewed-on: https://chromium-review.googlesource.com/461215
Commit-Ready: Ben Chan <benchan@chromium.org>
Tested-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[delete] https://crrev.com/9d80f9ff33b1a705c0447c9115c8f1422c7a0fb5/debugd/src/dbus_utils.cc
[modify] https://crrev.com/42aa30f570b5140838e58c00161b048bb032cfc9/debugd/debugd.gyp
[delete] https://crrev.com/9d80f9ff33b1a705c0447c9115c8f1422c7a0fb5/debugd/src/dbus_utils.h
[delete] https://crrev.com/9d80f9ff33b1a705c0447c9115c8f1422c7a0fb5/debugd/src/dbus_utils_unittest.cc

Comment 12 by kbr@chromium.org, Apr 3 2017

Blockedon: 707986

Comment 13 by kbr@chromium.org, Apr 3 2017

There are dbus-related assertion failures firing on the Linux bots, now filed under  Issue 707986 , but I'm not sure whether these dbus-related changes might be the cause. Please help us triage the problem. Thanks.

(Per discussion on other bug: unrelated; failures are on desktop Linux Chrome tests while the only changes here are to a Chrome OS system daemon.)
Blockedon: -707986
Project Member

Comment 16 by bugdroid1@chromium.org, Apr 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/c93a15c328f511a26400c8fc77b2e5c9f67a5e70

commit c93a15c328f511a26400c8fc77b2e5c9f67a5e70
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Wed Apr 26 01:40:03 2017

debugd: remove unused DBus::Errors

To prepare for replacing dbus-c++ constructs with brillo dbus
bindings, this removes DBus::Error usage which is syntactically
dead (i.e. tools that take one as a parameter but never use it).
This simplifies the interfaces to a number of tools and breaks
the dependency on dbus-c++ constructs entirely for some of them.

BUG= chromium:705051 
TEST=emerge, unit tests

Change-Id: Ib3dd26992f6f97173d38008308dad4710456555e
Reviewed-on: https://chromium-review.googlesource.com/486202
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>

[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/debug_daemon.cc
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/modem_status_tool.cc
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/swap_tool.h
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/debug_mode_tool.h
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/network_status_tool.h
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/icmp_tool.cc
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/debug_logs_tool.h
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/systrace_tool.cc
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/crash_sender_tool.h
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/log_tool.cc
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/log_tool.h
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/memory_tool.cc
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/battery_tool.cc
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/cups_tool.cc
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/storage_tool.h
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/tracepath_tool.cc
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/oom_adj_tool.cc
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/wimax_status_tool.cc
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/netif_tool.h
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/icmp_tool.h
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/route_tool.h
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/debug_logs_tool.cc
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/swap_tool.cc
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/modem_status_tool.h
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/crash_sender_tool.cc
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/tracepath_tool.h
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/netif_tool.cc
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/route_tool.cc
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/battery_tool.h
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/memory_tool.h
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/example_tool.cc
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/example_tool.h
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/network_status_tool.cc
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/debug_mode_tool.cc
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/cups_tool.h
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/systrace_tool.h
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/storage_tool.cc
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/wimax_status_tool.h
[modify] https://crrev.com/c93a15c328f511a26400c8fc77b2e5c9f67a5e70/debugd/src/oom_adj_tool.h

Project Member

Comment 17 by bugdroid1@chromium.org, Apr 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/75eda0878aa34bcebb234bea1eaab9a1d8cf9f22

commit 75eda0878aa34bcebb234bea1eaab9a1d8cf9f22
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Thu Apr 27 02:46:57 2017

debugd: simplify DevFeaturesTool

base::Bind is the preferred method of creating and storing
callbacks, so replace uses of std::bind with that. This also
allows us to more easily use partial application, so we can
pass the DBus::Error pointer it at the callsite to make it
clearer that the error is being set each time a query function
is called.

This also moves some functions which were in the public API but
unused by anything outside of DevFeaturesTool into an anonymous
namespace inside of the .cc file, since they did not touch any
state and were marked const. This allows us to avoid binding
"this" in each of the query functions.

BUG= chromium:705051 
TEST=emerge, unit tests

Change-Id: I5c7a333816ea4caa6d90eea0fc028fd7ce3b6d35
Reviewed-on: https://chromium-review.googlesource.com/487071
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>

[modify] https://crrev.com/75eda0878aa34bcebb234bea1eaab9a1d8cf9f22/debugd/src/dev_features_tool.cc
[modify] https://crrev.com/75eda0878aa34bcebb234bea1eaab9a1d8cf9f22/debugd/src/dev_features_tool.h

Project Member

Comment 18 by bugdroid1@chromium.org, Apr 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/8fe49c7e6914fffc7a6737aa7e3d279b1055d025

commit 8fe49c7e6914fffc7a6737aa7e3d279b1055d025
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Fri Apr 28 21:16:06 2017

debugd: prefer bool(R*, Error*) over R(Error*)

Tool functions should return a boolean success value and use that
to signal whether an error occurred, rather than checking the error
directly. This allows us to easily switch to other implementations
of dbus errors without having to rely on the implementation details
of the error object to figure out if something went wrong.

BUG= chromium:705051 
TEST=emerge, unit tests

Change-Id: I2a694d22c84c91d07039e58d9c17a4731035a1bd
Reviewed-on: https://chromium-review.googlesource.com/487072
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>

[modify] https://crrev.com/8fe49c7e6914fffc7a6737aa7e3d279b1055d025/debugd/src/dev_features_tool.cc
[modify] https://crrev.com/8fe49c7e6914fffc7a6737aa7e3d279b1055d025/debugd/src/sysrq_tool.h
[modify] https://crrev.com/8fe49c7e6914fffc7a6737aa7e3d279b1055d025/debugd/src/subprocess_tool.cc
[modify] https://crrev.com/8fe49c7e6914fffc7a6737aa7e3d279b1055d025/debugd/src/sysrq_tool.cc
[modify] https://crrev.com/8fe49c7e6914fffc7a6737aa7e3d279b1055d025/debugd/src/debug_daemon.cc
[modify] https://crrev.com/8fe49c7e6914fffc7a6737aa7e3d279b1055d025/debugd/src/subprocess_tool.h
[modify] https://crrev.com/8fe49c7e6914fffc7a6737aa7e3d279b1055d025/debugd/src/perf_tool.h
[modify] https://crrev.com/8fe49c7e6914fffc7a6737aa7e3d279b1055d025/debugd/src/ping_tool.h
[modify] https://crrev.com/8fe49c7e6914fffc7a6737aa7e3d279b1055d025/debugd/src/packet_capture_tool.cc
[modify] https://crrev.com/8fe49c7e6914fffc7a6737aa7e3d279b1055d025/debugd/src/ping_tool.cc
[modify] https://crrev.com/8fe49c7e6914fffc7a6737aa7e3d279b1055d025/debugd/src/perf_tool.cc
[modify] https://crrev.com/8fe49c7e6914fffc7a6737aa7e3d279b1055d025/debugd/src/packet_capture_tool.h
[modify] https://crrev.com/8fe49c7e6914fffc7a6737aa7e3d279b1055d025/debugd/src/dev_features_tool.h

Project Member

Comment 19 by bugdroid1@chromium.org, Apr 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/96d03d3ff28f3ec30364648cb57216ff309cfdf6

commit 96d03d3ff28f3ec30364648cb57216ff309cfdf6
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Fri Apr 28 21:16:06 2017

debugd: fix using declarations

Some of these using declarations are dead, some are used
inconsistently, and some are used when there is only one
relevant use in the .cc file. This removes them where they
don't save space, and makes their use more consistent
otherwise.

BUG= chromium:705051 
TEST=emerge, unit tests

Change-Id: I57e8528c692c2777f54dc375ca4cb5bda4d21235
Reviewed-on: https://chromium-review.googlesource.com/487323
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>

[modify] https://crrev.com/96d03d3ff28f3ec30364648cb57216ff309cfdf6/debugd/src/modem_status_tool_test.cc
[modify] https://crrev.com/96d03d3ff28f3ec30364648cb57216ff309cfdf6/debugd/src/anonymizer_tool.cc
[modify] https://crrev.com/96d03d3ff28f3ec30364648cb57216ff309cfdf6/debugd/src/process_with_id_test.cc
[modify] https://crrev.com/96d03d3ff28f3ec30364648cb57216ff309cfdf6/debugd/src/modem_status_tool.cc
[modify] https://crrev.com/96d03d3ff28f3ec30364648cb57216ff309cfdf6/debugd/src/log_tool.cc
[modify] https://crrev.com/96d03d3ff28f3ec30364648cb57216ff309cfdf6/debugd/src/memory_tool.cc
[modify] https://crrev.com/96d03d3ff28f3ec30364648cb57216ff309cfdf6/debugd/src/battery_tool.cc
[modify] https://crrev.com/96d03d3ff28f3ec30364648cb57216ff309cfdf6/debugd/src/packet_capture_tool.cc
[modify] https://crrev.com/96d03d3ff28f3ec30364648cb57216ff309cfdf6/debugd/src/icmp_tool.cc
[modify] https://crrev.com/96d03d3ff28f3ec30364648cb57216ff309cfdf6/debugd/src/storage_tool.cc
[modify] https://crrev.com/96d03d3ff28f3ec30364648cb57216ff309cfdf6/debugd/src/wimax_status_tool.cc
[modify] https://crrev.com/96d03d3ff28f3ec30364648cb57216ff309cfdf6/debugd/src/dev_mode_no_owner_restriction.cc
[modify] https://crrev.com/96d03d3ff28f3ec30364648cb57216ff309cfdf6/debugd/src/perf_tool.cc

Project Member

Comment 20 by bugdroid1@chromium.org, May 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/7e432462d2a35ffaf1493a1632f0375be4029480

commit 7e432462d2a35ffaf1493a1632f0375be4029480
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Wed May 03 03:24:20 2017

debugd: simplify RestrictedToolWrapper

The CallToolFunction interface forced users to do strange things
such as create lambdas for simple method calls, and these lambdas
often involved capture-by-reference, which is not allowed in the
style guide. Using base::Bind to generate callbacks is also hard
because template argument deduction doesn't work here.

Instead, just get rid of this function and have callers use
GetTool directly. This still forces callers to do the restriction
check, but also makes it easier to return values from methods
without having to go through hoops such as closing over a stack
variable by reference and assigning it in a lambda. It also makes
the implementation of QueryDevFeatures much simpler.

BUG= chromium:705051 
TEST=unit tests, interact with DevFeaturesTool using dbus-send

Change-Id: I593d5081645c82dc6476586953c7aca249498fc9
Reviewed-on: https://chromium-review.googlesource.com/487873
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>

[modify] https://crrev.com/7e432462d2a35ffaf1493a1632f0375be4029480/debugd/src/restricted_tool_wrapper.h
[modify] https://crrev.com/7e432462d2a35ffaf1493a1632f0375be4029480/debugd/src/debug_daemon.cc

Project Member

Comment 21 by bugdroid1@chromium.org, May 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/0488139788fcdf4b1279756af2ce6b183fc4d913

commit 0488139788fcdf4b1279756af2ce6b183fc4d913
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Fri May 05 21:42:49 2017

platform_DebugDaemonGetPerfData: remove introspection

Unlike dbus-c++, chromeos-dbus-bindings does not expose the
introspectable interface, so dbus-python has a hard time figuring
out what the method signature of GetPerfData is by itself. This
just manually passes the signature so that we don't have to use
introspection at all.

BUG= chromium:705051 
TEST=run test on cyan with refactored debugd, verify it passes

Change-Id: Iba314d1359cca55101e4815fa9c88145b68abaa0
Reviewed-on: https://chromium-review.googlesource.com/495412
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Simon Que <sque@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>

[modify] https://crrev.com/0488139788fcdf4b1279756af2ce6b183fc4d913/client/site_tests/platform_DebugDaemonGetPerfData/platform_DebugDaemonGetPerfData.py

Project Member

Comment 22 by bugdroid1@chromium.org, May 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/63c9743ae0dc9bed2799919f0e68d4394798d327

commit 63c9743ae0dc9bed2799919f0e68d4394798d327
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Mon May 08 19:40:49 2017

debugd: rename DebugDaemon to DebugdDBusAdaptor

In a future CL, the DebugDaemon class will contain the DBus adaptor
implementation, and the actual daemon will be a small helper class
in the main file that registers it. This renames the class to better
fit the purpose of this class in the post-dbus-c++ world.

BUG= chromium:705051 
TEST=emerge

Change-Id: Ia51e55cc4dc12f6804c514926d0b751479a7f4c7
Reviewed-on: https://chromium-review.googlesource.com/489404
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>

[rename] https://crrev.com/63c9743ae0dc9bed2799919f0e68d4394798d327/debugd/src/debugd_dbus_adaptor.h
[modify] https://crrev.com/63c9743ae0dc9bed2799919f0e68d4394798d327/debugd/debugd.gyp
[rename] https://crrev.com/63c9743ae0dc9bed2799919f0e68d4394798d327/debugd/src/debugd_dbus_adaptor.cc
[modify] https://crrev.com/63c9743ae0dc9bed2799919f0e68d4394798d327/debugd/src/main.cc

Project Member

Comment 23 by bugdroid1@chromium.org, May 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/8e0b00864248bf78dcb18b6a96e20c29209379d7

commit 8e0b00864248bf78dcb18b6a96e20c29209379d7
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Mon May 08 19:40:50 2017

debugd: disable exceptions

Now that it uses chromeos-dbus-bindings, we don't need to allow
exceptions anymore.

BUG= chromium:705051 
TEST=emerge

Change-Id: I2e3feb02adb86c0e6da4e13ff94acef8185e3a9e
Reviewed-on: https://chromium-review.googlesource.com/490711
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/8e0b00864248bf78dcb18b6a96e20c29209379d7/debugd/debugd.gyp

Project Member

Comment 24 by bugdroid1@chromium.org, May 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/cc7106cb69839ad1f9ed4b9635f454353ee01f14

commit cc7106cb69839ad1f9ed4b9635f454353ee01f14
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Mon May 08 19:40:49 2017

debugd: switch from dbus-c++ to chromeos-dbus-bindings

This allows us to remove a major user of dbus-c++, which does
not play nicely with chrome code, as it uses exceptions. It
also uses non-const references for multiple return values and
errors which is strictly disallowed. This also allows us to use
the libchrome message loop.

Most of this is a replacement of types:
* DBus::Connection -> dbus::Bus
* DBus::Error -> brillo::ErrorPtr
* DBus::FileDescriptor -> dbus::FileDescriptor
* std::map<std::string, DBus::Variant> -> brillo::VariantDictionary
as well as use of libchrome dbus objects where debugd calls out
to other D-Bus services.

CQ-DEPEND=CL:495412
BUG= chromium:705051 
TEST=emerge, unit tests, autotests, deploy and use from crosh

Change-Id: I5a7d34be6eec5f88949551dde2f02110b8c90487
Reviewed-on: https://chromium-review.googlesource.com/490830
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>

[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/process_with_output.h
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/dev_features_tool.cc
[add] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/variant_utils.cc
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/wifi_debug_tool.h
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/route_tool.cc
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/subprocess_tool.h
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/swap_tool.h
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/debug_mode_tool.h
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/packet_capture_tool.cc
[add] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/error_utils.h
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/session_manager_proxy.h
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/subprocess_tool.cc
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/perf_tool.cc
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/subprocess_tool_test.cc
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/dev_mode_no_owner_restriction.cc
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/systrace_tool.cc
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/sysrq_tool.cc
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/dev_mode_no_owner_restriction.h
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/debugd_dbus_adaptor.h
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/log_tool.cc
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/log_tool.h
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/memory_tool.cc
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/debug_logs_tool.h
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/tracepath_tool.cc
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/process_with_output.cc
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/route_tool.h
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/dev_features_tool.h
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/swap_tool.cc
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/debugd_dbus_adaptor.cc
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/debugd.gyp
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/storage_tool.h
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/tracepath_tool.h
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/ping_tool.h
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/ping_tool.cc
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/dbus_bindings/org.chromium.debugd.xml
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/restricted_tool_wrapper.h
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/main.cc
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/packet_capture_tool.h
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/dbus_bindings/dbus-service-config.json
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/debug_logs_tool.cc
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/memory_tool.h
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/sysrq_tool.h
[add] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/variant_utils.h
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/perf_tool.h
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/session_manager_proxy.cc
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/debug_mode_tool.cc
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/systrace_tool.h
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/wifi_debug_tool.cc
[modify] https://crrev.com/cc7106cb69839ad1f9ed4b9635f454353ee01f14/debugd/src/storage_tool.cc

Status: Fixed (was: Started)
Labels: VerifyIn-61
Project Member

Comment 27 by bugdroid1@chromium.org, Sep 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/acc0a45fdd024ab2f262e8407ca6fbe771d766a0

commit acc0a45fdd024ab2f262e8407ca6fbe771d766a0
Author: Mike Frysinger <vapier@chromium.org>
Date: Thu Sep 21 08:08:04 2017

debugd: update dbus bindings docs

We migrated away from dbus-c++, so update the docs.

BUG= chromium:705051 
TEST=read the doc

Change-Id: Ic21bcacfb068621fd1f8aa14eb73343d189c6f93
Reviewed-on: https://chromium-review.googlesource.com/676169
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>

[modify] https://crrev.com/acc0a45fdd024ab2f262e8407ca6fbe771d766a0/debugd/doc/hacking.md
[modify] https://crrev.com/acc0a45fdd024ab2f262e8407ca6fbe771d766a0/debugd/doc/implementation.md

Comment 28 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment