[subresource_filter] Remove Forwarding NavigationThrottles

[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

[Charlie Harrison, 2017-05-17 02:05:34]

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

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

Dry run: CQ is trying da patch.

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

[Charlie Harrison, 2017-05-17 02:07:17]

Description was changed from

==========
[subresource_filter] Remove URL matching persistence in the driver factory

Now that all activation goes through the safe browsing throttle, we can
just hook up everything there.

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

to

==========
[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
==========

[Charlie Harrison, 2017-05-17 02:13:40]

csharrison@chromium.org changed reviewers:
+ creis@chromium.org, shivanisha@chromium.org

[Charlie Harrison, 2017-05-17 02:13:41]

shivanisha, more refactoring :D PTAL? Let me know if you're getting overloaded
and I can give this one to jkarlin.

creis: PTAL at simple addition to NavigationSimulator API

[commit-bot: I haz the power, 2017-05-17 07:15:04]

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

[commit-bot: I haz the power, 2017-05-17 07:15:05]

Dry run: This issue passed the CQ dry run.

[Charlie Reis, 2017-05-17 22:39:24]

content/ LGTM with nits.

https://codereview.chromium.org/2889913002/diff/20001/content/public/test/nav...
File content/public/test/navigation_simulator.cc (right):

https://codereview.chromium.org/2889913002/diff/20001/content/public/test/nav...
content/public/test/navigation_simulator.cc:487: CHECK_EQ(state_, STARTED);
nit: Reverse order, as on line 470 (expected, actual).

https://codereview.chromium.org/2889913002/diff/20001/content/public/test/nav...
File content/public/test/navigation_simulator.h (right):

https://codereview.chromium.org/2889913002/diff/20001/content/public/test/nav...
content/public/test/navigation_simulator.h:149: virtual NavigationHandle*
GetNavigationHandle() const;
nit: Please document all methods in content/public.

[Charlie Harrison, 2017-05-18 19:13:41]

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

[Charlie Harrison, 2017-05-18 19:14:05]

Thanks Charlie

https://codereview.chromium.org/2889913002/diff/20001/content/public/test/nav...
File content/public/test/navigation_simulator.cc (right):

https://codereview.chromium.org/2889913002/diff/20001/content/public/test/nav...
content/public/test/navigation_simulator.cc:487: CHECK_EQ(state_, STARTED);
On 2017/05/17 22:39:24, Charlie Reis wrote:
> nit: Reverse order, as on line 470 (expected, actual).

Done.

https://codereview.chromium.org/2889913002/diff/20001/content/public/test/nav...
File content/public/test/navigation_simulator.h (right):

https://codereview.chromium.org/2889913002/diff/20001/content/public/test/nav...
content/public/test/navigation_simulator.h:149: virtual NavigationHandle*
GetNavigationHandle() const;
On 2017/05/17 22:39:24, Charlie Reis wrote:
> nit: Please document all methods in content/public.

Done.

[commit-bot: I haz the power, 2017-05-18 19:14:42]

Dry run: CQ is trying da patch.

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

[shivanisha, 2017-05-18 19:54:21]

lgtm % nit

https://codereview.chromium.org/2889913002/diff/20001/components/subresource_...
File
components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h
(right):

https://codereview.chromium.org/2889913002/diff/20001/components/subresource_...
components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h:90:
// and |redirects| are saved.
Need to change the comment since |url| is no longer an argument

[Charlie Harrison, 2017-05-18 20:22:35]

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

[Charlie Harrison, 2017-05-18 20:22:41]

https://codereview.chromium.org/2889913002/diff/20001/components/subresource_...
File
components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h
(right):

https://codereview.chromium.org/2889913002/diff/20001/components/subresource_...
components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h:90:
// and |redirects| are saved.
On 2017/05/18 19:54:21, shivanisha wrote:
> Need to change the comment since |url| is no longer an argument

Yeah actually the whole comment is wrong :/ I've changed the name to
OnSafeBrowsingMatchComputed and updated the comment.

[commit-bot: I haz the power, 2017-05-18 20:23:48]

Dry run: CQ is trying da patch.

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

[Charlie Harrison, 2017-05-18 20:24:21]

The CQ bit was unchecked by csharrison@chromium.org

[Charlie Harrison, 2017-05-18 20:24:22]

The CQ bit was checked by csharrison@chromium.org

[Charlie Harrison, 2017-05-18 20:24:22]

The patchset sent to the CQ was uploaded after l-g-t-m from creis@chromium.org,
shivanisha@chromium.org
Link to the patchset: https://codereview.chromium.org/2889913002/#ps60001
(title: "shivanisha review")

[commit-bot: I haz the power, 2017-05-18 20:24:51]

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-05-19 00:36:35]

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

[commit-bot: I haz the power, 2017-05-19 00:36:36]

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_...)

