Add tests for FeaturePolicy integration with RenderFrameHost

Add tests for FeaturePolicy integration with RenderFrameHost

This adds integration tests for FeaturePolicy in RenderFrameHosts. These tests
are not meant to cover every edge case as the FeaturePolicy class itself is
tested thoroughly in feature_policy_unittest.cc. Instead they are meant to
ensure that integration with RenderFrameHost works correctly.

BUG=689802
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2883213002
Cr-Commit-Position: refs/heads/master@{#474113}
Committed: https://chromium.googlesource.com/chromium/src/+/d3da4b4f6567f5ceb91d12c5887dc49cca77cc06

[raymes, 2017-05-16 04:03:09]

Description was changed from

==========
Add tests for FeaturePolicy integration with RenderFrameHost

This adds integration tests for FeaturePolicy in RenderFrameHosts. These tests
are not meant to cover every edge case as the FeaturePolicy class itself is
tested thoroughly in feature_policy_unittest.cc. Instead they are meant to
ensure that integration with RenderFrameHost works correctly.

BUG=689802
==========

to

==========
Add tests for FeaturePolicy integration with RenderFrameHost

This adds integration tests for FeaturePolicy in RenderFrameHosts. These tests
are not meant to cover every edge case as the FeaturePolicy class itself is
tested thoroughly in feature_policy_unittest.cc. Instead they are meant to
ensure that integration with RenderFrameHost works correctly.

BUG=689802
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[raymes, 2017-05-16 04:08:55]

raymes@chromium.org changed reviewers:
+ alexmos@chromium.org, iclelland@chromium.org

[raymes, 2017-05-16 04:08:55]

alexmos/iclelland: could you ptal and let me know your thoughts about this? 

Thanks!

[alexmos, 2017-05-16 21:40:05]

Thanks, more tests is always good. :)  A couple of comments below.

https://codereview.chromium.org/2883213002/diff/20001/content/browser/frame_h...
File content/browser/frame_host/render_frame_host_manager_unittest.cc (right):

https://codereview.chromium.org/2883213002/diff/20001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_manager_unittest.cc:3176: class
RenderFrameFeaturePolicyTest : public content::RenderViewHostTestHarness {
After looking at this and also consulting with creis@, I think this might be
better in web_contents_impl_unittest.cc.  render_frame_host_manager_unittests.cc
should be for testing proxies, swapping behavior, etc - things that you didn't
need to get involved with here.

Alternatively, given that you have quite a bit of FP-specific setup here, it
might be ok to just put these in a new file, perhaps
frame_host/render_frame_host_feature_policy_unittest.cc?

https://codereview.chromium.org/2883213002/diff/20001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_manager_unittest.cc:3232:
TEST_F(RenderFrameFeaturePolicyTest, DefaultPolicy) {
I'd include Host in the name, i.e. RenderFrameHostFeaturePolicyTest

https://codereview.chromium.org/2883213002/diff/20001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_manager_unittest.cc:3306:
SetHeaderPolicy(parent, kDefaultSelfFeature, {});
Is this testing a scenario that's actually possible?  If the header policy for
the parent frame changes, doesn't that imply that the parent has committed a new
document?  And in that case, the old child frames should go away, so |child|
wouldn't normally exist for the checks below.

[raymes, 2017-05-17 05:46:41]

Thanks!

https://codereview.chromium.org/2883213002/diff/20001/content/browser/frame_h...
File content/browser/frame_host/render_frame_host_manager_unittest.cc (right):

https://codereview.chromium.org/2883213002/diff/20001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_manager_unittest.cc:3176: class
RenderFrameFeaturePolicyTest : public content::RenderViewHostTestHarness {
On 2017/05/16 21:40:05, alexmos wrote:
> After looking at this and also consulting with creis@, I think this might be
> better in web_contents_impl_unittest.cc. 
render_frame_host_manager_unittests.cc
> should be for testing proxies, swapping behavior, etc - things that you didn't
> need to get involved with here.
> 
> Alternatively, given that you have quite a bit of FP-specific setup here, it
> might be ok to just put these in a new file, perhaps
> frame_host/render_frame_host_feature_policy_unittest.cc?

I like the idea of a separate test file :) (giant files scare me in general)

https://codereview.chromium.org/2883213002/diff/20001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_manager_unittest.cc:3232:
TEST_F(RenderFrameFeaturePolicyTest, DefaultPolicy) {
On 2017/05/16 21:40:04, alexmos wrote:
> I'd include Host in the name, i.e. RenderFrameHostFeaturePolicyTest

Oops, done.

https://codereview.chromium.org/2883213002/diff/20001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_manager_unittest.cc:3306:
SetHeaderPolicy(parent, kDefaultSelfFeature, {});
On 2017/05/16 21:40:05, alexmos wrote:
> Is this testing a scenario that's actually possible?  If the header policy for
> the parent frame changes, doesn't that imply that the parent has committed a
new
> document?  And in that case, the old child frames should go away, so |child|
> wouldn't normally exist for the checks below.

It's true - I was cheating :) I changed it to "RefreshPageAndSetHeaderPolicy". A
few questions though:
1) I added a DCHECK in RenderFrameHostImpl to ensure a header isn't set twice
(to make what I was trying to do throw an error). Does that seem like it holds
to you? 
2) I had to navigate to a different URL in order to actually have the navigation
stick to simulate a refresh. Is there a way around that?
3) It doesn't seem like the child RenderFrameHost gets destroyed when its parent
navigates. It's not problematic for this test, but just want to make sure that
I'm not doing something wrong.

