New issue
Advanced search Search tips

Issue 776452 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Task

Blocked on:
issue 211405



Sign in to add a comment

Remove dbus-c++ and glib deps from ChromeOS services.

Project Member Reported by hidehiko@chromium.org, Oct 19 2017

Issue description

This is a tracking issue.
For each bigger service, we should make a break down bug.
 
Components: OS>Systems
Cc: benchan@chromium.org
Owner: ejcaruso@chromium.org
Ben and I have been looking at this for a while, e.g.  issue #705051 ,  issue #719073 ,  issue #757584 , and  issue #767645  to name a few.

I think cryptohome is the last major one that needs refactoring in this way. cromo and wimax_manager also use dbus-c++ but we're hoping to deprecate and get rid of them this year as far as I remember so it's not super urgent that we put much effort into those.
Re #2: yes, cromo and wimax-manager will be deprecated soon after the devices depend on them reach EoL this year and stop receiving AU. Given the challenge to find those affected devices for comprehensive testing, I think it's best to skip the chrome-dbus migration for cromo and wimax-manager.
Blockedon: 211405
This seems tobe blocked on issue 211405. 
ejcaruso@, do you plan to work on issue 211405? If so, that'd be great. 
I do plan to migrate cryptohomed off dbus-glib/dbus-c++ and onto chromeos-dbus-bindings at some point. I'm working on some other refactoring for cryptohome at the moment which should hopefully reduce the D-Bus API surface area and make this process a little easier.
Status: Assigned (was: Untriaged)
sounds good!

Comment 7 by derat@chromium.org, Feb 14 2018

Cc: derat@chromium.org
Just as an aside, I can't wait for wimax-manager to go away. Its generated org::chromium::PowerManager_proxy interface at dbus_proxies/org.chromium.PowerManager.h "helpfully" makes all of its signal handlers pure virtual:

    /* signal handlers for this interface.
     * you will have to implement them in your ObjectProxy.
     */
    virtual void BrightnessChanged(const int32_t &brightness_percent, const bool &user_initiated) = 0;
    virtual void KeyboardBrightnessChanged(const int32_t &brightness_percent, const bool &user_initiated) = 0;
    virtual void PeripheralBatteryStatus(const std::vector< uint8_t > &serialized_proto) = 0;
    virtual void PowerSupplyPoll(const std::vector< uint8_t > &serialized_proto) = 0;
    virtual void SuspendImminent(const std::vector< uint8_t > &serialized_proto) = 0;
    virtual void SuspendDone(const std::vector< uint8_t > &serialized_proto) = 0;
    virtual void DarkSuspendImminent(const std::vector< uint8_t > &serialized_proto) = 0;
    virtual void InputEvent(const std::vector< uint8_t > &serialized_proto) = 0;
    virtual void IdleActionImminent(const std::vector< uint8_t > &serialized_proto) = 0;
    virtual void IdleActionDeferred() = 0;
    virtual void ScreenIdleStateChanged(const std::vector< uint8_t > &serialized_proto) = 0;
    virtual void InactivityDelaysChanged(const std::vector< uint8_t > &serialized_proto) = 0;

So whenever I make a change to the signals that powerd emits, I need to change wimax-manager even if it doesn't actually care about the change.
Re #7: derat@, I think this change may be a good compromise:

https://chromium-review.googlesource.com/c/chromiumos/third_party/dbus-cplusplus/+/917528
Project Member

Comment 9 by bugdroid1@chromium.org, Feb 14 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/dbus-cplusplus/+/907f6244dc1c4c3f58a809147d348eccbb932631

commit 907f6244dc1c4c3f58a809147d348eccbb932631
Author: Ben Chan <benchan@chromium.org>
Date: Wed Feb 14 12:07:09 2018

CHROMIUM: provide default implementation for signal handlers

A D-Bus client may not be interested of all the D-Bus signals emitted by
a D-Bus server. This CL modifies the proxy template in dbus-c++ such
that the base proxy class provides a default implementation for signal
handlers, which can be overridden by a subclass as needed.

BUG=chromium:776452
TEST=Build cromo and wimax-manager.
TEST=Remote trybot runs.

Change-Id: I64057d31dc1784bddc29bbb02977fe9a29e1a81e
Reviewed-on: https://chromium-review.googlesource.com/917528
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/907f6244dc1c4c3f58a809147d348eccbb932631/tools/proxy-stubs.tpl

Project Member

Comment 10 by bugdroid1@chromium.org, Feb 24 2018

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

commit 3fde0d30abfef15d984a212a93c64e934a922f79
Author: Ben Chan <benchan@chromium.org>
Date: Sat Feb 24 07:13:00 2018

wimax: remove implementation for ignored D-Bus signals

dbus-c++ has been modified to provide a default implementation for
signal handlers in the generated proxy base class.

BUG=chromium:776452
CQ-DEPEND=CL:917528
TEST=Run unit tests.

Change-Id: Id3803a63227718cb9024e33da6f40ac0e21775e9
Reviewed-on: https://chromium-review.googlesource.com/917533
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/3fde0d30abfef15d984a212a93c64e934a922f79/wimax_manager/power_manager_dbus_proxy.h

Issue 426307 has been merged into this issue.

Sign in to add a comment