Protect Chrome WebStore based on process IDs

Protect Chrome WebStore based on process IDs


BUG=134101
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=152551

[battre, 2012-07-31 15:51:30]

Hi.

PTAL whether you consider this the right way to do permission checks for the
declarative and non-declarative WebRequest API.

Thanks,
Dominic

[jochen (gone - plz use gerrit), 2012-08-01 08:24:53]

Yes, I like it.

One general question. What happens if you go to chrome.google.com/webstore which
is then redirected to https://chrome.google.com/webstore (and so the extension
won't get the see any more network events). Will you send some onErrorOccurred
event? Otherwise, an extension that tracks ongoing requests might leak memory

[battre, 2012-08-01 08:37:00]

On 2012/08/01 08:24:53, jochen wrote:
> Yes, I like it.
> 
> One general question. What happens if you go to chrome.google.com/webstore
which
> is then redirected to https://chrome.google.com/webstore (and so the extension
> won't get the see any more network events). Will you send some onErrorOccurred
> event? Otherwise, an extension that tracks ongoing requests might leak memory

That is an interesting point. It is not a regression, so I suggest to fix this
in a separate CL. I have filed http://crbug.com/139982 and try to fix this
today.

[Aaron Boodman, 2012-08-01 10:01:06]

Yes, this is what I had in mind. Glad it worked out.

LGTM

[battre, 2012-08-01 10:03:56]

On 2012/08/01 10:01:06, Aaron Boodman wrote:
> Yes, this is what I had in mind. Glad it worked out.
> 
> LGTM

Thanks. I will wait for Adam's LGTM as well.

[battre, 2012-08-02 11:24:11]

On 2012/08/01 10:03:56, battre wrote:
> On 2012/08/01 10:01:06, Aaron Boodman wrote:
> > Yes, this is what I had in mind. Glad it worked out.
> > 
> > LGTM
> 
> Thanks. I will wait for Adam's LGTM as well.

Adam: friendly ping

Best regards,
Dominic

[abarth-chromium, 2012-08-02 14:48:04]

I don't know this code that well, but this looks plausible.  It's too bad we
need to special case so many URLs.  For example, what if we introduce
client11.google.com?  We'd need to update this code.

http://codereview.chromium.org/10825102/diff/7001/chrome/browser/extensions/a...
File chrome/browser/extensions/api/web_request/web_request_permissions.cc
(right):

http://codereview.chromium.org/10825102/diff/7001/chrome/browser/extensions/a...
chrome/browser/extensions/api/web_request/web_request_permissions.cc:41:
EndsWith(url.host(), "client.google.com", true);
Should this be ".client.google.com"

[battre, 2012-08-02 16:43:58]

I have changed the URL comparison function such that we don't generate client0
to client9 and compare it with the URL any more.

[battre, 2012-08-07 09:43:50]

Friendly ping. If nobody has any more comments, I am going to commit this
tomorrow.

Best regards,
Dominic

[commit-bot: I haz the power, 2012-08-21 09:16:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/battre@chromium.org/10825102/5005

[commit-bot: I haz the power, 2012-08-21 10:15:07]

Try job failure for 10825102-5005 (retry) on mac_rel for step "unit_tests".
It's a second try, previously, step "unit_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-08-21 11:02:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/battre@chromium.org/10825102/18003

[commit-bot: I haz the power, 2012-08-21 13:19:12]

Change committed as 152551