[raymes, 2017-05-17 05:47:27]

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

[commit-bot: I haz the power, 2017-05-17 05:48:26]

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, 2017-05-17 06:24:24]

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

[commit-bot: I haz the power, 2017-05-17 06:24:25]

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

[alexmos, 2017-05-17 22:48:49]

Thanks, responses below, and looks like the trybots aren't happy yet.

Also, one other thing -- since this is testing RFH::IsFeatureEnabled, would you
mind adding a comment on   IsFeatureEnabled in
content/public/browser/render_frame_host.h?  I think I might've missed it in an
earlier review, but it'd be nice to document what the public API does. :)

https://codereview.chromium.org/2883213002/diff/20001/content/browser/frame_h...
File content/browser/frame_host/render_frame_host_manager_unittest.cc (right):

https://codereview.chromium.org/2883213002/diff/20001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_manager_unittest.cc:3306:
SetHeaderPolicy(parent, kDefaultSelfFeature, {});
On 2017/05/17 05:46:41, raymes wrote:
> On 2017/05/16 21:40:05, alexmos wrote:
> > Is this testing a scenario that's actually possible?  If the header policy
for
> > the parent frame changes, doesn't that imply that the parent has committed a
> new
> > document?  And in that case, the old child frames should go away, so |child|
> > wouldn't normally exist for the checks below.
> 
> It's true - I was cheating :) I changed it to "RefreshPageAndSetHeaderPolicy".
A
> few questions though:
> 1) I added a DCHECK in RenderFrameHostImpl to ensure a header isn't set twice
> (to make what I was trying to do throw an error). Does that seem like it holds
> to you? 

Will that work across multiple documents in the same frame?  (I.e., same-site
renderer-initiated navigations that reuse the RFH?)  I guess we should be
resetting the header as part of commit, and then
FrameHostMsg_DidSetFeaturePolicyHeader would come in right afterward, so it
should work.

> 2) I had to navigate to a different URL in order to actually have the
navigation
> stick to simulate a refresh. Is there a way around that?

Are you referring to the about:blank navigation that precedes the real one? 
Might be some weirdness with the unit test setup -- e.g., it might be thinking
that we haven't changed the document, due to this url comparison:
https://cs.chromium.org/chromium/src/content/test/test_render_frame_host.cc?l...

