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

Issue 720240 link

Starred by 1 user
Status: Archived
Owner:
hidehiko@chromium.org
Closed: Jun 2017
Cc:
█ lhchavez@chromium.org
derat@chromium.org
mnissler@chromium.org
█ ejcaruso@chromium.org
hashimoto@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

clean up org.chromium.SessionManagerInterface XML file

Project Member Reported by hidehiko@chromium.org, May 10 2017 
Chrome Version: ToT
OS: ChromeOS

session_manager's DBus API is maintained in

platform2/login_manager/dbus_binding/org.chromium.SessionManagerInterface looks

Currently, it seems to be used for three purposes.
1) Generate C++ binding.
2) Returning the file content via SessionManagerDBusAdaptor::Introspect
3) Installed to /usr/share/dbus-1/interfaces/.

As for 1), no one uses it, so we should be delete that simply, at least.

Dan, do you know we have more usage? Also, if no one calls Introspect (no 2) use) and no one reads .xml file directly (no 3) use), then the file can be simply removed so that we can free from its maintenance.

WDYT?

Comment 1 by derat@chromium.org, May 10 2017

Cc: mnissler@chromium.org ejcaruso@chromium.org
I think that the XML file is mostly (entirely?) used as documentation.

Cc-ing mnissler@ (other session_manager owner) and ejcaruso@ (who's been doing some work on D-Bus bindings for other packages and may have opinions).

Comment 2 by derat@chromium.org, May 10 2017

Summary: clean up org.chromium.SessionManagerInterface XML file (was: clean up org.chromium.SessionManagerInterface.)

Comment 3 by mnissler@chromium.org, May 10 2017 

I can't say from the top of my head whether the file is technically used or not.

+1 to keeping the documentation that it holds in comments though.

Also, there might be a case for migrating session_manager to use generated dbus bindings? The background here is that session_manager is as old as Chrome OS. There was no bindings generator when it started existence, and since session_manager kinda lacks maintainers that can spend serious cycles, it hasn't been converted to use generated bindings.

Comment 4 by hidehiko@chromium.org, May 11 2017 

#3, thank you for sharing background. Then moving to chromeos-dbus-bindings sounds a way to go.
I'll take a look in my spare time.

Comment 5 by hidehiko@chromium.org, May 15 2017

Owner: hidehiko@chromium.org
Status: Started (was: Untriaged)
Started with slightly lower priority in my spare time.

Comment 6 by mnissler@chromium.org, May 15 2017 

Thanks hidehiko@!
Project Member

Comment 7 by bugdroid1@chromium.org, May 15 2017 

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

commit da100acfcab1c909f6694fadff0e58100d7dfaef
Author: Hidehiko Abe <hidehiko@chromium.org>
Date: Mon May 15 15:18:23 2017

login: Generate SessionManager's dbus adaptor.

This CL just edits the build configuration to generate
dbus adaptor.
In this CL, generated code is not yet used, but following
CL will use it.

BUG= chromium:720240 
TEST=Locally built successfully.

Change-Id: I7f08883edde80777cd285fdbef899b1b9ccf5fab
Reviewed-on: https://chromium-review.googlesource.com/505975
Commit-Ready: Hidehiko Abe <hidehiko@chromium.org>
Tested-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>

[modify] https://crrev.com/da100acfcab1c909f6694fadff0e58100d7dfaef/login_manager/login_manager.gyp
Project Member

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

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

commit 90260658a4ecd78385a5d56b62f4fa602026efce
Author: Hidehiko Abe <hidehiko@chromium.org>
Date: Thu May 18 02:06:32 2017

login: Adapt EnableChromeTesting signature to generated one.

Now, we're moving to use generated dbus-binding code in SessionManager.
This CL is the preparation of it.

BUG= chromium:720240 
TEST=Ran bot. Ran unittests locally.

Change-Id: I4cda3a34d8ae5e84db7e2f0ddf3fa2e74b99a3ba
Reviewed-on: https://chromium-review.googlesource.com/505831
Commit-Ready: Hidehiko Abe <hidehiko@chromium.org>
Tested-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/90260658a4ecd78385a5d56b62f4fa602026efce/login_manager/session_manager_impl.cc
[modify] https://crrev.com/90260658a4ecd78385a5d56b62f4fa602026efce/login_manager/session_manager_impl_unittest.cc
[modify] https://crrev.com/90260658a4ecd78385a5d56b62f4fa602026efce/login_manager/session_manager_dbus_adaptor.cc
[modify] https://crrev.com/90260658a4ecd78385a5d56b62f4fa602026efce/login_manager/session_manager_impl.h
Project Member

Comment 9 by bugdroid1@chromium.org, May 18 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6eff4f61a15bfee8451bf106d7b74b99f6b4264c

commit 6eff4f61a15bfee8451bf106d7b74b99f6b4264c
Author: hidehiko <hidehiko@chromium.org>
Date: Thu May 18 18:18:44 2017

Do not read redundant output param for StorePolicy DBus call family.

For consistency, these will be removed from SessionManager.
In case of success, the value is always true, so it is just redundant.

BUG= 720240 
TEST=Ran bots.

Review-Url: https://codereview.chromium.org/2887323002
Cr-Commit-Position: refs/heads/master@{#472877}

[modify] https://crrev.com/6eff4f61a15bfee8451bf106d7b74b99f6b4264c/chromeos/dbus/session_manager_client.cc
Project Member

Comment 10 by bugdroid1@chromium.org, May 19 2017 

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

commit 93313218c7a7dd8a5e6ded952fa07cbfd3f7d7a5
Author: Hidehiko Abe <hidehiko@chromium.org>
Date: Fri May 19 20:57:50 2017

Do not check redundant output params.

The redundant output params will be removed.
Checking those param is just unnecessary. So remove them.

BUG= chromium:720240 
TEST=Ran edited tests locally. Ran bots.

Change-Id: Ia63e43e664df0a66089841076f05c91c0893b7b0
Reviewed-on: https://chromium-review.googlesource.com/508493
Commit-Ready: Hidehiko Abe <hidehiko@chromium.org>
Tested-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/93313218c7a7dd8a5e6ded952fa07cbfd3f7d7a5/client/site_tests/login_OwnershipRetaken/login_OwnershipRetaken.py
[modify] https://crrev.com/93313218c7a7dd8a5e6ded952fa07cbfd3f7d7a5/client/site_tests/login_GuestAndActualSession/login_GuestAndActualSession.py
[modify] https://crrev.com/93313218c7a7dd8a5e6ded952fa07cbfd3f7d7a5/client/site_tests/login_MultipleSessions/login_MultipleSessions.py
[modify] https://crrev.com/93313218c7a7dd8a5e6ded952fa07cbfd3f7d7a5/client/site_tests/login_MultiUserPolicy/login_MultiUserPolicy.py
[modify] https://crrev.com/93313218c7a7dd8a5e6ded952fa07cbfd3f7d7a5/client/site_tests/login_UserPolicyKeys/login_UserPolicyKeys.py
[modify] https://crrev.com/93313218c7a7dd8a5e6ded952fa07cbfd3f7d7a5/client/site_tests/login_OwnershipApi/login_OwnershipApi.py
[modify] https://crrev.com/93313218c7a7dd8a5e6ded952fa07cbfd3f7d7a5/client/site_tests/login_RetrieveActiveSessions/login_RetrieveActiveSessions.py
[modify] https://crrev.com/93313218c7a7dd8a5e6ded952fa07cbfd3f7d7a5/client/site_tests/login_RemoteOwnership/login_RemoteOwnership.py
[modify] https://crrev.com/93313218c7a7dd8a5e6ded952fa07cbfd3f7d7a5/client/site_tests/login_CryptohomeOwnerQuery/login_CryptohomeOwnerQuery.py
[modify] https://crrev.com/93313218c7a7dd8a5e6ded952fa07cbfd3f7d7a5/client/site_tests/login_SameSessionTwice/login_SameSessionTwice.py
Project Member

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

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

commit 8757c3cc791b91326a572061492daf37149a8398
Author: Hidehiko Abe <hidehiko@chromium.org>
Date: Sat May 20 17:34:18 2017

login: Adapt SessionManager DBus methods to generated ones part2.

This CL adapts the following DBus method signatures to generated ones.
- EmitLoginPromptVisible
- StartSession
- StopSession
- RestartJob

For StartSession and StopSession, clients (=Chrome) does not
read the output params. Thus, these are simply dropped.
EmitLoginPromptVisible and StopSession does not return an error.
Thus, these are annotated "simple".

CQ-DEPEND=CL:508493
BUG= chromium:720240 
TEST=Ran unittests locally. Ran try.

Change-Id: I6c671c7bab0b052c6163ee047dbb8c182f252034
Reviewed-on: https://chromium-review.googlesource.com/508492
Commit-Ready: Hidehiko Abe <hidehiko@chromium.org>
Tested-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/8757c3cc791b91326a572061492daf37149a8398/login_manager/session_manager_impl.cc
[modify] https://crrev.com/8757c3cc791b91326a572061492daf37149a8398/login_manager/dbus_bindings/org.chromium.SessionManagerInterface.xml
[modify] https://crrev.com/8757c3cc791b91326a572061492daf37149a8398/login_manager/session_manager_dbus_adaptor.cc
[modify] https://crrev.com/8757c3cc791b91326a572061492daf37149a8398/login_manager/session_manager_impl_unittest.cc
[modify] https://crrev.com/8757c3cc791b91326a572061492daf37149a8398/login_manager/session_manager_impl.h
Project Member

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

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

commit 50c832d32f845401faff9c60dba581ed752d5434
Author: Hidehiko Abe <hidehiko@chromium.org>
Date: Tue May 23 17:24:04 2017

login: Adapt SessionManager DBus methods to generated ones part3.

This CL:
- removes |done| our params, which are no longer used.
- Adapt StartWipeDevice() signature to generated one.
  - Along with the change, StartWipeDevice test is renamed to
    InitiateDeviceWipe_TooLongReason.

BUG= chromium:720240 
TEST=Ran unittests locally. Ran try.

Change-Id: I8dca6181c0720e205b965713958c6b522cf5cdfc
Reviewed-on: https://chromium-review.googlesource.com/512464
Commit-Ready: Hidehiko Abe <hidehiko@chromium.org>
Tested-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/50c832d32f845401faff9c60dba581ed752d5434/login_manager/session_manager_impl.cc
[modify] https://crrev.com/50c832d32f845401faff9c60dba581ed752d5434/login_manager/dbus_bindings/org.chromium.SessionManagerInterface.xml
[modify] https://crrev.com/50c832d32f845401faff9c60dba581ed752d5434/login_manager/session_manager_dbus_adaptor.cc
[modify] https://crrev.com/50c832d32f845401faff9c60dba581ed752d5434/login_manager/session_manager_impl_unittest.cc
[modify] https://crrev.com/50c832d32f845401faff9c60dba581ed752d5434/login_manager/session_manager_impl.h
Project Member

Comment 13 by bugdroid1@chromium.org, May 26 2017 

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

commit 0a09d6fcc25dacb2dd29f0eeeed8b264f51ab90b
Author: Hidehiko Abe <hidehiko@chromium.org>
Date: Fri May 26 03:48:53 2017

login: Migrate PolicyService::Error to brillo::Error.

This is prepration to migrate async StorePolicy DBus method
family into generated one.
- Migrated PolicyService::Error into brillo::Error.
- Reorder completion callback to the end for consistency.
- Moved CreateError from session_manager_impl to dbus_util
  to share it across files.

BUG= chromium:720240 
TEST=Ran cros_workon_make chromeos-login --test. Ran bots.

Change-Id: I23409e6ea1e6ea051559dd02ebc23361b9393277
Reviewed-on: https://chromium-review.googlesource.com/513845
Commit-Ready: Hidehiko Abe <hidehiko@chromium.org>
Tested-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/0a09d6fcc25dacb2dd29f0eeeed8b264f51ab90b/login_manager/session_manager_impl.cc
[modify] https://crrev.com/0a09d6fcc25dacb2dd29f0eeeed8b264f51ab90b/login_manager/user_policy_service_unittest.cc
[modify] https://crrev.com/0a09d6fcc25dacb2dd29f0eeeed8b264f51ab90b/login_manager/device_local_account_policy_service.cc
[modify] https://crrev.com/0a09d6fcc25dacb2dd29f0eeeed8b264f51ab90b/login_manager/mock_device_policy_service.h
[modify] https://crrev.com/0a09d6fcc25dacb2dd29f0eeeed8b264f51ab90b/login_manager/policy_service_unittest.cc
[modify] https://crrev.com/0a09d6fcc25dacb2dd29f0eeeed8b264f51ab90b/login_manager/mock_policy_service.h
[modify] https://crrev.com/0a09d6fcc25dacb2dd29f0eeeed8b264f51ab90b/login_manager/session_manager_impl_unittest.cc
[modify] https://crrev.com/0a09d6fcc25dacb2dd29f0eeeed8b264f51ab90b/login_manager/user_policy_service.cc
[modify] https://crrev.com/0a09d6fcc25dacb2dd29f0eeeed8b264f51ab90b/login_manager/policy_service.cc
[modify] https://crrev.com/0a09d6fcc25dacb2dd29f0eeeed8b264f51ab90b/login_manager/policy_service.h
[modify] https://crrev.com/0a09d6fcc25dacb2dd29f0eeeed8b264f51ab90b/login_manager/device_policy_service.cc
[modify] https://crrev.com/0a09d6fcc25dacb2dd29f0eeeed8b264f51ab90b/login_manager/device_policy_service.h
[add] https://crrev.com/0a09d6fcc25dacb2dd29f0eeeed8b264f51ab90b/login_manager/dbus_util.cc
[modify] https://crrev.com/0a09d6fcc25dacb2dd29f0eeeed8b264f51ab90b/login_manager/login_manager.gyp
[modify] https://crrev.com/0a09d6fcc25dacb2dd29f0eeeed8b264f51ab90b/login_manager/user_policy_service.h
[modify] https://crrev.com/0a09d6fcc25dacb2dd29f0eeeed8b264f51ab90b/login_manager/device_policy_service_unittest.cc
[modify] https://crrev.com/0a09d6fcc25dacb2dd29f0eeeed8b264f51ab90b/login_manager/session_manager_dbus_adaptor.cc
[add] https://crrev.com/0a09d6fcc25dacb2dd29f0eeeed8b264f51ab90b/login_manager/dbus_util.h
[modify] https://crrev.com/0a09d6fcc25dacb2dd29f0eeeed8b264f51ab90b/login_manager/vpd_process_impl.cc
Project Member

Comment 14 by bugdroid1@chromium.org, May 31 2017 

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

commit 157f902b1ae0376ba318fd5265db7c11b444255c
Author: Hidehiko Abe <hidehiko@chromium.org>
Date: Wed May 31 09:15:30 2017

login: Adapt SessionManager DBus methods to generated ones part4.

This CL migrates following four method signatures.
- RetreiveSessionState
- RetrieveActiveSessions
- HandleSupervisedUserCreationStarting
- HandleSupervisedUserCreationFinished

BUG= chromium:720240 
TEST=Ran cros_workon_make chromeos-login --test. Ran try.

Change-Id: I2d2384a6e65389a9e78cdfe9495e61558f2f8484
Reviewed-on: https://chromium-review.googlesource.com/513811
Commit-Ready: Hidehiko Abe <hidehiko@chromium.org>
Tested-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>

[modify] https://crrev.com/157f902b1ae0376ba318fd5265db7c11b444255c/login_manager/session_manager_impl.cc
[modify] https://crrev.com/157f902b1ae0376ba318fd5265db7c11b444255c/login_manager/dbus_bindings/org.chromium.SessionManagerInterface.xml
[modify] https://crrev.com/157f902b1ae0376ba318fd5265db7c11b444255c/login_manager/session_manager_dbus_adaptor.cc
[modify] https://crrev.com/157f902b1ae0376ba318fd5265db7c11b444255c/login_manager/session_manager_impl_unittest.cc
[modify] https://crrev.com/157f902b1ae0376ba318fd5265db7c11b444255c/login_manager/session_manager_impl.h
Project Member

Comment 15 by bugdroid1@chromium.org, Jun 2 2017 

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

commit 25a1236a6011f0608444d211dab60910d127dc2b
Author: Hidehiko Abe <hidehiko@chromium.org>
Date: Fri Jun 02 06:49:34 2017

login: Adapt SessionManager DBus methods to generated ones part5.

This CL migrates following seven method signatures.
- LockScreen
- HandleLockScreenShown
- HandleLockScreenDismissed
- SetFlagsForUser
- InitMachineInfo
- StartContainer
- StopContainer

BUG= chromium:720240 
TEST=Ran cros_workon_make chromeos-login --test. Ran try.

Change-Id: I2b37a931d779675a621fc681a8f2e9db3749a218
Reviewed-on: https://chromium-review.googlesource.com/513687
Commit-Ready: Hidehiko Abe <hidehiko@chromium.org>
Tested-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/25a1236a6011f0608444d211dab60910d127dc2b/login_manager/session_manager_impl.cc
[modify] https://crrev.com/25a1236a6011f0608444d211dab60910d127dc2b/login_manager/dbus_bindings/org.chromium.SessionManagerInterface.xml
[modify] https://crrev.com/25a1236a6011f0608444d211dab60910d127dc2b/login_manager/session_manager_dbus_adaptor.cc
[modify] https://crrev.com/25a1236a6011f0608444d211dab60910d127dc2b/login_manager/session_manager_impl_unittest.cc
[modify] https://crrev.com/25a1236a6011f0608444d211dab60910d127dc2b/login_manager/session_manager_impl.h
Project Member

Comment 16 by bugdroid1@chromium.org, Jun 5 2017 

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

commit 2312f8c1ffd95dcea6eb26ad5be20b6738407190
Author: Hidehiko Abe <hidehiko@chromium.org>
Date: Mon Jun 05 06:44:25 2017

login: Adapt SessionManagerDBus methods to generated ones part7.

This CL migrates ARC related method signatures, except StartArcInstance.

BUG= chromium:720240 
TEST=Ran cros_workon_make chromeos-login --test. Ran try.

Change-Id: I56609e53697c8d8d69df8b56656546323c1b3d65
Reviewed-on: https://chromium-review.googlesource.com/522467
Commit-Ready: Hidehiko Abe <hidehiko@chromium.org>
Tested-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/2312f8c1ffd95dcea6eb26ad5be20b6738407190/login_manager/session_manager_impl.cc
[modify] https://crrev.com/2312f8c1ffd95dcea6eb26ad5be20b6738407190/login_manager/session_manager_impl_unittest.cc
[modify] https://crrev.com/2312f8c1ffd95dcea6eb26ad5be20b6738407190/login_manager/session_manager_dbus_adaptor.cc
[modify] https://crrev.com/2312f8c1ffd95dcea6eb26ad5be20b6738407190/login_manager/session_manager_impl.h
Project Member

Comment 17 by bugdroid1@chromium.org, Jun 5 2017 

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

commit 9f3cb87e19628ebfc73ae95c9fbaaab0b6adaebf
Author: Hidehiko Abe <hidehiko@chromium.org>
Date: Mon Jun 05 23:39:52 2017

login: Rework StartArcInstance.

This CL cleans up StartArcInstance code.
- Move container_instance_id generating back from SessionManagerDBusAdapter
  so that it just focuses to read/write messages.
  This is prepration to switch dbus generating tool.
- Fix container start up error handling.
  Original code sends stop-arc-instance upstart signal always if StartContainer
  or StartArcNetwork fails. However, once StartContainer succeeds,
  it should be done in OnAndroidContainerStopped() should handle
  the signal sending.
- Use brillo::ErrorPtr in helper methods.

BUG= chromium:720240 
TEST=Ran cros_workon_make chromeos-login --test. Ran try.

Change-Id: I90cefc2e3f9a6f608fed23e120b9e89a7a4d3ec1
Reviewed-on: https://chromium-review.googlesource.com/523982
Commit-Ready: Yusuke Sato <yusukes@chromium.org>
Tested-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/9f3cb87e19628ebfc73ae95c9fbaaab0b6adaebf/login_manager/session_manager_impl.cc
[modify] https://crrev.com/9f3cb87e19628ebfc73ae95c9fbaaab0b6adaebf/login_manager/session_manager_impl_unittest.cc
[modify] https://crrev.com/9f3cb87e19628ebfc73ae95c9fbaaab0b6adaebf/login_manager/session_manager_dbus_adaptor.cc
[modify] https://crrev.com/9f3cb87e19628ebfc73ae95c9fbaaab0b6adaebf/login_manager/session_manager_impl.h
Project Member

Comment 20 by bugdroid1@chromium.org, Jun 8 2017 

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

commit cfd2569972783b02606f5c88b660386dcd31cdd9
Author: Hidehiko Abe <hidehiko@chromium.org>
Date: Thu Jun 08 11:48:30 2017

login: Drop Introspect supporting from SessionManager.

Looks no one uses it.
For migration of chromeos-dbus-bindings tool, drop it.

BUG= chromium:720240 
TEST=Ran cros_workon_make chromeos-login --test. Ran try.

Change-Id: Ib17fe001dd86466684c8a313927e565bf4a96114
Reviewed-on: https://chromium-review.googlesource.com/526898
Commit-Ready: Hidehiko Abe <hidehiko@chromium.org>
Tested-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/cfd2569972783b02606f5c88b660386dcd31cdd9/login_manager/session_manager_dbus_adaptor.cc
[modify] https://crrev.com/cfd2569972783b02606f5c88b660386dcd31cdd9/login_manager/session_manager_dbus_adaptor.h
Project Member

Comment 21 by bugdroid1@chromium.org, Jun 9 2017 

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

commit dc2f4e4f7d3162836ce1fd023e133c674f5ae520
Author: Hidehiko Abe <hidehiko@chromium.org>
Date: Fri Jun 09 10:42:17 2017

login: Switching D-Bus implementation, part 1.

All method signatures are adapted to the generated one.
So, this CL makes SessionManagerImpl inherit
org::chromium::SessionManagerInterfaceInterface.

Now, SessionManagerInterface.xml and SessionManagerImpl needs to be in sync.

BUG= chromium:720240 
TEST=Ran cros_workon_make chromeos-login --test. Ran try.

Change-Id: I65907c0bc75ebfa8c7744d7c369d877da8f0b7b4
Reviewed-on: https://chromium-review.googlesource.com/527858
Commit-Ready: Hidehiko Abe <hidehiko@chromium.org>
Tested-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>

[modify] https://crrev.com/dc2f4e4f7d3162836ce1fd023e133c674f5ae520/login_manager/session_manager_impl.cc
[modify] https://crrev.com/dc2f4e4f7d3162836ce1fd023e133c674f5ae520/login_manager/session_manager_impl_unittest.cc
[modify] https://crrev.com/dc2f4e4f7d3162836ce1fd023e133c674f5ae520/login_manager/session_manager_dbus_adaptor.cc
[modify] https://crrev.com/dc2f4e4f7d3162836ce1fd023e133c674f5ae520/login_manager/session_manager_dbus_adaptor.h
[modify] https://crrev.com/dc2f4e4f7d3162836ce1fd023e133c674f5ae520/login_manager/session_manager_impl.h
Project Member

Comment 22 by bugdroid1@chromium.org, Jun 9 2017 

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

commit b777fabe28a5303484caaa7ddbc1884fe589f7ad
Author: Hidehiko Abe <hidehiko@chromium.org>
Date: Fri Jun 09 18:22:17 2017

login: Get rid of MockDbusSignalEmitter.

If we switched to tool generated DBus interface,
D-Bus signal emitting interface cannot be mocked by gmock.
As preparation for the switching, this CL gets rid of
MockDBusSignalEmitter, and mock ExportedObject::SendSignal,
instead.

BUG= chromium:720240 
TEST=Ran cros_workon_make chromeos-login --test. Ran try.

Change-Id: Id9bb509cc5fd6cee32b0b48e60588fd9b7851c28
Reviewed-on: https://chromium-review.googlesource.com/529344
Commit-Ready: Hidehiko Abe <hidehiko@chromium.org>
Tested-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>

[delete] https://crrev.com/cd01872c8126e350c94f13bfed896f49b075e573/login_manager/mock_dbus_signal_emitter.h
[modify] https://crrev.com/b777fabe28a5303484caaa7ddbc1884fe589f7ad/login_manager/dbus_signal_emitter.cc
[modify] https://crrev.com/b777fabe28a5303484caaa7ddbc1884fe589f7ad/login_manager/mock_constructors.cc
[modify] https://crrev.com/b777fabe28a5303484caaa7ddbc1884fe589f7ad/login_manager/session_manager_service.h
[modify] https://crrev.com/b777fabe28a5303484caaa7ddbc1884fe589f7ad/login_manager/dbus_signal_emitter.h
[modify] https://crrev.com/b777fabe28a5303484caaa7ddbc1884fe589f7ad/login_manager/session_manager_service.cc
[modify] https://crrev.com/b777fabe28a5303484caaa7ddbc1884fe589f7ad/login_manager/session_manager_impl.cc
[modify] https://crrev.com/b777fabe28a5303484caaa7ddbc1884fe589f7ad/login_manager/session_manager_impl_unittest.cc
[modify] https://crrev.com/b777fabe28a5303484caaa7ddbc1884fe589f7ad/login_manager/session_manager_impl.h
Project Member

Comment 23 by bugdroid1@chromium.org, Jun 12 2017 

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

commit 5316aebe5e3572615aadada3ad7c386a5d14d8a9
Author: Hidehiko Abe <hidehiko@chromium.org>
Date: Mon Jun 12 19:30:37 2017

login: Move SessionManagerDBusAdaptor from Service to Impl.

As preparation to switch the D-Bus library,
this CL removes the dependency from SessionManagerDBusService
to SessionManagerDBusAdaptor.

Along with the change, SessionManagerImpl::DBusService is
introduced to control shutdown timing.

BUG= chromium:720240 
TEST=Ran cros_workon_make chromeos-login --test. Ran try.

Change-Id: I6b9dda8b31149e4c1e977795a93285904245a6cd
Reviewed-on: https://chromium-review.googlesource.com/530609
Commit-Ready: Hidehiko Abe <hidehiko@chromium.org>
Tested-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>

[modify] https://crrev.com/5316aebe5e3572615aadada3ad7c386a5d14d8a9/login_manager/session_manager_interface.h
[modify] https://crrev.com/5316aebe5e3572615aadada3ad7c386a5d14d8a9/login_manager/mock_session_manager.h
[modify] https://crrev.com/5316aebe5e3572615aadada3ad7c386a5d14d8a9/login_manager/session_manager_service.cc
[modify] https://crrev.com/5316aebe5e3572615aadada3ad7c386a5d14d8a9/login_manager/session_manager_service.h
[modify] https://crrev.com/5316aebe5e3572615aadada3ad7c386a5d14d8a9/login_manager/session_manager_impl.cc
[modify] https://crrev.com/5316aebe5e3572615aadada3ad7c386a5d14d8a9/login_manager/session_manager_impl_unittest.cc
[modify] https://crrev.com/5316aebe5e3572615aadada3ad7c386a5d14d8a9/login_manager/session_manager_impl.h
Project Member

Comment 24 by bugdroid1@chromium.org, Jun 14 2017 

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

commit 9e4221b654411e1dbc355b5f00b2f92a072d35c9
Author: Hidehiko Abe <hidehiko@chromium.org>
Date: Wed Jun 14 04:23:57 2017

login: Switch D-Bus implementation, part 2.

This CL switches from manually handled D-Bus implementation to
tool generated interfaces completely.

BUG= chromium:720240 
TEST=Ran cros_workon_make chromeos-login --test. Ran try.

Change-Id: Ibc84c08a6c8441e2380f635508167c737e79986e
Reviewed-on: https://chromium-review.googlesource.com/532467
Commit-Ready: Hidehiko Abe <hidehiko@chromium.org>
Tested-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>

[modify] https://crrev.com/9e4221b654411e1dbc355b5f00b2f92a072d35c9/login_manager/session_manager_main.cc
[delete] https://crrev.com/e025866917117caf522640cc9613d7987d986cfe/login_manager/dbus_signal_emitter.cc
[delete] https://crrev.com/e025866917117caf522640cc9613d7987d986cfe/login_manager/dbus_signal_emitter.h
[modify] https://crrev.com/9e4221b654411e1dbc355b5f00b2f92a072d35c9/login_manager/session_manager_impl.cc
[modify] https://crrev.com/9e4221b654411e1dbc355b5f00b2f92a072d35c9/login_manager/dbus_bindings/org.chromium.SessionManagerInterface.xml
[modify] https://crrev.com/9e4221b654411e1dbc355b5f00b2f92a072d35c9/login_manager/session_manager_impl_unittest.cc
[modify] https://crrev.com/9e4221b654411e1dbc355b5f00b2f92a072d35c9/login_manager/login_manager.gyp
[delete] https://crrev.com/e025866917117caf522640cc9613d7987d986cfe/login_manager/session_manager_dbus_adaptor.cc
[delete] https://crrev.com/e025866917117caf522640cc9613d7987d986cfe/login_manager/session_manager_dbus_adaptor.h
[modify] https://crrev.com/9e4221b654411e1dbc355b5f00b2f92a072d35c9/login_manager/session_manager_impl.h

Comment 25 by hidehiko@chromium.org, Jun 14 2017

Status: Fixed (was: Started)
Done.

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

Status: Archived (was: Fixed)
►

Sign in to add a comment