Allow navigations performed by extensions to pass through AllowExtensionResourceLoad.

Allow navigations performed by extensions to pass through AllowExtensionResourceLoad.

BUG=312269
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=232733

[nasko, 2013-11-01 18:44:52]

Hey Charlie,
Can you review this fix for me?
Thanks,
Nasko

[Charlie Reis, 2013-11-01 18:47:50]

Nice.  LGTM.

https://codereview.chromium.org/55163006/diff/30001/chrome/browser/extensions...
File chrome/browser/extensions/extension_protocols.cc (right):

https://codereview.chromium.org/55163006/diff/30001/chrome/browser/extensions...
chrome/browser/extensions/extension_protocols.cc:374: // navigation logic will
properly transfer the navigation to a
nit: navigation logic in CrossSiteResourceHandler will...

[nasko, 2013-11-01 18:49:53]

Hey Matt,
Can you please review this CL for me?
Thanks in advance,
Nasko

https://codereview.chromium.org/55163006/diff/30001/chrome/browser/extensions...
File chrome/browser/extensions/extension_protocols.cc (right):

https://codereview.chromium.org/55163006/diff/30001/chrome/browser/extensions...
chrome/browser/extensions/extension_protocols.cc:374: // navigation logic will
properly transfer the navigation to a
On 2013/11/01 18:47:50, creis wrote:
> nit: navigation logic in CrossSiteResourceHandler will...

Done.

[Matt Perry, 2013-11-01 19:16:09]

https://codereview.chromium.org/55163006/diff/70001/chrome/browser/extensions...
File chrome/browser/extensions/extension_protocols.cc (right):

https://codereview.chromium.org/55163006/diff/70001/chrome/browser/extensions...
chrome/browser/extensions/extension_protocols.cc:376: if
(ResourceType::IsFrame(info->GetResourceType()))
Does this mean an arbitrary webpage can navigate an iframe to a chrome-extension
URL? Can it detect success or failure? If so, that's not good - it's why we have
the web-accessible-resource check below.

[nasko, 2013-11-01 19:58:56]

On 2013/11/01 19:16:09, Matt Perry wrote:
>
https://codereview.chromium.org/55163006/diff/70001/chrome/browser/extensions...
> File chrome/browser/extensions/extension_protocols.cc (right):
> 
>
https://codereview.chromium.org/55163006/diff/70001/chrome/browser/extensions...
> chrome/browser/extensions/extension_protocols.cc:376: if
> (ResourceType::IsFrame(info->GetResourceType()))
> Does this mean an arbitrary webpage can navigate an iframe to a
chrome-extension
> URL? Can it detect success or failure? If so, that's not good - it's why we
have
> the web-accessible-resource check below.

The code I've added here is in addition to the checks performed in the renderer
process. The only way I can think of getting past the renderer check (other than
exploited process) is for iframe request to a page that does a server redirect
to a chrome-extension:// URL. I would assume this type of redirect is disallowed
and if not, this might be an issue.

[Matt Perry, 2013-11-01 20:12:50]

OK sounds good. LGTM

[commit-bot: I haz the power, 2013-11-01 20:45:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nasko@chromium.org/55163006/140001

[commit-bot: I haz the power, 2013-11-02 00:41:58]

Retried try job too often on linux_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-11-02 02:15:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nasko@chromium.org/55163006/140001

[commit-bot: I haz the power, 2013-11-02 04:45:51]

Retried try job too often on linux_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-11-02 16:23:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nasko@chromium.org/55163006/120003

[commit-bot: I haz the power, 2013-11-02 19:36:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nasko@chromium.org/55163006/120003

[commit-bot: I haz the power, 2013-11-02 22:17:37]

Retried try job too often on win_rel for step(s) content_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-11-04 16:18:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nasko@chromium.org/55163006/120003

[commit-bot: I haz the power, 2013-11-04 17:45:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nasko@chromium.org/55163006/120003

[commit-bot: I haz the power, 2013-11-04 18:08:48]

Change committed as 232733