Fix SubResourceFilter to work with PlzNavigate.

Fix SubResourceFilter to work with PlzNavigate.

There are two main fixes:
-use the new NavigationHandle callbacks on WebContentsObserver which are compatible with PlzNavigate
-update the renderer side so that handles the new order of IPCs: while ReadyToCommitNavigation means the browser side can send its IPC before the internal content ones, with PlzNavigate one content IPC will fire both DidStartProvisionalLoad & DidCommitProvisionalLoad. So add extra logic to the renderer side to handle this.

This fixes
SubresourceFilterBrowserTest.MainFrameActivation
SubresourceFilterBrowserTest.MainFrameActivationOnStartup
SubresourceFilterBrowserTest.SubFrameActivation
with PlzNavigate.

BUG=504347, 653807
Committed: https://crrev.com/8ab445a553266b0b6b6f4eaa6976c6f8e2b4a7b6
Cr-Commit-Position: refs/heads/master@{#427085}

[jam, 2016-10-23 08:23:13]

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

[commit-bot: I haz the power, 2016-10-23 08:23:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jam, 2016-10-23 08:24:50]

Description was changed from

==========
Fix SubResourceFilter to work with PlzNavigate

BUG=504347
==========

to

==========
Fix SubResourceFilter to work with PlzNavigate.

This fixes
SubresourceFilterBrowserTest.MainFrameActivation
SubresourceFilterBrowserTest.MainFrameActivationOnStartup
SubresourceFilterBrowserTest.SubFrameActivation
with PlzNavigate.

BUG=504347
==========

[commit-bot: I haz the power, 2016-10-23 09:03:57]

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

[commit-bot: I haz the power, 2016-10-23 09:03:58]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jam, 2016-10-24 01:44:02]

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

[commit-bot: I haz the power, 2016-10-24 01:44:07]

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, 2016-10-24 02:38:11]

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

[commit-bot: I haz the power, 2016-10-24 02:38:12]

Dry run: This issue passed the CQ dry run.

[jam, 2016-10-24 04:33:26]

jam@chromium.org changed reviewers:
+ melandory@chromium.org

[jam, 2016-10-24 04:45:29]

Description was changed from

==========
Fix SubResourceFilter to work with PlzNavigate.

This fixes
SubresourceFilterBrowserTest.MainFrameActivation
SubresourceFilterBrowserTest.MainFrameActivationOnStartup
SubresourceFilterBrowserTest.SubFrameActivation
with PlzNavigate.

BUG=504347
==========

to

==========
Fix SubResourceFilter to work with PlzNavigate.

There are two main fixes:
-use the new NavigationHandle callbacks on WebContentsObserver which are
compatible with PlzNavigate
-update the renderer side so that handles the new order of IPCs: while
ReadyToCommitNavigation means the browser side can send its IPC before the
internal content ones, with PlzNavigate one content IPC will fire both
DidStartProvisionalLoad & DidCommitProvisionalLoad. So add extra logic to the
renderer side to handle this.

This fixes
SubresourceFilterBrowserTest.MainFrameActivation
SubresourceFilterBrowserTest.MainFrameActivationOnStartup
SubresourceFilterBrowserTest.SubFrameActivation
with PlzNavigate.

BUG=504347
==========

[jam, 2016-10-24 04:51:20]

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

https://codereview.chromium.org/2446503002/diff/20001/components/subresource_...
components/subresource_filter/content/browser/content_subresource_filter_driver_factory.cc:75:
return whitelisted_hosts_.find(url.host()) != whitelisted_hosts_.end();
I hope you don't mind, but I removed the accessor here and below because they
were unnecessary (they make debugging less convenient since one can't just hover
over the variables to see their contents in an IDE).

https://codereview.chromium.org/2446503002/diff/20001/components/subresource_...
File components/subresource_filter/content/renderer/subresource_filter_agent.cc
(right):

https://codereview.chromium.org/2446503002/diff/20001/components/subresource_...
components/subresource_filter/content/renderer/subresource_filter_agent.cc:152:
OnActivateForProvisionalLoad)
nit: changed this to match convention for IPC message handlers

[melandory, 2016-10-24 08:55:42]

