Move SystemLogsSource class into new file

Move SystemLogsSource class into new file

Splits SystemLogsSource out of system_logs_fetcher.h.

BUG=710168
Review-Url: https://codereview.chromium.org/2969573002
Cr-Commit-Position: refs/heads/master@{#484674}
Committed: https://chromium.googlesource.com/chromium/src/+/e28523ca804072b27d88cf58d597afd3890a20ec

[michaelpg, 2017-06-29 23:25:12]

Description was changed from

==========
Move SystemLogsSource class into new file

BUG=
==========

to

==========
Move SystemLogsSource class into new file

Splits SystemLogsSource out of system_logs_fetcher.h.

BUG=710168
==========

[michaelpg, 2017-06-29 23:25:12]

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

[michaelpg, 2017-06-29 23:26:09]

PTAL.

Not using gerrit due to
https://bugs.chromium.org/p/gerrit/issues/detail?id=6441.

[michaelpg, 2017-06-30 16:44:00]

rebase

[afakhry, 2017-06-30 23:59:27]

lgtm with Nit.

https://codereview.chromium.org/2969573002/diff/20001/chrome/browser/feedback...
File chrome/browser/feedback/system_logs/system_logs_fetcher.h (right):

https://codereview.chromium.org/2969573002/diff/20001/chrome/browser/feedback...
chrome/browser/feedback/system_logs/system_logs_fetcher.h:24: using
SystemLogsResponse = FeedbackCommon::SystemLogsMap;
Nit: Please remove this from here since you moved it to the new
system_logs_source.h

[michaelpg, 2017-07-05 21:27:27]

The CQ bit was checked by michaelpg@chromium.org

[michaelpg, 2017-07-05 21:27:28]

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/2969573002/#ps40001
(title: "redundant using")

[michaelpg, 2017-07-05 21:27:39]

Thanks.

https://codereview.chromium.org/2969573002/diff/20001/chrome/browser/feedback...
File chrome/browser/feedback/system_logs/system_logs_fetcher.h (right):

https://codereview.chromium.org/2969573002/diff/20001/chrome/browser/feedback...
chrome/browser/feedback/system_logs/system_logs_fetcher.h:24: using
SystemLogsResponse = FeedbackCommon::SystemLogsMap;
On 2017/06/30 23:59:27, afakhry wrote:
> Nit: Please remove this from here since you moved it to the new
> system_logs_source.h

Done.

[commit-bot: I haz the power, 2017-07-05 21:27: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-07-05 23:28:55]

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

[commit-bot: I haz the power, 2017-07-05 23:28:56]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_TIMED_OUT, build has not
started yet; builder either lacks capacity or does not exist (misspelled?))
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_TIMED_OUT, build
has not started yet; builder either lacks capacity or does not exist
(misspelled?))

[michaelpg, 2017-07-06 18:11:48]

The CQ bit was checked by michaelpg@chromium.org

[commit-bot: I haz the power, 2017-07-06 18:12:11]

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-07-06 18:17:06]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1499364708024070,
"parent_rev": "b35720f5e102a25733b3dce0daa1fafda680a5bb", "commit_rev":
"e28523ca804072b27d88cf58d597afd3890a20ec"}

[commit-bot: I haz the power, 2017-07-06 18:17:17]

Description was changed from

==========
Move SystemLogsSource class into new file

Splits SystemLogsSource out of system_logs_fetcher.h.

BUG=710168
==========

to

==========
Move SystemLogsSource class into new file

Splits SystemLogsSource out of system_logs_fetcher.h.

BUG=710168

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

[commit-bot: I haz the power, 2017-07-06 18:17:18]

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

[brucedawson, 2017-07-06 21:35:27]

brucedawson@chromium.org changed reviewers:
+ brucedawson@chromium.org

[brucedawson, 2017-07-06 21:35:28]

This CL creates ODR violations that lead to these warnings:

system_logs_fetcher.obj : warning LNK4006: "public: __thiscall
system_logs::SystemLogsSource::SystemLogsSource(class
std::basic_string<char,struct std::char_traits<char>,class std::allocator<char>
> const &)"
(??0SystemLogsSource@system_logs@@QAE@ABV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@Z)
already defined in system_logs_source.obj; second definition ignored
system_logs_fetcher.obj : warning LNK4006: "public: virtual __thiscall
system_logs::SystemLogsSource::~SystemLogsSource(void)"
(??1SystemLogsSource@system_logs@@UAE@XZ) already defined in
system_logs_source.obj; second definition ignored

It looks like system_logs_fetcher.cc needs to be deleted and removed from
BUILD.gn.

https://codereview.chromium.org/2969573002/diff/40001/chrome/browser/BUILD.gn
File chrome/browser/BUILD.gn (right):

https://codereview.chromium.org/2969573002/diff/40001/chrome/browser/BUILD.gn...
chrome/browser/BUILD.gn:3184: "feedback/system_logs/system_logs_fetcher.cc",
SystemLogsSource constructor and destructor are still defined here.

https://codereview.chromium.org/2969573002/diff/40001/chrome/browser/feedback...
File chrome/browser/feedback/system_logs/system_logs_source.cc (right):

https://codereview.chromium.org/2969573002/diff/40001/chrome/browser/feedback...
chrome/browser/feedback/system_logs/system_logs_source.cc:12:
SystemLogsSource::~SystemLogsSource() {}
SystemLogsSource constructor and destructor defined here...

[afakhry, 2017-07-06 21:43:06]

https://codereview.chromium.org/2969573002/diff/40001/chrome/browser/feedback...
File chrome/browser/feedback/system_logs/system_logs_fetcher.h (left):

https://codereview.chromium.org/2969573002/diff/40001/chrome/browser/feedback...
chrome/browser/feedback/system_logs/system_logs_fetcher.h:38: virtual
~SystemLogsSource();
Sorry, I think I missed noticing in the review that you forgot to remove the
corresponding definitions in the system_logs_fetcher.cc file. Can you please do
that?

[brucedawson, 2017-07-07 18:58:17]

On 2017/07/06 21:43:06, afakhry wrote:
>
https://codereview.chromium.org/2969573002/diff/40001/chrome/browser/feedback...
> File chrome/browser/feedback/system_logs/system_logs_fetcher.h (left):
> 
>
https://codereview.chromium.org/2969573002/diff/40001/chrome/browser/feedback...
> chrome/browser/feedback/system_logs/system_logs_fetcher.h:38: virtual
> ~SystemLogsSource();
> Sorry, I think I missed noticing in the review that you forgot to remove the
> corresponding definitions in the system_logs_fetcher.cc file. Can you please
do
> that?

I created a fix CL: https://chromium-review.googlesource.com/c/563762/ - just
waiting for the try bots to finish.