You might be able to simulate the commit some other way to avoid this.  The
comment on SimulateNavigationCommit actually says it's deprecated in favor of
NavigationSimulator, so you might want to give that a try.  (example:
https://cs.chromium.org/chromium/src/chrome/browser/page_load_metrics/observe...)

> 3) It doesn't seem like the child RenderFrameHost gets destroyed when its
parent
> navigates. It's not problematic for this test, but just want to make sure that
> I'm not doing something wrong.

That might just be a problem with unit tests.  For cross-process navigations,
the child FrameTreeNodes are removed by ResetForNewProcess(), and for
same-process navigations, I think they'd be detached by FrameHostMsg_Detach
messages as part of unloading the previous document, which probably aren't
sent/simulated in unit tests.

https://codereview.chromium.org/2883213002/diff/40001/content/common/feature_...
File content/common/feature_policy/feature_policy.cc (right):

https://codereview.chromium.org/2883213002/diff/40001/content/common/feature_...
content/common/feature_policy/feature_policy.cc:150: return
!whitelists_.empty();
Can you set an empty policy in the header, one that would keep whitelists_
empty?  If so, it seems like this would return the wrong thing here.

[raymes, 2017-05-21 22:11:20]

Thanks Alex. I think I'm almost there.

The main problem now is that the android tests are failing. The reason is
because in NavigatorImpl::DidNavigate |oopifs_possible| is set to false. So
RenderFrameHostManager::DidNavigateFrame never gets called and so
FrameTreeNode::CommitPendingFramePolicy never gets called.

Is this a bug? Is there some state I should set up to make oopifs_possible true
for the unittests (that seemed wrong to me)? Or is there something else I'm
missing?

https://codereview.chromium.org/2883213002/diff/40001/content/common/feature_...
File content/common/feature_policy/feature_policy.cc (right):

https://codereview.chromium.org/2883213002/diff/40001/content/common/feature_...
content/common/feature_policy/feature_policy.cc:150: return
!whitelists_.empty();
On 2017/05/17 22:48:49, alexmos (mostly OOO on 5-19) wrote:
> Can you set an empty policy in the header, one that would keep whitelists_
> empty?  If so, it seems like this would return the wrong thing here.

Ahh good catch. I've removed this for now, as right now we don't have the state
anywhere to check this. If you felt comfortable about it, I could add a bool
which was set to true in RenderFrameHostImpl::OnDidSetFeaturePolicyHeader but it
would only be for DCHECKing.

[alexmos, 2017-05-22 23:05:47]

On 2017/05/21 22:11:20, raymes wrote:
> Thanks Alex. I think I'm almost there.
> 
> The main problem now is that the android tests are failing. The reason is
> because in NavigatorImpl::DidNavigate |oopifs_possible| is set to false. So
> RenderFrameHostManager::DidNavigateFrame never gets called and so
> FrameTreeNode::CommitPendingFramePolicy never gets called.
> 
> Is this a bug? Is there some state I should set up to make oopifs_possible
true
> for the unittests (that seemed wrong to me)? Or is there something else I'm
> missing?

