Make call sites of subresource_filter::GetActiveConfigurations const-correct.

Clean up subresource_filter::GetActiveConfigurations.

Make call sites const-correct, and also make the ConfigurationList(Configuration)
constructor explicit.

BUG=708181
Review-Url: https://codereview.chromium.org/2837163004
Cr-Commit-Position: refs/heads/master@{#467352}
Committed: https://chromium.googlesource.com/chromium/src/+/2c33559b6bb6c8e574063e0f23520125f08bd393

[engedy, 2017-04-25 08:30:23]

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

[commit-bot: I haz the power, 2017-04-25 08:30:39]

Dry run: CQ is trying da patch.

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

[engedy, 2017-04-25 08:30:52]

Description was changed from

==========
Make call sites of subresource_filter::GetActiveConfigurations const-correct.

Also make the ConfigurationList(Configuration) constructor explicit.

BUG=708181
==========

to

==========
Make call sites of subresource_filter::GetActiveConfigurations const-correct.

Also make the ConfigurationList(Configuration) constructor explicit.

BUG=708181
==========

[engedy, 2017-04-25 08:30:52]

engedy@chromium.org changed reviewers:
+ pkalinnikov@chromium.org, sorin@chromium.org

[engedy, 2017-04-25 08:31:31]

Description was changed from

==========
Make call sites of subresource_filter::GetActiveConfigurations const-correct.

Also make the ConfigurationList(Configuration) constructor explicit.

BUG=708181
==========

to

==========
Clean up subresource_filter::GetActiveConfigurations.

Make call sites const-correct, and also make the
ConfigurationList(Configuration)
constructor explicit.

BUG=708181
==========

[engedy, 2017-04-25 09:07:52]

@Pavel, @Sorin, please take a look.

(Sorin, this addresses your comments on
https://codereview.chromium.org/2831373002/.)

[commit-bot: I haz the power, 2017-04-25 09:42:51]

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

[commit-bot: I haz the power, 2017-04-25 09:42:52]

Dry run: This issue passed the CQ dry run.

[engedy, 2017-04-25 19:06:42]

engedy@chromium.org changed reviewers:
+ csharrison@chromium.org
- pkalinnikov@chromium.org

[engedy, 2017-04-25 19:06:55]

@Charlie, could you please take a quick look?

[Charlie Harrison, 2017-04-25 19:13:06]

LGTM

[pkalinnikov, 2017-04-26 09:20:18]

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

[pkalinnikov, 2017-04-26 09:20:19]

LGTM % minor nits/questions.

https://codereview.chromium.org/2837163004/diff/1/chrome/browser/component_up...
File chrome/browser/component_updater/subresource_filter_component_installer.cc
(right):

https://codereview.chromium.org/2837163004/diff/1/chrome/browser/component_up...
chrome/browser/component_updater/subresource_filter_component_installer.cc:116:
const std::string ruleset_flavor =
nit: Why not const ref? Seems all the way the same (but copy instead of
copy+move), except when the flow goes down to return "invalid". Unless I am
missing some RVO subtleties.

https://codereview.chromium.org/2837163004/diff/1/chrome/browser/component_up...
chrome/browser/component_updater/subresource_filter_component_installer.cc:123:
ruleset_flavor.at(0) <= 'd')
I vaguely remember that we had a rule to put a statement into braces if the
condition spans multiple lines. Unfortunately, I couldn't find it in the
guidelines. Am I missing something?

https://codereview.chromium.org/2837163004/diff/1/chrome/browser/component_up...
chrome/browser/component_updater/subresource_filter_component_installer.cc:148:
subresource_filter::kSafeBrowsingSubresourceFilter))
Same as above.

[engedy, 2017-04-26 09:58:20]

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

[commit-bot: I haz the power, 2017-04-26 09:58:28]

Dry run: CQ is trying da patch.

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

[engedy, 2017-04-26 10:00:32]

https://codereview.chromium.org/2837163004/diff/1/chrome/browser/component_up...
File chrome/browser/component_updater/subresource_filter_component_installer.cc
(right):

https://codereview.chromium.org/2837163004/diff/1/chrome/browser/component_up...
chrome/browser/component_updater/subresource_filter_component_installer.cc:116:
const std::string ruleset_flavor =
On 2017/04/26 09:20:19, pkalinnikov wrote:
> nit: Why not const ref? Seems all the way the same (but copy instead of
> copy+move), except when the flow goes down to return "invalid". Unless I am
> missing some RVO subtleties.

No reason, I just didn't realize that a reference can be taken here. Done.

https://codereview.chromium.org/2837163004/diff/1/chrome/browser/component_up...
chrome/browser/component_updater/subresource_filter_component_installer.cc:123:
ruleset_flavor.at(0) <= 'd')
On 2017/04/26 09:20:19, pkalinnikov wrote:
> I vaguely remember that we had a rule to put a statement into braces if the
> condition spans multiple lines. Unfortunately, I couldn't find it in the
> guidelines. Am I missing something?

I, too, am unable to find it, and I could not find any announcements about a
change here. Added curlies for good measure.

https://codereview.chromium.org/2837163004/diff/1/chrome/browser/component_up...
chrome/browser/component_updater/subresource_filter_component_installer.cc:148:
subresource_filter::kSafeBrowsingSubresourceFilter))
On 2017/04/26 09:20:18, pkalinnikov wrote:
> Same as above.

Done.

[commit-bot: I haz the power, 2017-04-26 10:41:04]

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

[commit-bot: I haz the power, 2017-04-26 10:41:04]

Dry run: This issue passed the CQ dry run.

[pkalinnikov, 2017-04-26 12:01:57]

Thanks, still LGTM.

[Sorin Jianu, 2017-04-26 16:54:40]

lgtm

thank you!

[engedy, 2017-04-26 16:56:16]

The CQ bit was checked by engedy@chromium.org

[engedy, 2017-04-26 16:56:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
csharrison@chromium.org
Link to the patchset: https://codereview.chromium.org/2837163004/#ps20001
(title: "Addressed comments from pkalinnikov@.")

[commit-bot: I haz the power, 2017-04-26 16:56:40]

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-04-26 17:03:56]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493225776618920,
"parent_rev": "de1aa71cbaa27e66039ca786106b66cb3ebf0965", "commit_rev":
"2c33559b6bb6c8e574063e0f23520125f08bd393"}

[commit-bot: I haz the power, 2017-04-26 17:04:08]

Description was changed from

==========
Clean up subresource_filter::GetActiveConfigurations.

Make call sites const-correct, and also make the
ConfigurationList(Configuration)
constructor explicit.

BUG=708181
==========

to

==========
Clean up subresource_filter::GetActiveConfigurations.

Make call sites const-correct, and also make the
ConfigurationList(Configuration)
constructor explicit.

BUG=708181

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

[commit-bot: I haz the power, 2017-04-26 17:04:09]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/2c33559b6bb6c8e574063e0f2352...