Add log source to read from one DebugDaemon log

Add log source to read from one DebugDaemon log

DebugDaemon has a function, GetLog(), that returns the output of one
log command. This patch adds a new log source class to fetch the output
of GetLog().

BUG=733846
Review-Url: https://codereview.chromium.org/2955463002
Cr-Commit-Position: refs/heads/master@{#481884}
Committed: https://chromium.googlesource.com/chromium/src/+/f1f0b837c9b55453209f84a4fb8dbecec1f383da

[Simon Que, 2017-06-22 16:10:38]

sque@chromium.org changed reviewers:
+ afakhry@chromium.org

[afakhry, 2017-06-22 22:20:59]

Didn't look at tests yet.

https://codereview.chromium.org/2955463002/diff/1/chrome/browser/chromeos/sys...
File chrome/browser/chromeos/system_logs/single_debug_daemon_log_source.cc
(right):

https://codereview.chromium.org/2955463002/diff/1/chrome/browser/chromeos/sys...
chrome/browser/chromeos/system_logs/single_debug_daemon_log_source.cc:21: //
added, this function should return an empty file path.
Copy-and-paste comment. Please modify! :)

https://codereview.chromium.org/2955463002/diff/1/chrome/browser/chromeos/sys...
chrome/browser/chromeos/system_logs/single_debug_daemon_log_source.cc:48:
client->GetLog(source_name(),
I don't see this function belonging to DebugDaemonClient at all! Was that added
recently?

https://cs.chromium.org/chromium/src/chromeos/dbus/debug_daemon_client.h?q=De...

https://codereview.chromium.org/2955463002/diff/1/chrome/browser/chromeos/sys...
chrome/browser/chromeos/system_logs/single_debug_daemon_log_source.cc:55: bool
success,
Can you check "success" and add a test for that?

https://codereview.chromium.org/2955463002/diff/1/chrome/browser/chromeos/sys...
chrome/browser/chromeos/system_logs/single_debug_daemon_log_source.cc:62:
response[entry.first] = anonymizer.Anonymize(entry.second);
Nit: emplace(key, val);

[Simon Que, 2017-06-22 22:38:27]

https://codereview.chromium.org/2955463002/diff/1/chrome/browser/chromeos/sys...
File chrome/browser/chromeos/system_logs/single_debug_daemon_log_source.cc
(right):

https://codereview.chromium.org/2955463002/diff/1/chrome/browser/chromeos/sys...
chrome/browser/chromeos/system_logs/single_debug_daemon_log_source.cc:21: //
added, this function should return an empty file path.
On 2017/06/22 22:20:58, afakhry wrote:
> Copy-and-paste comment. Please modify! :)

Done.

