Misc refactoring of SessionManagerClient.

Misc refactoring of SessionManagerClient.

- In ObjectProxy::CallMethod, in case of error, always LOG.
- SessionManagerClientImpl's dbus call error logging is now relying
  on ObjectProxy, so use EmptyResponseCallback wherever possible.
- Introduce OnNoOutputParamResponse(), for sharing the code (OnStorePolicy
  and OnArcMethod).
- Use nullptr instead of NULL.
- Use "= default" for ctor and dtor if possible.
- Initialize member var on declaration, if possible.

BUG=n/a
TEST=Ran trybots.

Review-Url: https://codereview.chromium.org/2889683006
Cr-Commit-Position: refs/heads/master@{#473513}
Committed: https://chromium.googlesource.com/chromium/src/+/6c197efb6eaff2b3fee5e037ba53791000e400b2

[hidehiko, 2017-05-19 07:36:55]

The CQ bit was checked by hidehiko@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-19 07:37:32]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hidehiko, 2017-05-19 07:39:20]

hidehiko@chromium.org changed reviewers:
+ hashimoto@chromium.org

[hidehiko, 2017-05-19 07:39:21]

PTAL.

[commit-bot: I haz the power, 2017-05-19 08:37:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-19 08:37:31]

Dry run: This issue passed the CQ dry run.

[hashimoto, 2017-05-19 11:02:46]

https://codereview.chromium.org/2889683006/diff/1/dbus/object_proxy.cc
File dbus/object_proxy.cc (right):

https://codereview.chromium.org/2889683006/diff/1/dbus/object_proxy.cc#newcod...
dbus/object_proxy.cc:594: error_name = "(noname)";
How about making this more readable? (e.g. "No error response")
Usually, an error name looks like "org.freedesktop.DBus.Error.Failed" so it's
distinguishable from other errors even when you use a readable string.

https://codereview.chromium.org/2889683006/diff/1/dbus/object_proxy.cc#newcod...
dbus/object_proxy.cc:595: error_message = "(nomessage)";
error_massage is empty when error_response is non-null, but contains no string.
How about also leaving this empty to be consistent?

[hidehiko, 2017-05-19 12:28:49]

The CQ bit was checked by hidehiko@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-19 12:29:06]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hidehiko, 2017-05-19 12:32:04]

Thank you for review.

Addressed your comments at PS2, and I found there's lint warning to run "git cl
format", so I did (PS3).

PTAL.

https://codereview.chromium.org/2889683006/diff/1/dbus/object_proxy.cc
File dbus/object_proxy.cc (right):

https://codereview.chromium.org/2889683006/diff/1/dbus/object_proxy.cc#newcod...
dbus/object_proxy.cc:594: error_name = "(noname)";
On 2017/05/19 11:02:46, hashimoto wrote:
> How about making this more readable? (e.g. "No error response")
> Usually, an error name looks like "org.freedesktop.DBus.Error.Failed" so it's
> distinguishable from other errors even when you use a readable string.

Used "unknown error type" to be consistent with L100.

Or, we may want to pass empty string here, and handle it in LogMethodCallFailure
function? WDYT?

https://codereview.chromium.org/2889683006/diff/1/dbus/object_proxy.cc#newcod...
dbus/object_proxy.cc:595: error_message = "(nomessage)";
On 2017/05/19 11:02:46, hashimoto wrote:
> error_massage is empty when error_response is non-null, but contains no
string.
> How about also leaving this empty to be consistent?

Done.

[commit-bot: I haz the power, 2017-05-19 14:15:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-19 14:15:23]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[hashimoto, 2017-05-22 05:16:28]

lgtm

[hidehiko, 2017-05-22 05:17:09]

The CQ bit was checked by hidehiko@chromium.org

[commit-bot: I haz the power, 2017-05-22 05:17:22]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-22 06:18:31]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1495430229229640,
"parent_rev": "5a3def7f350a35e7e481714f0d3851c9434cdeec", "commit_rev":
"6c197efb6eaff2b3fee5e037ba53791000e400b2"}

[commit-bot: I haz the power, 2017-05-22 06:18:49]

Description was changed from

==========
Misc refactoring of SessionManagerClient.

- In ObjectProxy::CallMethod, in case of error, always LOG.
- SessionManagerClientImpl's dbus call error logging is now relying
  on ObjectProxy, so use EmptyResponseCallback wherever possible.
- Introduce OnNoOutputParamResponse(), for sharing the code (OnStorePolicy
  and OnArcMethod).
- Use nullptr instead of NULL.
- Use "= default" for ctor and dtor if possible.
- Initialize member var on declaration, if possible.

BUG=n/a
TEST=Ran trybots.
==========

to

==========
Misc refactoring of SessionManagerClient.

- In ObjectProxy::CallMethod, in case of error, always LOG.
- SessionManagerClientImpl's dbus call error logging is now relying
  on ObjectProxy, so use EmptyResponseCallback wherever possible.
- Introduce OnNoOutputParamResponse(), for sharing the code (OnStorePolicy
  and OnArcMethod).
- Use nullptr instead of NULL.
- Use "= default" for ctor and dtor if possible.
- Initialize member var on declaration, if possible.

BUG=n/a
TEST=Ran trybots.

Review-Url: https://codereview.chromium.org/2889683006
Cr-Commit-Position: refs/heads/master@{#473513}
Committed:
https://chromium.googlesource.com/chromium/src/+/6c197efb6eaff2b3fee5e037ba53...
==========

[commit-bot: I haz the power, 2017-05-22 06:18:51]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/6c197efb6eaff2b3fee5e037ba53...