Add Feature Policy interface to RenderFrameHost

Add Feature Policy interface to RenderFrameHost

This adds a simple API for querying whether a feature should be available in a
given frame, from the browser process.

The API is initially going to be used by WebUSB; see https://crrev.com/2815003005

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

Review-Url: https://codereview.chromium.org/2791063002
Cr-Commit-Position: refs/heads/master@{#464548}
Committed: https://chromium.googlesource.com/chromium/src/+/08debb3333b06d4bf652e72df8720582ee5d957e

[iclelland, 2017-04-01 03:28:14]

Description was changed from

==========
Add Feature Policy interface to RenderFrameHost

This adds a simple API for querying whether a feature should be available in a
given frame, from the browser process.

BUG=
==========

to

==========
Add Feature Policy interface to RenderFrameHost

This adds a simple API for querying whether a feature should be available in a
given frame, from the browser process.

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

[iclelland, 2017-04-01 03:28:15]

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

[commit-bot: I haz the power, 2017-04-01 03:28:27]

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-04-01 04:20:29]

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

[commit-bot: I haz the power, 2017-04-01 04:20:30]

Dry run: 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_...)

[iclelland, 2017-04-03 13:43:44]

Description was changed from

==========
Add Feature Policy interface to RenderFrameHost

This adds a simple API for querying whether a feature should be available in a
given frame, from the browser process.

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

to

==========
Add Feature Policy interface to RenderFrameHost

This adds a simple API for querying whether a feature should be available in a
given frame, from the browser process.

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

[iclelland, 2017-04-04 19:33:49]

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

[iclelland, 2017-04-04 19:33:50]

+r alexmos, can you PTAL?

This comes out of the code review comment at
https://codereview.chromium.org/2655663004/diff/60001/content/browser/frame_h....

The API has actually been requested now, so I figured it's about time I
implement it :)

[alexmos, 2017-04-04 23:28:04]

Thanks, this looks good, but we typically try to land new content public APIs
together with their first use in the embedder.  Otherwise there's no guarantee
that the other CL will land, leaving behind an unused public API.  Would it be
possible to include this in the CL that wants to use it?

[Reilly Grant (use Gerrit), 2017-04-06 21:47:45]

reillyg@chromium.org changed reviewers:
+ reillyg@chromium.org

[Reilly Grant (use Gerrit), 2017-04-06 21:47:45]

I'm waiting for this patch to land in order to finish my own change. My change
is rather large already and I'm considering ways of breaking it up so I'd rather
not merge this into it.

[alexmos, 2017-04-06 22:04:59]

On 2017/04/06 21:47:45, Reilly Grant wrote:
> I'm waiting for this patch to land in order to finish my own change. My change
> is rather large already and I'm considering ways of breaking it up so I'd
rather
> not merge this into it.

Ok, LGTM then.  Can you post a link to your CL here so there's a record of where
this will be used?  Ian - can you also update the description with a link to
that CL?

[iclelland, 2017-04-11 18:42:40]

On 2017/04/06 22:04:59, alexmos wrote:
> On 2017/04/06 21:47:45, Reilly Grant wrote:
> > I'm waiting for this patch to land in order to finish my own change. My
change
> > is rather large already and I'm considering ways of breaking it up so I'd
> rather
> > not merge this into it.
> 
> Ok, LGTM then.  Can you post a link to your CL here so there's a record of
where
> this will be used?  Ian - can you also update the description with a link to
> that CL?

Reilly, do you have a CL for that yet, or is it still a local change you're
working on?

[Reilly Grant (use Gerrit), 2017-04-11 18:46:54]

On 2017/04/11 18:42:40, iclelland wrote:
> On 2017/04/06 22:04:59, alexmos wrote:
> > On 2017/04/06 21:47:45, Reilly Grant wrote:
> > > I'm waiting for this patch to land in order to finish my own change. My
> change
> > > is rather large already and I'm considering ways of breaking it up so I'd
> > rather
> > > not merge this into it.
> > 
> > Ok, LGTM then.  Can you post a link to your CL here so there's a record of
> where
> > this will be used?  Ian - can you also update the description with a link to
> > that CL?
> 
> Reilly, do you have a CL for that yet, or is it still a local change you're
> working on?

It's a local change for now because I'm still working on how Feature Policy will
affect how WebUSB is exposed to script and writing LayoutTests for that
behavior.

[iclelland, 2017-04-13 01:35:23]

Description was changed from

==========
Add Feature Policy interface to RenderFrameHost