Ah yes, this is a known issue. :(  Android is the last platform where we have
AreCrossProcessFramesPossible returning false unless one of OOPIF modes like
--site-per-process is explicitly turned on.  (--isolate-extensions doesn't
count, due to no extensions on Android.)   I've just filed
https://crbug.com/725265 with details.  You're right that DidNavigateFrame won't
be called for subframes on Android (for main frames it will be called via
another path above) until we fix this, so it seems we have a bug there with both
sandbox flags and feature policy.  Glad you found this!

I agree that we shouldn't just enable OOPIFs in your tests, unless you don't
care about correctness on Android (is feature policy shipping there?).  We could
try to get rid of the "if (oopifs_possible)" and just always call
DidNavigateFrame from DidNavigate, and remove the other main-frame-specific call
to DidNavigateFrame above.  I think that code path should be safe to enable for
Android anyway.  I've put together https://codereview.chromium.org/2901643002/
to give it a try.

https://codereview.chromium.org/2883213002/diff/40001/content/common/feature_...
File content/common/feature_policy/feature_policy.cc (right):

https://codereview.chromium.org/2883213002/diff/40001/content/common/feature_...
content/common/feature_policy/feature_policy.cc:150: return
!whitelists_.empty();
On 2017/05/21 22:11:20, raymes wrote:
> On 2017/05/17 22:48:49, alexmos (mostly OOO on 5-19) wrote:
> > Can you set an empty policy in the header, one that would keep whitelists_
> > empty?  If so, it seems like this would return the wrong thing here.
> 
> Ahh good catch. I've removed this for now, as right now we don't have the
state
> anywhere to check this. If you felt comfortable about it, I could add a bool
> which was set to true in RenderFrameHostImpl::OnDidSetFeaturePolicyHeader but
it
> would only be for DCHECKing.

I'm fine leaving it out until there's a more substantial need. :)

https://codereview.chromium.org/2883213002/diff/60001/content/public/browser/...
File content/public/browser/render_frame_host.h (right):

https://codereview.chromium.org/2883213002/diff/60001/content/public/browser/...
content/public/browser/render_frame_host.h:266: // in the browser process to
determine whether access to a feature is allowed.
nit: duplicate "to determine", remove one of them.

[raymes, 2017-05-22 23:26:07]

On 2017/05/22 23:05:47, alexmos wrote:
> On 2017/05/21 22:11:20, raymes wrote:
> > Thanks Alex. I think I'm almost there.
> > 
> > The main problem now is that the android tests are failing. The reason is
> > because in NavigatorImpl::DidNavigate |oopifs_possible| is set to false. So
> > RenderFrameHostManager::DidNavigateFrame never gets called and so
> > FrameTreeNode::CommitPendingFramePolicy never gets called.
> > 
> > Is this a bug? Is there some state I should set up to make oopifs_possible
> true
> > for the unittests (that seemed wrong to me)? Or is there something else I'm
> > missing?
> 
> Ah yes, this is a known issue. :(  Android is the last platform where we have
> AreCrossProcessFramesPossible returning false unless one of OOPIF modes like
> --site-per-process is explicitly turned on.  (--isolate-extensions doesn't
> count, due to no extensions on Android.)   I've just filed
> https://crbug.com/725265 with details.  You're right that DidNavigateFrame
won't
> be called for subframes on Android (for main frames it will be called via
> another path above) until we fix this, so it seems we have a bug there with
both
> sandbox flags and feature policy.  Glad you found this!
> 
> I agree that we shouldn't just enable OOPIFs in your tests, unless you don't
> care about correctness on Android (is feature policy shipping there?).  We
could
> try to get rid of the "if (oopifs_possible)" and just always call
> DidNavigateFrame from DidNavigate, and remove the other main-frame-specific
call
> to DidNavigateFrame above.  I think that code path should be safe to enable
for
> Android anyway.  I've put together https://codereview.chromium.org/2901643002/
> to give it a try.

Ah cool, it's nice when testing pays off :) We do care about FP on Android so it
would be good to fix this. I'll wait and see how your fix goes before I try to
land. Thanks for your help!

> 
>
https://codereview.chromium.org/2883213002/diff/40001/content/common/feature_...
> File content/common/feature_policy/feature_policy.cc (right):
> 
>
https://codereview.chromium.org/2883213002/diff/40001/content/common/feature_...
> content/common/feature_policy/feature_policy.cc:150: return
> !whitelists_.empty();
> On 2017/05/21 22:11:20, raymes wrote:
> > On 2017/05/17 22:48:49, alexmos (mostly OOO on 5-19) wrote:
> > > Can you set an empty policy in the header, one that would keep whitelists_
> > > empty?  If so, it seems like this would return the wrong thing here.
> > 
> > Ahh good catch. I've removed this for now, as right now we don't have the
> state
> > anywhere to check this. If you felt comfortable about it, I could add a bool
> > which was set to true in RenderFrameHostImpl::OnDidSetFeaturePolicyHeader
but
> it
> > would only be for DCHECKing.
> 
> I'm fine leaving it out until there's a more substantial need. :)
> 
>
https://codereview.chromium.org/2883213002/diff/60001/content/public/browser/...
> File content/public/browser/render_frame_host.h (right):
> 
>
https://codereview.chromium.org/2883213002/diff/60001/content/public/browser/...
> content/public/browser/render_frame_host.h:266: // in the browser process to
> determine whether access to a feature is allowed.
> nit: duplicate "to determine", remove one of them.

