Limit protection of clients[0-9]*.google.com to requests from browser.

Limit protection of clients[0-9]*.google.com to requests from browser.

The WebRequest API protected network requests to clients[0-9]*.google.com from
being visible or modified by extensions. This was built under the assumption
that these domains are only used by Chrome internally, which was invalid. This
CL restrictes the protection to only those network requests that originate from
the browser. Requests from the renderers are exposed to extensions.

BUG=715184
TEST=https://crbug.com/715184#c12

Review-Url: https://codereview.chromium.org/2845943003
Cr-Commit-Position: refs/heads/master@{#470949}
Committed: https://chromium.googlesource.com/chromium/src/+/14ca4ff2719095e31e2f4cc49d1d5e8d5af175ce

[battre, 2017-04-27 11:53:17]

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

[commit-bot: I haz the power, 2017-04-27 11:53: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-27 12:22:49]

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

[commit-bot: I haz the power, 2017-04-27 12:22:50]

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

[battre, 2017-04-27 15:13:40]

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

[commit-bot: I haz the power, 2017-04-27 15:14:12]

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-27 16:22:11]

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

[commit-bot: I haz the power, 2017-04-27 16:22:11]

Dry run: This issue passed the CQ dry run.

[battre, 2017-05-04 13:10:35]

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

[commit-bot: I haz the power, 2017-05-04 13:10:47]

Dry run: CQ is trying da patch.

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

[battre, 2017-05-04 13:20:18]

Description was changed from

==========
Limit protection of clients[0-9]*.google.com to requests from browser.

The WebRequest API protected network requests to clients[0-9]*.google.com from
being visible or modified by extensions. This was built under the assumption
that these domains are only used by Chrome internally, which was invalid. This
CL restrictes the protection to only those network requests that originate from
the browser. Requests from the renderers are exposed to extensions.

BUG=715184
==========

to

==========
Limit protection of clients[0-9]*.google.com to requests from browser.

The WebRequest API protected network requests to clients[0-9]*.google.com from
being visible or modified by extensions. This was built under the assumption
that these domains are only used by Chrome internally, which was invalid. This
CL restrictes the protection to only those network requests that originate from
the browser. Requests from the renderers are exposed to extensions.

BUG=715184
TEST=https://crbug.com/715184#c12
==========

[battre, 2017-05-04 13:21:00]

battre@chromium.org changed reviewers:
+ mmenke@chromium.org, rdevlin.cronin@chromium.org

[battre, 2017-05-04 13:21:00]

Could you please review this CL? Thanks.

[commit-bot: I haz the power, 2017-05-04 14:08:24]

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

[commit-bot: I haz the power, 2017-05-04 14:08:24]

Dry run: This issue passed the CQ dry run.

[Devlin, 2017-05-04 15:33:29]