[Charlie Harrison, 2017-05-19 00:54:22]

The CQ bit was checked by csharrison@chromium.org

[commit-bot: I haz the power, 2017-05-19 00:55:49]

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-05-19 04:13:29]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1495155262418350,
"parent_rev": "2e65213dd03a7a0f277c5d16d460ae22d391f33c", "commit_rev":
"991638fa4c75139e0ce663e27c56299e9849faae"}

[commit-bot: I haz the power, 2017-05-19 04:13:41]

Description was changed from

==========
[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
==========

to

==========
[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/+/991638fa4c75139e0ce663e27c56...
==========

[commit-bot: I haz the power, 2017-05-19 04:13:45]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/991638fa4c75139e0ce663e27c56...

[Marc Treib, 2017-05-19 10:10:43]

On 2017/05/19 04:13:45, commit-bot: I haz the power wrote:
> Committed patchset #4 (id:60001) as
>
https://chromium.googlesource.com/chromium/src/+/991638fa4c75139e0ce663e27c56...

This seems to introduce an access into an empty vector on ChromiumOS dbg, making
lots of tests crash. See
https://uberchromegw.corp.google.com/i/chromium.chromiumos/builders/Linux%20C...

Example output:

../../build/linux/debian_jessie_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/debug/vector:376:
    error: attempt to access an element in an empty container.
Objects involved in the operation:
sequence "this" @ 0x0x6eae7e6c5b8 {
}
Received signal 6
#0 0x7f5aee03864b base::debug::StackTrace::StackTrace()
#1 0x7f5aee03738c base::debug::StackTrace::StackTrace()
#2 0x7f5aee03815f base::debug::(anonymous namespace)::StackDumpSignalHandler()
#3 0x7f5af1414330 <unknown>
#4 0x7f5ad4abcc37 gsignal
#5 0x7f5ad4ac0028 abort
#6 0x7f5ad5113fe5 __gnu_debug::_Error_formatter::_M_error()
#7 0x0000069352af std::__debug::vector<>::back()
#8 0x00000693489d
subresource_filter::SubresourceFilterSafeBrowsingActivationThrottle::NotifyResult()
#9 0x000006934773
subresource_filter::SubresourceFilterSafeBrowsingActivationThrottle::WillProcessResponse()
#10 0x7f5ae984b6d2 content::NavigationHandleImpl::CheckWillProcessResponse()
#11 0x7f5ae984d54a content::NavigationHandleImpl::WillProcessResponse()
#12 0x7f5ae9ab8765 content::(anonymous
namespace)::WillProcessResponseOnUIThread()
#13 0x7f5ae9abe6e3
_ZN4base8internal13FunctorTraitsIPFvNS_8CallbackIFvN7content18NavigationThrottle19ThrottleCheckResultEELNS0_8CopyModeE1ELNS0_10RepeatModeE1EEEii13scoped_refptrIN3net19HttpResponseHeadersEENSB_16HttpResponseInfo14ConnectionInfoERKNS3_9SSLStatusERKNS3_15GlobalRequestIDEbbbRKNS2_IFvvELS7_1ELS8_1EEESt10unique_ptrINS3_14NavigationDataESt14default_deleteISR_EEEvE6InvokeIJRKS9_RKiS12_RKSD_RKSF_SI_SL_RKbS18_S18_SP_SU_EEEvSW_DpOT_
#14 0x7f5ae9abe2df
_ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIRKPFvNS_8CallbackIFvN7content18NavigationThrottle19ThrottleCheckResultEELNS0_8CopyModeE1ELNS0_10RepeatModeE1EEEii13scoped_refptrIN3net19HttpResponseHeadersEENSD_16HttpResponseInfo14ConnectionInfoERKNS5_9SSLStatusERKNS5_15GlobalRequestIDEbbbRKNS4_IFvvELS9_1ELSA_1EEESt10unique_ptrINS5_14NavigationDataESt14default_deleteIST_EEEJRKSB_RKiS14_RKSF_RKSH_SK_SN_RKbS1A_S1A_SR_SW_EEEvOT_DpOT0_
#15 0x7f5ae9abe117
_ZN4base8internal7InvokerINS0_9BindStateIPFvNS_8CallbackIFvN7content18NavigationThrottle19ThrottleCheckResultEELNS0_8CopyModeE1ELNS0_10RepeatModeE1EEEii13scoped_refptrIN3net19HttpResponseHeadersEENSC_16HttpResponseInfo14ConnectionInfoERKNS4_9SSLStatusERKNS4_15GlobalRequestIDEbbbRKNS3_IFvvELS8_1ELS9_1EEESt10unique_ptrINS4_14NavigationDataESt14default_deleteISS_EEEJSA_iiSE_SG_SH_SK_bbbSO_NS0_13PassedWrapperISV_EEEEESN_E7RunImplIRKSX_RKSt5tupleIJSA_iiSE_SG_SH_SK_bbbSO_SZ_EEJLm0ELm1ELm2ELm3ELm4ELm5ELm6ELm7ELm8ELm9ELm10ELm11EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
#16 0x7f5ae9abdd1c
_ZN4base8internal7InvokerINS0_9BindStateIPFvNS_8CallbackIFvN7content18NavigationThrottle19ThrottleCheckResultEELNS0_8CopyModeE1ELNS0_10RepeatModeE1EEEii13scoped_refptrIN3net19HttpResponseHeadersEENSC_16HttpResponseInfo14ConnectionInfoERKNS4_9SSLStatusERKNS4_15GlobalRequestIDEbbbRKNS3_IFvvELS8_1ELS9_1EEESt10unique_ptrINS4_14NavigationDataESt14default_deleteISS_EEEJSA_iiSE_SG_SH_SK_bbbSO_NS0_13PassedWrapperISV_EEEEESN_E3RunEPNS0_13BindStateBaseE
#17 0x7f5aedff7a8e
_ZNO4base8CallbackIFvvELNS_8internal8CopyModeE0ELNS2_10RepeatModeE0EE3RunEv
#18 0x7f5aee03db41 base::debug::TaskAnnotator::RunTask()
#19 0x7f5aee0c82be base::MessageLoop::RunTask()
#20 0x7f5aee0c8527 base::MessageLoop::DeferOrRunPendingTask()
#21 0x7f5aee0c877d base::MessageLoop::DoWork()
#22 0x7f5aee0d9956 base::MessagePumpGlib::Run()
#23 0x7f5aee0c7d93 base::MessageLoop::Run()
#24 0x7f5aee166bd7 base::RunLoop::Run()
#25 0x000003aa57b6 content::RunThisRunLoop()
#26 0x000003aa6411 content::MessageLoopRunner::Run()
#27 0x000003aa2f84 content::TestNavigationObserver::Wait()
#28 0x000002c9488b
ui_test_utils::NavigateToURLWithDispositionBlockUntilNavigationsComplete()
#29 0x000002c943d1 ui_test_utils::NavigateToURLWithDisposition()
#30 0x000002c94397 ui_test_utils::NavigateToURL()
#31 0x0000007d0216 SitePerProcessTextInputManagerTest::CreateIframePage()
#32 0x0000007c6844
SitePerProcessTextInputManagerTest_ClosingTabWillNotCrash_Test::RunTestOnMainThread()
#33 0x000003a76767 content::BrowserTestBase::ProxyRunTestOnMainThreadLoop()
#34 0x00000072d3d5
_ZN4base8internal13FunctorTraitsIM26WebViewInteractiveTestBaseFvvEvE6InvokeIP66WebViewDragDropInteractiveTest_DISABLED_DragDropWithinWebView_TestJEEEvS4_OT_DpOT0_
#35 0x00000072d2e1
_ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIM26WebViewInteractiveTestBaseFvvEJPS4_EEEvOT_DpOT0_
#36 0x000003a78357
_ZN4base8internal7InvokerINS0_9BindStateIMN7content15BrowserTestBaseEFvvEJNS0_17UnretainedWrapperIS4_EEEEEFvvEE7RunImplIRKS6_RKSt5tupleIJS8_EEJLm0EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
#37 0x000003a7829c
_ZN4base8internal7InvokerINS0_9BindStateIMN7content15BrowserTestBaseEFvvEJNS0_17UnretainedWrapperIS4_EEEEEFvvEE3RunEPNS0_13BindStateBaseE
#38 0x00000072f08d
_ZNKR4base8CallbackIFvvELNS_8internal8CopyModeE1ELNS2_10RepeatModeE1EE3RunEv
#39 0x000002cb865e ChromeBrowserMainParts::PreMainMessageLoopRunImpl()
#40 0x000002cb72d0 ChromeBrowserMainParts::PreMainMessageLoopRun()
#41 0x0000018a1b07
chromeos::ChromeBrowserMainPartsChromeos::PreMainMessageLoopRun()
#42 0x7f5ae95034c1 content::BrowserMainLoop::PreMainMessageLoopRun()
#43 0x7f5ae8839895
_ZN4base8internal13FunctorTraitsIMN7content13URLLoaderImplEFvvEvE6InvokeIPS3_JEEEvS5_OT_DpOT0_
#44 0x7f5ae950c4c1
_ZN4base8internal12InvokeHelperILb0EiE8MakeItSoIRKMN7content15BrowserMainLoopEFivEJPS5_EEEiOT_DpOT0_
#45 0x7f5ae950c467
_ZN4base8internal7InvokerINS0_9BindStateIMN7content15BrowserMainLoopEFivEJNS0_17UnretainedWrapperIS4_EEEEEFivEE7RunImplIRKS6_RKSt5tupleIJS8_EEJLm0EEEEiOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
#46 0x7f5ae950c3ac
_ZN4base8internal7InvokerINS0_9BindStateIMN7content15BrowserMainLoopEFivEJNS0_17UnretainedWrapperIS4_EEEEEFivEE3RunEPNS0_13BindStateBaseE
#47 0x7f5ae880d7bd
_ZNKR4base8CallbackIFvvELNS_8internal8CopyModeE1ELNS2_10RepeatModeE1EE3RunEv
#48 0x7f5aea080ccb content::StartupTaskRunner::RunAllTasksNow()
#49 0x7f5ae9501050 content::BrowserMainLoop::CreateStartupTasks()
#50 0x7f5ae950f8f7 content::BrowserMainRunnerImpl::Initialize()
#51 0x7f5ae94fd63f content::BrowserMain()
#52 0x7f5aead706a6 content::RunNamedProcessTypeMain()
#53 0x7f5aead7278f content::ContentMainRunnerImpl::Run()
#54 0x7f5aead6f33a
content::ContentServiceManagerMainDelegate::RunEmbedderProcess()
#55 0x7f5ad80ced47 service_manager::Main()
#56 0x7f5aead701cb content::ContentMain()
#57 0x000003a7648f content::BrowserTestBase::SetUp()
#58 0x000002c8f85c InProcessBrowserTest::SetUp()
#59 0x000000a1744e testing::internal::HandleSehExceptionsInMethodIfSupported<>()
#60 0x000000a07d62 testing::internal::HandleExceptionsInMethodIfSupported<>()
#61 0x0000009fc9c4 testing::Test::Run()
  r8: 00007f5ad4e499d0  r9: 00007fff589d99e8 r10: 0000000000000008 r11:
0000000000000202
 r12: 0000000000000001 r13: 00007fff589e0700 r14: 000006eae7aeff00 r15:
0000000000000000
  di: 0000000000004719  si: 0000000000004719  bp: 00007fff589d9c70  bx:
00007fff589d9c28
  dx: 0000000000000006  ax: 0000000000000000  cx: ffffffffffffffff  sp:
00007fff589d9a38
  ip: 00007f5ad4abcc37 efl: 0000000000000202 cgf: 0000000000000033 erf:
0000000000000000
 trp: 0000000000000000 msk: 0000000000000000 cr2: 0000000000000000
[end of stack trace]

[Marc Treib, 2017-05-19 10:11:33]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/2897613002/ by treib@chromium.org.

The reason for reverting is: Seems to introduce an access into an empty vector
on ChromiumOS dbg, making lots of tests crash. See
https://uberchromegw.corp.google.com/i/chromium.chromiumos/builders/Linux%20C....

[Charlie Harrison, 2017-05-19 11:15:41]

https://codereview.chromium.org/2889913002/diff/60001/components/subresource_...
File
components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.cc
(right):

https://codereview.chromium.org/2889913002/diff/60001/components/subresource_...
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 empty vector. Let me add more unit tests
for this class in pass-through mode.

[findit-for-me, 2017-05-19 12:47:27]

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...