Done.

[alexmos, 2017-05-22 23:47:29]

LGTM, thanks!

On 2017/05/22 23:26:07, raymes wrote:
> On 2017/05/22 23:05:47, alexmos wrote:
> > On 2017/05/21 22:11:20, raymes wrote:
> > > Thanks Alex. I think I'm almost there.
> > > 
> > > The main problem now is that the android tests are failing. The reason is
> > > because in NavigatorImpl::DidNavigate |oopifs_possible| is set to false.
So
> > > RenderFrameHostManager::DidNavigateFrame never gets called and so
> > > FrameTreeNode::CommitPendingFramePolicy never gets called.
> > > 
> > > Is this a bug? Is there some state I should set up to make oopifs_possible
> > true
> > > for the unittests (that seemed wrong to me)? Or is there something else
I'm
> > > missing?
> > 
> > Ah yes, this is a known issue. :(  Android is the last platform where we
have
> > AreCrossProcessFramesPossible returning false unless one of OOPIF modes like
> > --site-per-process is explicitly turned on.  (--isolate-extensions doesn't
> > count, due to no extensions on Android.)   I've just filed
> > https://crbug.com/725265 with details.  You're right that DidNavigateFrame
> won't
> > be called for subframes on Android (for main frames it will be called via
> > another path above) until we fix this, so it seems we have a bug there with
> both
> > sandbox flags and feature policy.  Glad you found this!
> > 
> > I agree that we shouldn't just enable OOPIFs in your tests, unless you don't
> > care about correctness on Android (is feature policy shipping there?).  We
> could
> > try to get rid of the "if (oopifs_possible)" and just always call
> > DidNavigateFrame from DidNavigate, and remove the other main-frame-specific
> call
> > to DidNavigateFrame above.  I think that code path should be safe to enable
> for
> > Android anyway.  I've put together
https://codereview.chromium.org/2901643002/
> > to give it a try.
> 
> Ah cool, it's nice when testing pays off :) We do care about FP on Android so
it
> would be good to fix this. I'll wait and see how your fix goes before I try to
> land. Thanks for your help!

Charlie approved it and it's in the CQ now. :)

[raymes, 2017-05-23 00:33:35]

Thanks for your help!

On Tue, 23 May 2017 at 09:47 <alexmos@chromium.org> wrote:

> LGTM, thanks!
>
>
> On 2017/05/22 23:26:07, raymes wrote:
> > On 2017/05/22 23:05:47, alexmos wrote:
> > > On 2017/05/21 22:11:20, raymes wrote:
> > > > Thanks Alex. I think I'm almost there.
> > > >
> > > > The main problem now is that the android tests are failing. The
> reason is
> > > > because in NavigatorImpl::DidNavigate |oopifs_possible| is set to
> false.
> So
> > > > RenderFrameHostManager::DidNavigateFrame never gets called and so
> > > > FrameTreeNode::CommitPendingFramePolicy never gets called.
> > > >
> > > > Is this a bug? Is there some state I should set up to make
> oopifs_possible
> > > true
> > > > for the unittests (that seemed wrong to me)? Or is there something
> else
> I'm
> > > > missing?
> > >
> > > Ah yes, this is a known issue. :( Android is the last platform where we
> have
> > > AreCrossProcessFramesPossible returning false unless one of OOPIF
> modes like
> > > --site-per-process is explicitly turned on. (--isolate-extensions
> doesn't
> > > count, due to no extensions on Android.) I've just filed
> > > https://crbug.com/725265 with details. You're right that
> DidNavigateFrame
> > won't
> > > be called for subframes on Android (for main frames it will be called
> via
> > > another path above) until we fix this, so it seems we have a bug there
> with
> > both
> > > sandbox flags and feature policy. Glad you found this!
> > >
> > > I agree that we shouldn't just enable OOPIFs in your tests, unless you
> don't
> > > care about correctness on Android (is feature policy shipping there?).
> We
> > could
> > > try to get rid of the "if (oopifs_possible)" and just always call
> > > DidNavigateFrame from DidNavigate, and remove the other
> main-frame-specific
> > call
> > > to DidNavigateFrame above. I think that code path should be safe to
> enable
> > for
> > > Android anyway. I've put together
> https://codereview.chromium.org/2901643002/
> > > to give it a try.
> >
> > Ah cool, it's nice when testing pays off :) We do care about FP on
> Android so
> it
> > would be good to fix this. I'll wait and see how your fix goes before I
> try to
> > land. Thanks for your help!
>
> Charlie approved it and it's in the CQ now. :)
>
> https://codereview.chromium.org/2883213002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[raymes, 2017-05-23 01:09:00]

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

[commit-bot: I haz the power, 2017-05-23 01:09:31]

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, 2017-05-23 02:26:08]

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

[commit-bot: I haz the power, 2017-05-23 02:26:09]

Dry run: This issue passed the CQ dry run.

[raymes, 2017-05-23 02:49:22]

The CQ bit was checked by raymes@chromium.org

[commit-bot: I haz the power, 2017-05-23 02:49:41]

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-23 02:59:19]

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