On 2016/10/24 04:51:20, jam wrote:
>
https://codereview.chromium.org/2446503002/diff/20001/components/subresource_...
> File
>
components/subresource_filter/content/browser/content_subresource_filter_driver_factory.cc
> (right):
> 
>
https://codereview.chromium.org/2446503002/diff/20001/components/subresource_...
>
components/subresource_filter/content/browser/content_subresource_filter_driver_factory.cc:75:
> return whitelisted_hosts_.find(url.host()) != whitelisted_hosts_.end();
> I hope you don't mind, but I removed the accessor here and below because they
> were unnecessary (they make debugging less convenient since one can't just
hover
> over the variables to see their contents in an IDE).
> 
>
https://codereview.chromium.org/2446503002/diff/20001/components/subresource_...
> File
components/subresource_filter/content/renderer/subresource_filter_agent.cc
> (right):
> 
>
https://codereview.chromium.org/2446503002/diff/20001/components/subresource_...
>
components/subresource_filter/content/renderer/subresource_filter_agent.cc:152:
> OnActivateForProvisionalLoad)
> nit: changed this to match convention for IPC message handlers

lgtm, thanks a lot for the change.

[melandory, 2016-10-24 09:00:13]

On 2016/10/24 08:55:42, melandory wrote:
> On 2016/10/24 04:51:20, jam wrote:
> >
>
https://codereview.chromium.org/2446503002/diff/20001/components/subresource_...
> > File
> >
>
components/subresource_filter/content/browser/content_subresource_filter_driver_factory.cc
> > (right):
> > 
> >
>
https://codereview.chromium.org/2446503002/diff/20001/components/subresource_...
> >
>
components/subresource_filter/content/browser/content_subresource_filter_driver_factory.cc:75:
> > return whitelisted_hosts_.find(url.host()) != whitelisted_hosts_.end();
> > I hope you don't mind, but I removed the accessor here and below because
they
> > were unnecessary (they make debugging less convenient since one can't just
> hover
> > over the variables to see their contents in an IDE).
> > 
> >
>
https://codereview.chromium.org/2446503002/diff/20001/components/subresource_...
> > File
> components/subresource_filter/content/renderer/subresource_filter_agent.cc
> > (right):
> > 
> >
>
https://codereview.chromium.org/2446503002/diff/20001/components/subresource_...
> >
>
components/subresource_filter/content/renderer/subresource_filter_agent.cc:152:
> > OnActivateForProvisionalLoad)
> > nit: changed this to match convention for IPC message handlers
> 
> lgtm, thanks a lot for the change.

Minor note: could you please add 653807 to the bug list. Thanks!

[jam, 2016-10-24 14:57:03]

Description was changed from

==========
Fix SubResourceFilter to work with PlzNavigate.

There are two main fixes:
-use the new NavigationHandle callbacks on WebContentsObserver which are
compatible with PlzNavigate
-update the renderer side so that handles the new order of IPCs: while
ReadyToCommitNavigation means the browser side can send its IPC before the
internal content ones, with PlzNavigate one content IPC will fire both
DidStartProvisionalLoad & DidCommitProvisionalLoad. So add extra logic to the
renderer side to handle this.

This fixes
SubresourceFilterBrowserTest.MainFrameActivation
SubresourceFilterBrowserTest.MainFrameActivationOnStartup
SubresourceFilterBrowserTest.SubFrameActivation
with PlzNavigate.

BUG=504347
==========

to

==========
Fix SubResourceFilter to work with PlzNavigate.

There are two main fixes:
-use the new NavigationHandle callbacks on WebContentsObserver which are
compatible with PlzNavigate
-update the renderer side so that handles the new order of IPCs: while
ReadyToCommitNavigation means the browser side can send its IPC before the
internal content ones, with PlzNavigate one content IPC will fire both
DidStartProvisionalLoad & DidCommitProvisionalLoad. So add extra logic to the
renderer side to handle this.

This fixes
SubresourceFilterBrowserTest.MainFrameActivation
SubresourceFilterBrowserTest.MainFrameActivationOnStartup
SubresourceFilterBrowserTest.SubFrameActivation
with PlzNavigate.

BUG=504347, 653807
==========

[jam, 2016-10-24 14:57:10]

The CQ bit was checked by jam@chromium.org

[commit-bot: I haz the power, 2016-10-24 14:57:28]

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

