[win] Create utilities for helping to populate ModuleDatabase.

[win] Create utilities for helping to populate ModuleDatabase.

Further CLs will move (and test!) the rest of the enumerate_modules_model logic to this collection of helper functions.

BUG=662084
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

Review-Url: https://codereview.chromium.org/2653333002
Cr-Commit-Position: refs/heads/master@{#448536}
Committed: https://chromium.googlesource.com/chromium/src/+/2b10cfea6a7bfad0a361aa79d66e7ede05eb397a

[chrisha, 2017-01-25 22:28:33]

chrisha@chromium.org changed reviewers:
+ pmonette@chromium.org

[chrisha, 2017-01-25 22:28:33]

This is mostly just moving around already reviewed code.

PTAL?

[chrisha, 2017-01-26 19:21:45]

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

[commit-bot: I haz the power, 2017-01-26 19:22:35]

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-01-26 20:33:53]

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

[commit-bot: I haz the power, 2017-01-26 20:33:53]

Dry run: This issue passed the CQ dry run.

[Patrick Monette, 2017-01-26 20:49:07]

https://codereview.chromium.org/2653333002/diff/1/chrome/browser/conflicts/mo...
File chrome/browser/conflicts/module_database_win.cc (right):

https://codereview.chromium.org/2653333002/diff/1/chrome/browser/conflicts/mo...
chrome/browser/conflicts/module_database_win.cc:349:
ModuleDatabase::ModuleInfoData::ModuleInfoData() : process_types(0) {}
Change to PROCESS_TYPE_UNKNOWN whose value is 1. There is no type of value 0.

https://codereview.chromium.org/2653333002/diff/1/chrome/browser/conflicts/mo...
chrome/browser/conflicts/module_database_win.cc:354:
ModuleDatabase::ModuleInfoData::~ModuleInfoData() {}
= default;

https://codereview.chromium.org/2653333002/diff/1/chrome/browser/conflicts/mo...
File chrome/browser/conflicts/module_database_win.h (right):