Thanks for looking into this!

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
File extensions/browser/api/web_request/web_request_permissions.cc (right):

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
extensions/browser/api/web_request/web_request_permissions.cc:64:
base::StringPiece::size_type pos = host.rfind(kClient);
Is this change to protect subdomains of clients*.google.com?  Do we want/need to
do that?

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
extensions/browser/api/web_request/web_request_permissions.cc:64:
base::StringPiece::size_type pos = host.rfind(kClient);
nit: given the area of code this is called in and how sensitive it is to
performance, can we only do this find if is_request_from_renderer is true?

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
extensions/browser/api/web_request/web_request_permissions.cc:70: for
(base::StringPiece::const_iterator
I think this loop is right, but it's kind of a shame that we have to get so
close to reparsing the url.  mmenke@, do you know of any utilities that do this
type of thing already?  e.g., this would be much easier if there was a
GetHostPieces() type method that returned a vector of the parts of the URL host.

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
extensions/browser/api/web_request/web_request_permissions.cc:120: process_id))
{
I'd actually prefer we only do this in the case of IsSensitiveURL().  For
instance, the NTP might have webui bindings, but I think that requests from that
to the web *should* be interceptable.

Also, it'd be nice to add webui-related test cases if we can.

[battre, 2017-05-04 15:53:46]

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
File extensions/browser/api/web_request/web_request_permissions.cc (right):

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
extensions/browser/api/web_request/web_request_permissions.cc:64:
base::StringPiece::size_type pos = host.rfind(kClient);
On 2017/05/04 15:33:29, Devlin wrote:
> Is this change to protect subdomains of clients*.google.com?  Do we want/need
to
> do that?

Yes, this is to generalize the code. A couple of valid test cases failed.

.clients1.google.com would not match
a.clients1.google.com would not match

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
extensions/browser/api/web_request/web_request_permissions.cc:64:
base::StringPiece::size_type pos = host.rfind(kClient);
On 2017/05/04 15:33:29, Devlin wrote:
> nit: given the area of code this is called in and how sensitive it is to
> performance, can we only do this find if is_request_from_renderer is true?

Sure, will do this if Matt is ok with the general direction of this CL.

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
extensions/browser/api/web_request/web_request_permissions.cc:120: process_id))
{
On 2017/05/04 15:33:29, Devlin wrote:
> I'd actually prefer we only do this in the case of IsSensitiveURL().  For
> instance, the NTP might have webui bindings, but I think that requests from
that
> to the web *should* be interceptable.

Sure, will do this if Matt is ok with the general direction.

> Also, it'd be nice to add webui-related test cases if we can.

This one will be really difficult because it is a huge integration test. Given
the flakiness of the current webrequest API tests and the fact that many of them
got disabled, this would be a major undertaking. I definitely won't be able to
get that done before Google I/O and the next branch point.

[mmenke, 2017-05-04 15:54:42]

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
File extensions/browser/api/web_request/web_request_permissions.cc (right):

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
extensions/browser/api/web_request/web_request_permissions.cc:65: if
(!is_request_from_renderer && pos != base::StringPiece::npos) {
So we're only filtering out sensitive requests not from the renderer?  Could we
just filter out all non-renderer requests instead?  Not a big fan of host-based
filtering here.

[mmenke, 2017-05-04 15:56:33]

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
File extensions/browser/api/web_request/web_request_permissions.cc (right):

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
extensions/browser/api/web_request/web_request_permissions.cc:120: process_id))
{
On 2017/05/04 15:53:46, battre wrote:
> On 2017/05/04 15:33:29, Devlin wrote:
> > I'd actually prefer we only do this in the case of IsSensitiveURL().  For
> > instance, the NTP might have webui bindings, but I think that requests from
> that
> > to the web *should* be interceptable.
> 
> Sure, will do this if Matt is ok with the general direction.

I'm fine with that.

> > Also, it'd be nice to add webui-related test cases if we can.
> 
> This one will be really difficult because it is a huge integration test. Given
> the flakiness of the current webrequest API tests and the fact that many of
them
> got disabled, this would be a major undertaking. I definitely won't be able to
> get that done before Google I/O and the next branch point.

[Devlin, 2017-05-04 17:31:26]

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
File extensions/browser/api/web_request/web_request_permissions.cc (right):

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
extensions/browser/api/web_request/web_request_permissions.cc:65: if
(!is_request_from_renderer && pos != base::StringPiece::npos) {
On 2017/05/04 15:54:41, mmenke wrote:
> So we're only filtering out sensitive requests not from the renderer?  Could
we
> just filter out all non-renderer requests instead?  Not a big fan of
host-based
> filtering here.

I'm not comfortable with that (yet).  We have a *ton* of browser-initiated
requests (including some that might send cookies or other sensitive data), and I
don't think I'd be confident in saying that all of them are critical to browser
functionality and shouldn't be interceptable.  As an example, we use a
browser-initiated request to get thumbnail image data in ThumbnailSource, but
that seems like something that *should* be visible to extensions.

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
extensions/browser/api/web_request/web_request_permissions.cc:120: process_id))
{
On 2017/05/04 15:53:46, battre wrote:
> On 2017/05/04 15:33:29, Devlin wrote:
> > Also, it'd be nice to add webui-related test cases if we can.
> 
> This one will be really difficult because it is a huge integration test. Given
> the flakiness of the current webrequest API tests and the fact that many of
them
> got disabled, this would be a major undertaking. I definitely won't be able to
> get that done before Google I/O and the next branch point.

We could just pass a bool is_webui_process to IsSensitiveURL() and stub it out
in the tests (just like you did with is_request_from_renderer).  That's fine for
me, since hopefully HasWebUIBindings() is tested. :)