[jam, 2016-10-24 15:12:42]

jam@chromium.org changed reviewers:
+ tsepez@chromium.org

[jam, 2016-10-24 15:12:42]

+Tom for subresource_filter_messages.h

[commit-bot: I haz the power, 2016-10-24 15:21:34]

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

[commit-bot: I haz the power, 2016-10-24 15:21:35]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Tom Sepez, 2016-10-24 16:12:00]

lgtm

[jam, 2016-10-24 16:20:29]

The CQ bit was checked by jam@chromium.org

[commit-bot: I haz the power, 2016-10-24 16:20: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, 2016-10-24 16:35:13]

Description was changed from

==========
Fix SubResourceFilter to work with PlzNavigate.

There are two main fixes:
-use the new NavigationHandle callbacks on WebContentsObserver which are
compatible with PlzNavigate
-update the renderer side so that handles the new order of IPCs: while
ReadyToCommitNavigation means the browser side can send its IPC before the
internal content ones, with PlzNavigate one content IPC will fire both
DidStartProvisionalLoad & DidCommitProvisionalLoad. So add extra logic to the
renderer side to handle this.

This fixes
SubresourceFilterBrowserTest.MainFrameActivation
SubresourceFilterBrowserTest.MainFrameActivationOnStartup
SubresourceFilterBrowserTest.SubFrameActivation
with PlzNavigate.

BUG=504347, 653807
==========

to

==========
Fix SubResourceFilter to work with PlzNavigate.

There are two main fixes:
-use the new NavigationHandle callbacks on WebContentsObserver which are
compatible with PlzNavigate
-update the renderer side so that handles the new order of IPCs: while
ReadyToCommitNavigation means the browser side can send its IPC before the
internal content ones, with PlzNavigate one content IPC will fire both
DidStartProvisionalLoad & DidCommitProvisionalLoad. So add extra logic to the
renderer side to handle this.

This fixes
SubresourceFilterBrowserTest.MainFrameActivation
SubresourceFilterBrowserTest.MainFrameActivationOnStartup
SubresourceFilterBrowserTest.SubFrameActivation
with PlzNavigate.

BUG=504347, 653807
==========

[commit-bot: I haz the power, 2016-10-24 16:35:14]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-24 16:50:02]

Description was changed from

==========
Fix SubResourceFilter to work with PlzNavigate.

There are two main fixes:
-use the new NavigationHandle callbacks on WebContentsObserver which are
compatible with PlzNavigate
-update the renderer side so that handles the new order of IPCs: while
ReadyToCommitNavigation means the browser side can send its IPC before the
internal content ones, with PlzNavigate one content IPC will fire both
DidStartProvisionalLoad & DidCommitProvisionalLoad. So add extra logic to the
renderer side to handle this.

This fixes
SubresourceFilterBrowserTest.MainFrameActivation
SubresourceFilterBrowserTest.MainFrameActivationOnStartup
SubresourceFilterBrowserTest.SubFrameActivation
with PlzNavigate.

BUG=504347, 653807
==========

to

==========
Fix SubResourceFilter to work with PlzNavigate.

There are two main fixes:
-use the new NavigationHandle callbacks on WebContentsObserver which are
compatible with PlzNavigate
-update the renderer side so that handles the new order of IPCs: while
ReadyToCommitNavigation means the browser side can send its IPC before the
internal content ones, with PlzNavigate one content IPC will fire both
DidStartProvisionalLoad & DidCommitProvisionalLoad. So add extra logic to the
renderer side to handle this.

This fixes
SubresourceFilterBrowserTest.MainFrameActivation
SubresourceFilterBrowserTest.MainFrameActivationOnStartup
SubresourceFilterBrowserTest.SubFrameActivation
with PlzNavigate.

BUG=504347, 653807

Committed: https://crrev.com/8ab445a553266b0b6b6f4eaa6976c6f8e2b4a7b6
Cr-Commit-Position: refs/heads/master@{#427085}
==========

[commit-bot: I haz the power, 2016-10-24 16:50:03]

Patchset 2 (id:??) landed as
https://crrev.com/8ab445a553266b0b6b6f4eaa6976c6f8e2b4a7b6
Cr-Commit-Position: refs/heads/master@{#427085}