https://codereview.chromium.org/2653333002/diff/1/chrome/browser/conflicts/mo...
chrome/browser/conflicts/module_database_win.h:227: struct
ModuleDatabase::ModuleInfoData {
Maybe we should add a more explicit "inspected" or "populated" bit that is false
until it has been processed on the blocking pool.

https://codereview.chromium.org/2653333002/diff/1/chrome/browser/conflicts/mo...
chrome/browser/conflicts/module_database_win.h:229: ModuleInfoData(const
ModuleInfoData& others);
What's the reason for explicitly defining the default copy constructor?

https://codereview.chromium.org/2653333002/diff/1/chrome/browser/conflicts/mo...
chrome/browser/conflicts/module_database_win.h:261: CertificateInfo cert_info;
Nit. This is the only name that is truncated. Inconsistent.

Feel free to ignore.

https://codereview.chromium.org/2653333002/diff/1/chrome/browser/conflicts/mo...
File chrome/browser/conflicts/module_info_util_win.h (right):

https://codereview.chromium.org/2653333002/diff/1/chrome/browser/conflicts/mo...
chrome/browser/conflicts/module_info_util_win.h:12: void
GetCertificateInfo(const base::FilePath& file,
Is it too much trouble to remove the duplicate code in
enumerate_modules_model.cc and directly call this?

[chrisha, 2017-02-06 18:15:27]

Description was changed from

==========
[win] Create utilities for helping to populate ModuleDatabase.

Further CLs will move (and test!) the rest of the enumerate_modules_model logic
to this collection of helper functions.

BUG=662084
==========

to

==========
[win] Create utilities for helping to populate ModuleDatabase.

Further CLs will move (and test!) the rest of the enumerate_modules_model logic
to this collection of helper functions.

BUG=662084
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

[chrisha, 2017-02-06 18:38:46]

Thanks, committing.

https://codereview.chromium.org/2653333002/diff/1/chrome/browser/conflicts/mo...
File chrome/browser/conflicts/module_database_win.cc (right):

https://codereview.chromium.org/2653333002/diff/1/chrome/browser/conflicts/mo...
chrome/browser/conflicts/module_database_win.cc:349:
ModuleDatabase::ModuleInfoData::ModuleInfoData() : process_types(0) {}
On 2017/01/26 20:49:07, Patrick Monette wrote:
> Change to PROCESS_TYPE_UNKNOWN whose value is 1. There is no type of value 0.

This is a bitmask of the various process types. 0 means none of them have been
observed.

https://codereview.chromium.org/2653333002/diff/1/chrome/browser/conflicts/mo...
chrome/browser/conflicts/module_database_win.cc:354:
ModuleDatabase::ModuleInfoData::~ModuleInfoData() {}
On 2017/01/26 20:49:07, Patrick Monette wrote:
> = default;

Done.

https://codereview.chromium.org/2653333002/diff/1/chrome/browser/conflicts/mo...
File chrome/browser/conflicts/module_database_win.h (right):

https://codereview.chromium.org/2653333002/diff/1/chrome/browser/conflicts/mo...
chrome/browser/conflicts/module_database_win.h:227: struct
ModuleDatabase::ModuleInfoData {
On 2017/01/26 20:49:07, Patrick Monette wrote:
> Maybe we should add a more explicit "inspected" or "populated" bit that is
false
> until it has been processed on the blocking pool.

Yeah, that's a good point. I won't bother with that for now, but let you do that
when we actually start populating the data :)

https://codereview.chromium.org/2653333002/diff/1/chrome/browser/conflicts/mo...
chrome/browser/conflicts/module_database_win.h:229: ModuleInfoData(const
ModuleInfoData& others);
On 2017/01/26 20:49:07, Patrick Monette wrote:
> What's the reason for explicitly defining the default copy constructor?

Because the clang bots complain that this needs to be done :(

https://codereview.chromium.org/2653333002/diff/1/chrome/browser/conflicts/mo...
chrome/browser/conflicts/module_database_win.h:261: CertificateInfo cert_info;
On 2017/01/26 20:49:07, Patrick Monette wrote:
> Nit. This is the only name that is truncated. Inconsistent.
> 
> Feel free to ignore.

Done.

https://codereview.chromium.org/2653333002/diff/1/chrome/browser/conflicts/mo...
File chrome/browser/conflicts/module_info_util_win.h (right):

https://codereview.chromium.org/2653333002/diff/1/chrome/browser/conflicts/mo...
chrome/browser/conflicts/module_info_util_win.h:12: void
GetCertificateInfo(const base::FilePath& file,
On 2017/01/26 20:49:07, Patrick Monette wrote:
> Is it too much trouble to remove the duplicate code in
> enumerate_modules_model.cc and directly call this?

Done!

[chrisha, 2017-02-06 18:38:51]

The CQ bit was checked by chrisha@chromium.org

[commit-bot: I haz the power, 2017-02-06 18:39:28]

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-02-06 18:39:29]

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

[commit-bot: I haz the power, 2017-02-06 18:39:30]

No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
Committers are members of the group "project-chromium-committers".
Note that this has nothing to do with OWNERS files.

[chrisha, 2017-02-06 19:01:24]

Oops, not committing. Need your l-g-t-m Pat!

[Patrick Monette, 2017-02-06 19:05:23]

lgtm with one modification.

https://codereview.chromium.org/2653333002/diff/20001/chrome/browser/conflict...
File chrome/browser/conflicts/module_info_util_win.cc (right):

https://codereview.chromium.org/2653333002/diff/20001/chrome/browser/conflict...
chrome/browser/conflicts/module_info_util_win.cc:7: #include <mscat.h>  //
NOLINT: This must be after wincrypt and wintrust.
Did you inadvertently moved this?

[chrisha, 2017-02-06 20:54:46]

https://codereview.chromium.org/2653333002/diff/20001/chrome/browser/conflict...
File chrome/browser/conflicts/module_info_util_win.cc (right):

https://codereview.chromium.org/2653333002/diff/20001/chrome/browser/conflict...
chrome/browser/conflicts/module_info_util_win.cc:7: #include <mscat.h>  //
NOLINT: This must be after wincrypt and wintrust.
On 2017/02/06 19:05:23, Patrick Monette wrote:
> Did you inadvertently moved this?

Ah, that would be "git cl format" doing this for me! Moved to a separate block
to satisfy the new ordering logic as per this thread:

https://groups.google.com/a/chromium.org/forum/#!searchin/chromium-dev/includ...

[chrisha, 2017-02-06 20:55:17]

The CQ bit was checked by chrisha@chromium.org

[chrisha, 2017-02-06 20:55:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
pmonette@chromium.org
Link to the patchset: https://codereview.chromium.org/2653333002/#ps40001
(title: "Fix include order.")

[commit-bot: I haz the power, 2017-02-06 20:56:00]

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-02-07 00:39:44]

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

[commit-bot: I haz the power, 2017-02-07 00:39:45]

Try jobs failed on following builders:
  win10_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win10_chromium_x6...)

[Patrick Monette, 2017-02-07 02:13:47]

The CQ bit was checked by pmonette@chromium.org

[commit-bot: I haz the power, 2017-02-07 02:14:10]

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-02-07 03:34:25]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1486433627825390,
"parent_rev": "90c8bc34ef2ae379749a4c0c5a8a4fc55fe02056", "commit_rev":
"2b10cfea6a7bfad0a361aa79d66e7ede05eb397a"}

[commit-bot: I haz the power, 2017-02-07 03:34:59]

Description was changed from

==========
[win] Create utilities for helping to populate ModuleDatabase.

Further CLs will move (and test!) the rest of the enumerate_modules_model logic
to this collection of helper functions.

BUG=662084
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

to

==========
[win] Create utilities for helping to populate ModuleDatabase.

Further CLs will move (and test!) the rest of the enumerate_modules_model logic
to this collection of helper functions.

BUG=662084
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

Review-Url: https://codereview.chromium.org/2653333002
Cr-Commit-Position: refs/heads/master@{#448536}
Committed:
https://chromium.googlesource.com/chromium/src/+/2b10cfea6a7bfad0a361aa79d66e...
==========

[commit-bot: I haz the power, 2017-02-07 03:35:01]

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