[commit-bot: I haz the power, 2017-05-23 02:59:20]

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

[raymes, 2017-05-23 03:10:29]

raymes@chromium.org changed reviewers:
+ dstockwell@chromium.org

[raymes, 2017-05-23 03:10:30]

Apparently:

You need LGTM from owners of depends-on paths in DEPS that were modified in this
CL:
    '+third_party/WebKit/public/platform/WebFeaturePolicyFeature.h',


+dstockwell can you ptal at content/browser/DEPS (it's just adding an extra enum
from blink to DEPS)

[dstockwell, 2017-05-23 23:53:53]

lgtm

[raymes, 2017-05-23 23:55:50]

The CQ bit was checked by raymes@chromium.org

[commit-bot: I haz the power, 2017-05-23 23:56:55]

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-24 00:06:28]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1495583750189200,
"parent_rev": "b650e659c291030324aebd4cf50c003ad652bd98", "commit_rev":
"d3da4b4f6567f5ceb91d12c5887dc49cca77cc06"}

[commit-bot: I haz the power, 2017-05-24 00:06:38]

Description was changed from

==========
Add tests for FeaturePolicy integration with RenderFrameHost

This adds integration tests for FeaturePolicy in RenderFrameHosts. These tests
are not meant to cover every edge case as the FeaturePolicy class itself is
tested thoroughly in feature_policy_unittest.cc. Instead they are meant to
ensure that integration with RenderFrameHost works correctly.

BUG=689802
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add tests for FeaturePolicy integration with RenderFrameHost

This adds integration tests for FeaturePolicy in RenderFrameHosts. These tests
are not meant to cover every edge case as the FeaturePolicy class itself is
tested thoroughly in feature_policy_unittest.cc. Instead they are meant to
ensure that integration with RenderFrameHost works correctly.

BUG=689802
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2883213002
Cr-Commit-Position: refs/heads/master@{#474113}
Committed:
https://chromium.googlesource.com/chromium/src/+/d3da4b4f6567f5ceb91d12c5887d...
==========

[commit-bot: I haz the power, 2017-05-24 00:06:39]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/d3da4b4f6567f5ceb91d12c5887d...