Added check to prevent extensions from injecting scrips into other extensions

Added check to prevent extensions from injecting scrips into pages hosted in iframe context of other extensions (other than itself).

BUG=126257
TEST=ExtensionApiTest.ContentScriptOtherExtensions, added new checks to ExtensionScriptAndCaptureVisibleTest.Permissions

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

[abarth-chromium, 2012-08-22 00:54:19]

This patch looks reasonable to me.  You'll need to get an OWNER to sign off on
it.  I've left a few minor comments below mostly related to variable naming. 
I'm more familiar with WebKit naming conventions, so the below might not be
relevant to this part of the code.

Thanks for working on this patch!

https://chromiumcodereview.appspot.com/10863002/diff/7001/chrome/common/exten...
File chrome/common/extensions/extension.h (right):

https://chromiumcodereview.appspot.com/10863002/diff/7001/chrome/common/exten...
chrome/common/extensions/extension.h:535: const GURL& top_frame_url,
How does a page_url differ from a top_frame_url?  Maybe these terms are used
differently here than they are in WebKit, but Frames and Pages don't have URLs,
Documents have URLs.  Perhaps these should be called document_url and
top_document_url respectively?

https://chromiumcodereview.appspot.com/10863002/diff/7001/chrome/renderer/ext...
File chrome/renderer/extensions/user_script_scheduler.cc (right):

https://chromiumcodereview.appspot.com/10863002/diff/7001/chrome/renderer/ext...
chrome/renderer/extensions/user_script_scheduler.cc:178:
extension_helper->tab_id(),
Having frame and frame_ be different variables is really confusing.  Perhaps we
should call frame child_frame?

https://chromiumcodereview.appspot.com/10863002/diff/7001/chrome/renderer/ext...
File chrome/renderer/extensions/user_script_slave.cc (right):

https://chromiumcodereview.appspot.com/10863002/diff/7001/chrome/renderer/ext...
chrome/renderer/extensions/user_script_slave.cc:276: parent_frame =
parent_frame->parent();
Does WebFrame really not have a top() accessor?

https://chromiumcodereview.appspot.com/10863002/diff/7001/chrome/renderer/ext...
chrome/renderer/extensions/user_script_slave.cc:288:
parent_frame->document().url(),
parent_frame is really the top_frame or what we call elsewhere the main_frame.

[zel, 2012-08-22 01:39:20]

https://chromiumcodereview.appspot.com/10863002/diff/7001/chrome/common/exten...
File chrome/common/extensions/extension.h (right):

https://chromiumcodereview.appspot.com/10863002/diff/7001/chrome/common/exten...
chrome/common/extensions/extension.h:535: const GURL& top_frame_url,
On 2012/08/22 00:54:19, abarth wrote:
> How does a page_url differ from a top_frame_url?  Maybe these terms are used
> differently here than they are in WebKit, but Frames and Pages don't have
URLs,
> Documents have URLs.  Perhaps these should be called document_url and
> top_document_url respectively?

Done.

https://chromiumcodereview.appspot.com/10863002/diff/7001/chrome/renderer/ext...
File chrome/renderer/extensions/user_script_scheduler.cc (right):

https://chromiumcodereview.appspot.com/10863002/diff/7001/chrome/renderer/ext...
chrome/renderer/extensions/user_script_scheduler.cc:178:
extension_helper->tab_id(),
On 2012/08/22 00:54:19, abarth wrote:
> Having frame and frame_ be different variables is really confusing.  Perhaps
we
> should call frame child_frame?

Done.

https://chromiumcodereview.appspot.com/10863002/diff/7001/chrome/renderer/ext...
File chrome/renderer/extensions/user_script_slave.cc (right):

https://chromiumcodereview.appspot.com/10863002/diff/7001/chrome/renderer/ext...
chrome/renderer/extensions/user_script_slave.cc:276: parent_frame =
parent_frame->parent();
On 2012/08/22 00:54:19, abarth wrote:
> Does WebFrame really not have a top() accessor?

Done.

https://chromiumcodereview.appspot.com/10863002/diff/7001/chrome/renderer/ext...
chrome/renderer/extensions/user_script_slave.cc:288:
parent_frame->document().url(),
On 2012/08/22 00:54:19, abarth wrote:
> parent_frame is really the top_frame or what we call elsewhere the main_frame.

used frame->top() accessor instead. done.

[Aaron Boodman, 2012-08-23 00:45:59]

lgtm

http://codereview.chromium.org/10863002/diff/8023/chrome/common/extensions/ex...
File chrome/common/extensions/extension.cc (right):

http://codereview.chromium.org/10863002/diff/8023/chrome/common/extensions/ex...
chrome/common/extensions/extension.cc:3606: !CanExecuteScriptEverywhere())
Braces required here.

[zel, 2012-08-23 01:28:29]

http://codereview.chromium.org/10863002/diff/8023/chrome/common/extensions/ex...
File chrome/common/extensions/extension.cc (right):

http://codereview.chromium.org/10863002/diff/8023/chrome/common/extensions/ex...
chrome/common/extensions/extension.cc:3606: !CanExecuteScriptEverywhere())
On 2012/08/23 00:45:59, Aaron Boodman wrote:
> Braces required here.

Done.

[commit-bot: I haz the power, 2012-08-23 01:28:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zelidrag@chromium.org/10863002/21001

[commit-bot: I haz the power, 2012-08-23 01:29:07]

Presubmit check for 10863002-21001 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit Warnings **
License must match:
.*? Copyright (\(c\) )?(2012|2011|2010|2009|2008|2007|2006|2006-2008) The
Chromium Authors\. All rights reserved\.\n.*? Use of this source code is
governed by a BSD-style license that can be\n.*? found in the LICENSE file\.(?:
\*/)?\n
Found a bad license header in these files:
 
chrome/test/data/extensions/api_test/content_scripts/other_extensions/iframe_content.js
\
 
chrome/test/data/extensions/api_test/content_scripts/other_extensions/injector/background.js
\
 
chrome/test/data/extensions/api_test/content_scripts/other_extensions/injector/inject.js
\
 
chrome/test/data/extensions/api_test/content_scripts/other_extensions/injector/test.js
\
 
chrome/test/data/extensions/api_test/content_scripts/other_extensions/victim/background.js
\
 
chrome/test/data/extensions/api_test/content_scripts/other_extensions/victim/test.js

Presubmit checks took 1.7s to calculate.

[commit-bot: I haz the power, 2012-08-23 01:33:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zelidrag@chromium.org/10863002/22001

[commit-bot: I haz the power, 2012-08-23 03:07:28]

Try job failure for 10863002-22001 (retry) on linux_rel for step
"interactive_ui_tests".
It's a second try, previously, step "interactive_ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-08-23 03:16:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zelidrag@chromium.org/10863002/22001

[commit-bot: I haz the power, 2012-08-23 05:53:21]

Change committed as 152955