This adds a simple API for querying whether a feature should be available in a
given frame, from the browser process.

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

to

==========
Add Feature Policy interface to RenderFrameHost

This adds a simple API for querying whether a feature should be available in a
given frame, from the browser process.

The API is initially going to be used by WebUSB; see
https://crrev.com/2815003005

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

[iclelland, 2017-04-13 01:37:08]

On 2017/04/06 22:04:59, alexmos wrote:
> On 2017/04/06 21:47:45, Reilly Grant wrote:
> > I'm waiting for this patch to land in order to finish my own change. My
change
> > is rather large already and I'm considering ways of breaking it up so I'd
> rather
> > not merge this into it.
> 
> Ok, LGTM then.  Can you post a link to your CL here so there's a record of
where
> this will be used?  Ian - can you also update the description with a link to
> that CL?

Done. I'll commit this so that the other CL can start using the API.

[iclelland, 2017-04-13 01:37:16]

The CQ bit was checked by iclelland@chromium.org

[commit-bot: I haz the power, 2017-04-13 01:37: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-04-13 01:44:26]

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

[commit-bot: I haz the power, 2017-04-13 01:44:27]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_site_isolation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_site_isol...)
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[iclelland, 2017-04-13 20:19:08]

The CQ bit was checked by iclelland@chromium.org

[iclelland, 2017-04-13 20:19:08]

The patchset sent to the CQ was uploaded after l-g-t-m from alexmos@chromium.org
Link to the patchset: https://codereview.chromium.org/2791063002/#ps20001
(title: "Rebase")

[commit-bot: I haz the power, 2017-04-13 20:19:44]

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

[Reilly Grant (use Gerrit), 2017-04-13 20:21:45]

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

https://codereview.chromium.org/2791063002/diff/20001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.cc:1918: return
feature_policy_->IsFeatureEnabledForOrigin(feature,
I recommend making this feature_policy_ && feature_policy_->... because it seems
like right now there are cases where feature_policy_ will be null and we should
fail closed rather than with a null pointer dereference.

[iclelland, 2017-04-13 20:23:46]

The CQ bit was unchecked by iclelland@chromium.org

[iclelland, 2017-04-13 20:26:30]

On 2017/04/13 20:21:45, Reilly Grant wrote:
>
https://codereview.chromium.org/2791063002/diff/20001/content/browser/frame_h...
> File content/browser/frame_host/render_frame_host_impl.cc (right):
> 
>
https://codereview.chromium.org/2791063002/diff/20001/content/browser/frame_h...
> content/browser/frame_host/render_frame_host_impl.cc:1918: return
> feature_policy_->IsFeatureEnabledForOrigin(feature,
> I recommend making this feature_policy_ && feature_policy_->... because it
seems
> like right now there are cases where feature_policy_ will be null and we
should
> fail closed rather than with a null pointer dereference.

That's a good idea; I'll add that in. Thanks.

[iclelland, 2017-04-13 20:28:57]

The CQ bit was checked by iclelland@chromium.org

[iclelland, 2017-04-13 20:28:58]

The patchset sent to the CQ was uploaded after l-g-t-m from alexmos@chromium.org
Link to the patchset: https://codereview.chromium.org/2791063002/#ps40001
(title: "Guard against possible null pointer deref")

[commit-bot: I haz the power, 2017-04-13 20:29:26]

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-04-13 21:24:19]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1492115337999430,
"parent_rev": "c1b0cc9fd49defe5a1e5ace3c09779877fc4b5e0", "commit_rev":
"08debb3333b06d4bf652e72df8720582ee5d957e"}

[commit-bot: I haz the power, 2017-04-13 21:25:07]

Description was changed from

==========
Add Feature Policy interface to RenderFrameHost

This adds a simple API for querying whether a feature should be available in a
given frame, from the browser process.

The API is initially going to be used by WebUSB; see
https://crrev.com/2815003005

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

to

==========
Add Feature Policy interface to RenderFrameHost

This adds a simple API for querying whether a feature should be available in a
given frame, from the browser process.

The API is initially going to be used by WebUSB; see
https://crrev.com/2815003005

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

Review-Url: https://codereview.chromium.org/2791063002
Cr-Commit-Position: refs/heads/master@{#464548}
Committed:
https://chromium.googlesource.com/chromium/src/+/08debb3333b06d4bf652e72df872...
==========

[commit-bot: I haz the power, 2017-04-13 21:25:09]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/08debb3333b06d4bf652e72df872...