[mmenke, 2017-05-04 17:33:17]

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
File extensions/browser/api/web_request/web_request_permissions.cc (right):

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
extensions/browser/api/web_request/web_request_permissions.cc:65: if
(!is_request_from_renderer && pos != base::StringPiece::npos) {
On 2017/05/04 17:31:25, Devlin wrote:
> On 2017/05/04 15:54:41, mmenke wrote:
> > So we're only filtering out sensitive requests not from the renderer?  Could
> we
> > just filter out all non-renderer requests instead?  Not a big fan of
> host-based
> > filtering here.
> 
> I'm not comfortable with that (yet).  We have a *ton* of browser-initiated
> requests (including some that might send cookies or other sensitive data), and
I
> don't think I'd be confident in saying that all of them are critical to
browser
> functionality and shouldn't be interceptable.  As an example, we use a
> browser-initiated request to get thumbnail image data in ThumbnailSource, but
> that seems like something that *should* be visible to extensions.

So what if those thumbnails are from a clients.google.com URL?

[Devlin, 2017-05-04 18:06:01]

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
File extensions/browser/api/web_request/web_request_permissions.cc (right):

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
extensions/browser/api/web_request/web_request_permissions.cc:65: if
(!is_request_from_renderer && pos != base::StringPiece::npos) {
On 2017/05/04 17:33:16, mmenke wrote:
> On 2017/05/04 17:31:25, Devlin wrote:
> > On 2017/05/04 15:54:41, mmenke wrote:
> > > So we're only filtering out sensitive requests not from the renderer? 
Could
> > we
> > > just filter out all non-renderer requests instead?  Not a big fan of
> > host-based
> > > filtering here.
> > 
> > I'm not comfortable with that (yet).  We have a *ton* of browser-initiated
> > requests (including some that might send cookies or other sensitive data),
and
> I
> > don't think I'd be confident in saying that all of them are critical to
> browser
> > functionality and shouldn't be interceptable.  As an example, we use a
> > browser-initiated request to get thumbnail image data in ThumbnailSource,
but
> > that seems like something that *should* be visible to extensions.
> 
> So what if those thumbnails are from a http://clients.google.com URL?

For want of a better solution, we'll protect them.  We've had some discussions
on the bug and internal thread about a better long-term solution for this
(relying on network annotations or using a different request context for
requests that should be hidden) which would handle these cases in a better way,
but for now I think this is a reasonable compromise.

[mmenke, 2017-05-05 14:35:14]

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
File extensions/browser/api/web_request/web_request_permissions.cc (right):

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
extensions/browser/api/web_request/web_request_permissions.cc:65: if
(!is_request_from_renderer && pos != base::StringPiece::npos) {
On 2017/05/04 18:06:00, Devlin wrote:
> On 2017/05/04 17:33:16, mmenke wrote:
> > On 2017/05/04 17:31:25, Devlin wrote:
> > > On 2017/05/04 15:54:41, mmenke wrote:
> > > > So we're only filtering out sensitive requests not from the renderer? 
> Could
> > > we
> > > > just filter out all non-renderer requests instead?  Not a big fan of
> > > host-based
> > > > filtering here.
> > > 
> > > I'm not comfortable with that (yet).  We have a *ton* of browser-initiated
> > > requests (including some that might send cookies or other sensitive data),
> and
> > I
> > > don't think I'd be confident in saying that all of them are critical to
> > browser
> > > functionality and shouldn't be interceptable.  As an example, we use a
> > > browser-initiated request to get thumbnail image data in ThumbnailSource,
> but
> > > that seems like something that *should* be visible to extensions.
> > 
> > So what if those thumbnails are from a http://clients.google.com URL?
> 
> For want of a better solution, we'll protect them.  We've had some discussions
> on the bug and internal thread about a better long-term solution for this
> (relying on network annotations or using a different request context for
> requests that should be hidden) which would handle these cases in a better
way,
> but for now I think this is a reasonable compromise.

Could we instead have the initiators tag sensitive requests?  I'm not a fan of
using a magic domain, particularly as we have no idea just what services it is
and is not used for.

[Devlin, 2017-05-05 16:16:04]

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
File extensions/browser/api/web_request/web_request_permissions.cc (right):

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
extensions/browser/api/web_request/web_request_permissions.cc:65: if
(!is_request_from_renderer && pos != base::StringPiece::npos) {
On 2017/05/05 14:35:14, mmenke wrote:
> On 2017/05/04 18:06:00, Devlin wrote:
> > On 2017/05/04 17:33:16, mmenke wrote:
> > > On 2017/05/04 17:31:25, Devlin wrote:
> > > > On 2017/05/04 15:54:41, mmenke wrote:
> > > > > So we're only filtering out sensitive requests not from the renderer? 
> > Could
> > > > we
> > > > > just filter out all non-renderer requests instead?  Not a big fan of
> > > > host-based
> > > > > filtering here.
> > > > 
> > > > I'm not comfortable with that (yet).  We have a *ton* of
browser-initiated
> > > > requests (including some that might send cookies or other sensitive
data),
> > and
> > > I
> > > > don't think I'd be confident in saying that all of them are critical to
> > > browser
> > > > functionality and shouldn't be interceptable.  As an example, we use a
> > > > browser-initiated request to get thumbnail image data in
ThumbnailSource,
> > but
> > > > that seems like something that *should* be visible to extensions.
> > > 
> > > So what if those thumbnails are from a http://clients.google.com URL?
> > 
> > For want of a better solution, we'll protect them.  We've had some
discussions
> > on the bug and internal thread about a better long-term solution for this
> > (relying on network annotations or using a different request context for
> > requests that should be hidden) which would handle these cases in a better
> way,
> > but for now I think this is a reasonable compromise.
> 
> Could we instead have the initiators tag sensitive requests?  I'm not a fan of
> using a magic domain, particularly as we have no idea just what services it is
> and is not used for.

Yes - that's what I was mentioning about relying on network annotations (which
could have a "sensitive" field) or using a different request context (which
would imply it's sensitive).  The problem is that both of those are much larger
projects, and we should have a fix for this problem sooner rather than later.

[mmenke, 2017-05-05 16:18:03]

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
File extensions/browser/api/web_request/web_request_permissions.cc (right):

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
extensions/browser/api/web_request/web_request_permissions.cc:65: if
(!is_request_from_renderer && pos != base::StringPiece::npos) {
On 2017/05/05 16:16:03, Devlin wrote:
> On 2017/05/05 14:35:14, mmenke wrote:
> > On 2017/05/04 18:06:00, Devlin wrote:
> > > On 2017/05/04 17:33:16, mmenke wrote:
> > > > On 2017/05/04 17:31:25, Devlin wrote:
> > > > > On 2017/05/04 15:54:41, mmenke wrote:
> > > > > > So we're only filtering out sensitive requests not from the
renderer? 
> > > Could
> > > > > we
> > > > > > just filter out all non-renderer requests instead?  Not a big fan of
> > > > > host-based
> > > > > > filtering here.
> > > > > 
> > > > > I'm not comfortable with that (yet).  We have a *ton* of
> browser-initiated
> > > > > requests (including some that might send cookies or other sensitive
> data),
> > > and
> > > > I
> > > > > don't think I'd be confident in saying that all of them are critical
to
> > > > browser
> > > > > functionality and shouldn't be interceptable.  As an example, we use a
> > > > > browser-initiated request to get thumbnail image data in
> ThumbnailSource,
> > > but
> > > > > that seems like something that *should* be visible to extensions.
> > > > 
> > > > So what if those thumbnails are from a http://clients.google.com URL?
> > > 
> > > For want of a better solution, we'll protect them.  We've had some
> discussions
> > > on the bug and internal thread about a better long-term solution for this
> > > (relying on network annotations or using a different request context for
> > > requests that should be hidden) which would handle these cases in a better
> > way,
> > > but for now I think this is a reasonable compromise.
> > 
> > Could we instead have the initiators tag sensitive requests?  I'm not a fan
of
> > using a magic domain, particularly as we have no idea just what services it
is
> > and is not used for.
> 
> Yes - that's what I was mentioning about relying on network annotations (which
> could have a "sensitive" field) or using a different request context (which
> would imply it's sensitive).  The problem is that both of those are much
larger
> projects, and we should have a fix for this problem sooner rather than later.

Sorry, completely missed that.

[battre, 2017-05-10 09:16:42]

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

[commit-bot: I haz the power, 2017-05-10 09:17:29]

Dry run: CQ is trying da patch.

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

[battre, 2017-05-10 09:18:38]

PTAL. Thanks!

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
File extensions/browser/api/web_request/web_request_permissions.cc (right):

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
extensions/browser/api/web_request/web_request_permissions.cc:64:
base::StringPiece::size_type pos = host.rfind(kClient);
On 2017/05/04 15:53:46, battre wrote:
> On 2017/05/04 15:33:29, Devlin wrote:
> > Is this change to protect subdomains of clients*.google.com?  Do we
want/need
> to
> > do that?
> 
> Yes, this is to generalize the code. A couple of valid test cases failed.
> 
> .clients1.google.com would not match
> http://a.clients1.google.com would not match

Done.

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
extensions/browser/api/web_request/web_request_permissions.cc:65: if
(!is_request_from_renderer && pos != base::StringPiece::npos) {
On 2017/05/05 16:18:03, mmenke wrote:
> On 2017/05/05 16:16:03, Devlin wrote:
> > On 2017/05/05 14:35:14, mmenke wrote:
> > > On 2017/05/04 18:06:00, Devlin wrote:
> > > > On 2017/05/04 17:33:16, mmenke wrote:
> > > > > On 2017/05/04 17:31:25, Devlin wrote:
> > > > > > On 2017/05/04 15:54:41, mmenke wrote:
> > > > > > > So we're only filtering out sensitive requests not from the
> renderer? 
> > > > Could
> > > > > > we
> > > > > > > just filter out all non-renderer requests instead?  Not a big fan
of
> > > > > > host-based
> > > > > > > filtering here.
> > > > > > 
> > > > > > I'm not comfortable with that (yet).  We have a *ton* of
> > browser-initiated
> > > > > > requests (including some that might send cookies or other sensitive
> > data),
> > > > and
> > > > > I
> > > > > > don't think I'd be confident in saying that all of them are critical
> to
> > > > > browser
> > > > > > functionality and shouldn't be interceptable.  As an example, we use
a
> > > > > > browser-initiated request to get thumbnail image data in
> > ThumbnailSource,
> > > > but
> > > > > > that seems like something that *should* be visible to extensions.
> > > > > 
> > > > > So what if those thumbnails are from a http://clients.google.com URL?
> > > > 
> > > > For want of a better solution, we'll protect them.  We've had some
> > discussions
> > > > on the bug and internal thread about a better long-term solution for
this
> > > > (relying on network annotations or using a different request context for
> > > > requests that should be hidden) which would handle these cases in a
better
> > > way,
> > > > but for now I think this is a reasonable compromise.
> > > 
> > > Could we instead have the initiators tag sensitive requests?  I'm not a
fan
> of
> > > using a magic domain, particularly as we have no idea just what services
it
> is
> > > and is not used for.
> > 
> > Yes - that's what I was mentioning about relying on network annotations
(which
> > could have a "sensitive" field) or using a different request context (which
> > would imply it's sensitive).  The problem is that both of those are much
> larger
> > projects, and we should have a fix for this problem sooner rather than
later.
> 
> Sorry, completely missed that.

Acknowledged.

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
extensions/browser/api/web_request/web_request_permissions.cc:70: for
(base::StringPiece::const_iterator
On 2017/05/04 15:33:29, Devlin (OOO until May 12) wrote:
> I think this loop is right, but it's kind of a shame that we have to get so
> close to reparsing the url.  mmenke@, do you know of any utilities that do
this
> type of thing already?  e.g., this would be much easier if there was a
> GetHostPieces() type method that returned a vector of the parts of the URL
host.

Leaving this as is. I did not find anything better.

https://codereview.chromium.org/2845943003/diff/40001/extensions/browser/api/...
extensions/browser/api/web_request/web_request_permissions.cc:120: process_id))
{
On 2017/05/04 17:31:25, Devlin (OOO until May 12) wrote:
> On 2017/05/04 15:53:46, battre wrote:
> > On 2017/05/04 15:33:29, Devlin wrote:
> > > Also, it'd be nice to add webui-related test cases if we can.
> > 
> > This one will be really difficult because it is a huge integration test.
Given
> > the flakiness of the current webrequest API tests and the fact that many of
> them
> > got disabled, this would be a major undertaking. I definitely won't be able
to
> > get that done before Google I/O and the next branch point.
> 
> We could just pass a bool is_webui_process to IsSensitiveURL() and stub it out
> in the tests (just like you did with is_request_from_renderer).  That's fine
for
> me, since hopefully HasWebUIBindings() is tested. :)

Done.

[battre, 2017-05-10 09:18:38]

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

[commit-bot: I haz the power, 2017-05-10 09:19:22]

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-10 09:23:28]

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

[commit-bot: I haz the power, 2017-05-10 09:23:29]

Dry run: Try jobs failed on following builders:
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Devlin, 2017-05-10 15:02:06]

It looks like there's a patch failure, but assuming no significant changes are
needed, this LGTM.

[mmenke, 2017-05-10 15:23:24]

On 2017/05/10 15:02:06, Devlin (OOO until May 12) wrote:
> It looks like there's a patch failure, but assuming no significant changes are
> needed, this LGTM.

LGTM as well.

[mmenke, 2017-05-10 15:24:42]

On 2017/05/10 15:23:24, mmenke wrote:
> On 2017/05/10 15:02:06, Devlin (OOO until May 12) wrote:
> > It looks like there's a patch failure, but assuming no significant changes
are
> > needed, this LGTM.
> 
> LGTM as well.

Note that we really should have some integration tests, so that
WebRequestPermissions::HideRequest is tested as well, and make sure it does the
right thing in the fully configured environment.

[battre, 2017-05-11 10:49:31]

The CQ bit was checked by battre@chromium.org

[battre, 2017-05-11 10:49:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
rdevlin.cronin@chromium.org, mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/2845943003/#ps100001
(title: "Merged with ToT")

[commit-bot: I haz the power, 2017-05-11 10:50:15]

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-11 11:40:43]

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

[commit-bot: I haz the power, 2017-05-11 11:40:44]

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

[battre, 2017-05-11 12:40:37]

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

[commit-bot: I haz the power, 2017-05-11 12:40:59]

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-11 14:17:29]

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

[commit-bot: I haz the power, 2017-05-11 14:17:29]

Dry run: This issue passed the CQ dry run.

[battre, 2017-05-11 14:23:26]

The CQ bit was checked by battre@chromium.org

[battre, 2017-05-11 14:23:26]

The patchset sent to the CQ was uploaded after l-g-t-m from
rdevlin.cronin@chromium.org, mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/2845943003/#ps120001
(title: "Fix unittests")

[commit-bot: I haz the power, 2017-05-11 14:24:02]

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-11 14:31:51]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1494512606138860,
"parent_rev": "ee9188fc71ffc068df4a6a990f7278af5706641b", "commit_rev":
"14ca4ff2719095e31e2f4cc49d1d5e8d5af175ce"}

[commit-bot: I haz the power, 2017-05-11 14:32:02]

Description was changed from

==========
Limit protection of clients[0-9]*.google.com to requests from browser.

The WebRequest API protected network requests to clients[0-9]*.google.com from
being visible or modified by extensions. This was built under the assumption
that these domains are only used by Chrome internally, which was invalid. This
CL restrictes the protection to only those network requests that originate from
the browser. Requests from the renderers are exposed to extensions.

BUG=715184
TEST=https://crbug.com/715184#c12
==========

to

==========
Limit protection of clients[0-9]*.google.com to requests from browser.

The WebRequest API protected network requests to clients[0-9]*.google.com from
being visible or modified by extensions. This was built under the assumption
that these domains are only used by Chrome internally, which was invalid. This
CL restrictes the protection to only those network requests that originate from
the browser. Requests from the renderers are exposed to extensions.

BUG=715184
TEST=https://crbug.com/715184#c12

Review-Url: https://codereview.chromium.org/2845943003
Cr-Commit-Position: refs/heads/master@{#470949}
Committed:
https://chromium.googlesource.com/chromium/src/+/14ca4ff2719095e31e2f4cc49d1d...
==========

[commit-bot: I haz the power, 2017-05-11 14:32:04]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/14ca4ff2719095e31e2f4cc49d1d...