Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(320)

Issue 2887253004: [subresource_filter] Move driver factory tests to the SB throttle (Closed)

Created:
3 years, 7 months ago by Charlie Harrison
Modified:
3 years, 7 months ago
Reviewers:
jkarlin
CC:
chromium-reviews, darin-cc_chromium.org, jam, subresource-filter-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[subresource_filter] Move driver factory tests to the SB throttle BUG=717590 Review-Url: https://codereview.chromium.org/2887253004 Cr-Commit-Position: refs/heads/master@{#474000} Committed: https://chromium.googlesource.com/chromium/src/+/fb7da5949a9529ba0b63ab06e76cd4a8c351db26

Patch Set 1 #

Patch Set 2 : rebase #

Patch Set 3 : rebase #

Patch Set 4 : rebase #

Patch Set 5 : VERBATIM TEST COPY #

Patch Set 6 : FORWARD DIFF from 4 #

Total comments: 8

Patch Set 7 : jkarlin review #

Total comments: 2

Patch Set 8 : enum class : int #

Messages

Total messages: 36 (28 generated)
Charlie Harrison
Josh, PTAL I tried to do what you asked with patch 4->5 to see the ...
3 years, 7 months ago (2017-05-22 17:32:25 UTC) #16
jkarlin
Thanks. Admittedly not the closest ever inspection of the code but overall lgtm. https://codereview.chromium.org/2887253004/diff/100001/components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc File ...
3 years, 7 months ago (2017-05-23 13:04:33 UTC) #21
Charlie Harrison
https://codereview.chromium.org/2887253004/diff/100001/components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc File components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc (right): https://codereview.chromium.org/2887253004/diff/100001/components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc#newcode192 components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc:192: ruleset_dealer_.reset(); On 2017/05/23 13:04:33, jkarlin wrote: > Why this? ...
3 years, 7 months ago (2017-05-23 15:32:23 UTC) #23
jkarlin
https://codereview.chromium.org/2887253004/diff/100001/components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc File components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc (right): https://codereview.chromium.org/2887253004/diff/100001/components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc#newcode646 components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc:646: test_data.activation_level)); On 2017/05/23 15:32:23, Charlie Harrison wrote: > On ...
3 years, 7 months ago (2017-05-23 15:40:26 UTC) #25
Charlie Harrison
thanks https://codereview.chromium.org/2887253004/diff/100001/components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc File components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc (right): https://codereview.chromium.org/2887253004/diff/100001/components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc#newcode646 components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc:646: test_data.activation_level)); On 2017/05/23 15:40:25, jkarlin wrote: > On ...
3 years, 7 months ago (2017-05-23 16:19:37 UTC) #27
jkarlin
slgtm
3 years, 7 months ago (2017-05-23 16:21:11 UTC) #29
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2887253004/140001
3 years, 7 months ago (2017-05-23 16:22:20 UTC) #33
commit-bot: I haz the power
3 years, 7 months ago (2017-05-23 18:47:13 UTC) #36
Message was sent while issue was closed.
Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/fb7da5949a9529ba0b63ab06e76c...

Powered by Google App Engine
This is Rietveld 408576698