https://codereview.chromium.org/2955463002/diff/1/chrome/browser/chromeos/sys...
chrome/browser/chromeos/system_logs/single_debug_daemon_log_source.cc:48:
client->GetLog(source_name(),
On 2017/06/22 22:20:59, afakhry wrote:
> I don't see this function belonging to DebugDaemonClient at all! Was that
added
> recently?
> 
>
https://cs.chromium.org/chromium/src/chromeos/dbus/debug_daemon_client.h?q=De...

That function is being added in a CL that this one depends on:
https://codereview.chromium.org/2944403002#ps20001

https://codereview.chromium.org/2955463002/diff/1/chrome/browser/chromeos/sys...
chrome/browser/chromeos/system_logs/single_debug_daemon_log_source.cc:55: bool
success,
On 2017/06/22 22:20:59, afakhry wrote:
> Can you check "success" and add a test for that?

I can return an empty result on !success, but I don't know how to test for it in
the unit test.

One idea I had was to update FakeDebugDaemonClient::GetLog() to return !success
if log_name="invalid", but SingleDebugDaemonLogSource's constructor only takes
valid enums as an arg.

https://codereview.chromium.org/2955463002/diff/1/chrome/browser/chromeos/sys...
chrome/browser/chromeos/system_logs/single_debug_daemon_log_source.cc:62:
response[entry.first] = anonymizer.Anonymize(entry.second);
On 2017/06/22 22:20:58, afakhry wrote:
> Nit: emplace(key, val);

Done.

[afakhry, 2017-06-22 23:34:16]

lgtm with Nit.

https://codereview.chromium.org/2955463002/diff/1/chrome/browser/chromeos/sys...
File chrome/browser/chromeos/system_logs/single_debug_daemon_log_source.cc
(right):

https://codereview.chromium.org/2955463002/diff/1/chrome/browser/chromeos/sys...
chrome/browser/chromeos/system_logs/single_debug_daemon_log_source.cc:55: bool
success,
On 2017/06/22 22:38:27, Simon Que wrote:
> On 2017/06/22 22:20:59, afakhry wrote:
> > Can you check "success" and add a test for that?
> 
> I can return an empty result on !success, but I don't know how to test for it
in
> the unit test.
> 
> One idea I had was to update FakeDebugDaemonClient::GetLog() to return
!success
> if log_name="invalid", but SingleDebugDaemonLogSource's constructor only takes
> valid enums as an arg.

Forget about this test request. It's not worth it.

https://codereview.chromium.org/2955463002/diff/20001/chrome/browser/chromeos...
File
chrome/browser/chromeos/system_logs/single_debug_daemon_log_source_unittest.cc
(right):

https://codereview.chromium.org/2955463002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/system_logs/single_debug_daemon_log_source_unittest.cc:37:
new chromeos::FakeDebugDaemonClient));
Nit: MakeUnique<>

[Simon Que, 2017-06-23 05:06:40]

https://codereview.chromium.org/2955463002/diff/20001/chrome/browser/chromeos...
File
chrome/browser/chromeos/system_logs/single_debug_daemon_log_source_unittest.cc
(right):

https://codereview.chromium.org/2955463002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/system_logs/single_debug_daemon_log_source_unittest.cc:37:
new chromeos::FakeDebugDaemonClient));
On 2017/06/22 23:34:15, afakhry wrote:
> Nit: MakeUnique<>

Done.

[Simon Que, 2017-06-23 05:07:49]

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

[Simon Que, 2017-06-23 05:07:53]

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

[commit-bot: I haz the power, 2017-06-23 05:07:59]

Dry run: 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-06-23 06:51:03]

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

[commit-bot: I haz the power, 2017-06-23 06:51:04]

Dry run: This issue passed the CQ dry run.

[Simon Que, 2017-06-23 14:40:43]

The CQ bit was checked by sque@chromium.org

[Simon Que, 2017-06-23 14:40:44]

The patchset sent to the CQ was uploaded after l-g-t-m from afakhry@chromium.org
Link to the patchset: https://codereview.chromium.org/2955463002/#ps40001
(title: "Rebased: updated to latest DebugDaemonClient; Browser thread bundle")

[commit-bot: I haz the power, 2017-06-23 14:40:52]

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-06-23 14:44:56]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1498228843853390,
"parent_rev": "e4175877f5a72d240e89bde552f79fc31e2b0cad", "commit_rev":
"f1f0b837c9b55453209f84a4fb8dbecec1f383da"}

[commit-bot: I haz the power, 2017-06-23 14:45:11]

Description was changed from

==========
Add log source to read from one DebugDaemon log

DebugDaemon has a function, GetLog(), that returns the output of one
log command. This patch adds a new log source class to fetch the output
of GetLog().

BUG=733846
==========

to

==========
Add log source to read from one DebugDaemon log

DebugDaemon has a function, GetLog(), that returns the output of one
log command. This patch adds a new log source class to fetch the output
of GetLog().

BUG=733846

Review-Url: https://codereview.chromium.org/2955463002
Cr-Commit-Position: refs/heads/master@{#481884}
Committed:
https://chromium.googlesource.com/chromium/src/+/f1f0b837c9b55453209f84a4fb8d...
==========

[commit-bot: I haz the power, 2017-06-23 14:45:12]

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