Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(281)

Issue 2889913002: [subresource_filter] Remove Forwarding NavigationThrottles (Closed)

Created:
1 year, 1 month ago by Charlie Harrison
Modified:
1 year, 1 month ago
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] Remove Forwarding NavigationThrottles Now that all activation goes through the safe browsing throttle, we can just hook up everything there without doing any sort of persistence in the driver factory. BUG=708181 Review-Url: https://codereview.chromium.org/2889913002 Cr-Commit-Position: refs/heads/master@{#473082} Committed: https://chromium.googlesource.com/chromium/src/+/991638fa4c75139e0ce663e27c56299e9849faae

Patch Set 1 #

Patch Set 2 : Remove silly forwarding throttles #

Total comments: 6

Patch Set 3 : creis review #

Patch Set 4 : shivanisha review #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+104 lines, -199 lines) Patch
M components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h View 1 2 3 7 chunks +13 lines, -27 lines 0 comments Download
M components/subresource_filter/content/browser/content_subresource_filter_driver_factory.cc View 1 2 3 8 chunks +58 lines, -87 lines 0 comments Download
M components/subresource_filter/content/browser/content_subresource_filter_driver_factory_unittest.cc View 1 2 3 1 chunk +9 lines, -11 lines 0 comments Download
M components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.h View 1 1 chunk +0 lines, -4 lines 0 comments Download
M components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.cc View 1 2 chunks +0 lines, -35 lines 0 comments Download
M components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.cc View 1 2 3 2 chunks +13 lines, -8 lines 1 comment Download
M components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc View 1 2 2 chunks +0 lines, -27 lines 0 comments Download
M content/public/test/navigation_simulator.h View 1 2 2 chunks +6 lines, -0 lines 0 comments Download
M content/public/test/navigation_simulator.cc View 1 2 1 chunk +5 lines, -0 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 30 (17 generated)
Charlie Harrison
shivanisha, more refactoring :D PTAL? Let me know if you're getting overloaded and I can ...
1 year, 1 month ago (2017-05-17 02:13:41 UTC) #5
Charlie Reis
content/ LGTM with nits. https://codereview.chromium.org/2889913002/diff/20001/content/public/test/navigation_simulator.cc File content/public/test/navigation_simulator.cc (right): https://codereview.chromium.org/2889913002/diff/20001/content/public/test/navigation_simulator.cc#newcode487 content/public/test/navigation_simulator.cc:487: CHECK_EQ(state_, STARTED); nit: Reverse order, ...
1 year, 1 month ago (2017-05-17 22:39:24 UTC) #8
Charlie Harrison
Thanks Charlie https://codereview.chromium.org/2889913002/diff/20001/content/public/test/navigation_simulator.cc File content/public/test/navigation_simulator.cc (right): https://codereview.chromium.org/2889913002/diff/20001/content/public/test/navigation_simulator.cc#newcode487 content/public/test/navigation_simulator.cc:487: CHECK_EQ(state_, STARTED); On 2017/05/17 22:39:24, Charlie Reis ...
1 year, 1 month ago (2017-05-18 19:14:05 UTC) #10
shivanisha
lgtm % nit https://codereview.chromium.org/2889913002/diff/20001/components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h File components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h (right): https://codereview.chromium.org/2889913002/diff/20001/components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h#newcode90 components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h:90: // and |redirects| are saved. Need ...
1 year, 1 month ago (2017-05-18 19:54:21 UTC) #12
Charlie Harrison
https://codereview.chromium.org/2889913002/diff/20001/components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h File components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h (right): https://codereview.chromium.org/2889913002/diff/20001/components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h#newcode90 components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h:90: // and |redirects| are saved. On 2017/05/18 19:54:21, shivanisha ...
1 year, 1 month ago (2017-05-18 20:22:41 UTC) #14
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/2889913002/60001
1 year, 1 month ago (2017-05-18 20:24:51 UTC) #19
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/449004)
1 year, 1 month ago (2017-05-19 00:36:36 UTC) #21
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/2889913002/60001
1 year, 1 month ago (2017-05-19 00:55:49 UTC) #23
commit-bot: I haz the power
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/991638fa4c75139e0ce663e27c56299e9849faae
1 year, 1 month ago (2017-05-19 04:13:45 UTC) #26
Marc Treib
On 2017/05/19 04:13:45, commit-bot: I haz the power wrote: > Committed patchset #4 (id:60001) as ...
1 year, 1 month ago (2017-05-19 10:10:43 UTC) #27
Marc Treib
A revert of this CL (patchset #4 id:60001) has been created in https://codereview.chromium.org/2897613002/ by treib@chromium.org. ...
1 year, 1 month ago (2017-05-19 10:11:33 UTC) #28
Charlie Harrison
https://codereview.chromium.org/2889913002/diff/60001/components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.cc File components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.cc (right): https://codereview.chromium.org/2889913002/diff/60001/components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.cc#newcode137 components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.cc:137: check_results_.back(); Here's the issue, we call back() on an ...
1 year, 1 month ago (2017-05-19 11:15:41 UTC) #29
findit-for-me
1 year, 1 month ago (2017-05-19 12:47:27 UTC) #30
Message was sent while issue was closed.
Findit (https://goo.gl/kROfz5) identified this CL at revision 473082 as the
culprit for
failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3Itb...

Powered by Google App Engine
This is Rietveld 408576698