Respect the clipboardRead and clipboardWrite permissions in content scripts.

extensions/renderer/dispatcher.cc

Index: extensions/renderer/dispatcher.cc
diff --git a/extensions/renderer/dispatcher.cc b/extensions/renderer/dispatcher.cc
index d52d1505c06bfe2a7da38acde2639c831bf5fa29..20acee89b1e235a4ba292e463007f46cb595f6fd 100644
--- a/extensions/renderer/dispatcher.cc
+++ b/extensions/renderer/dispatcher.cc
@@ -219,7 +219,9 @@ bool Dispatcher::IsExtensionActive(const std::string& extension_id) const {
   return is_active;
 }
 
-std::string Dispatcher::GetExtensionID(const WebFrame* frame, int world_id) {
+std::string Dispatcher::GetExtensionID(const WebFrame* frame,
+                                       int world_id,
+                                       bool use_effective_url) {
   if (world_id != 0) {
     // Isolated worlds (content script).
     return ScriptInjection::GetExtensionIdForIsolatedWorld(world_id);
@@ -231,6 +233,8 @@ std::string Dispatcher::GetExtensionID(const WebFrame* frame, int world_id) {
 
   // Extension pages (chrome-extension:// URLs).
   GURL frame_url = ScriptContext::GetDataSourceURLForFrame(frame);
+  frame_url = ScriptContext::GetEffectiveDocumentURL(
+      frame, frame_url, use_effective_url);
   return extensions_.GetExtensionOrAppIDByURL(frame_url);
 }
 
@@ -243,7 +247,7 @@ void Dispatcher::DidCreateScriptContext(
   return;
 #endif
 
-  std::string extension_id = GetExtensionID(frame, world_id);
+  std::string extension_id = GetExtensionID(frame, world_id, false);
 
   const Extension* extension = extensions_.GetByID(extension_id);

[Devlin, 2014/09/03 22:09:16]

This is unnecessarily complicated.  We should just make it GetExtensionFromWorld
(or some other name) and have it return the extension
(ExtensionSet::GetExtensionOrAppIDByURL already looks up the full extension
anyway).

[Marijn Kruisselbrink, 2014/09/03 23:54:07]

Done, although I'm not entirely sure if it really was unnecessarily? I'm not
sure under which conditions "despite a context being associated with an
extension, no extension actually gets found", but by using
GetExtensionOrAppByURL instead of GetExtensionOrAppIDByURL followed by looking
up the extension by ID, I've eliminated at least one code paths that could
before have resulted in this LOG(ERROR). (I could of course change
GetExtensionFromFrameAndWorld back to first get the ID, but that doesn't really
seem much of an improvement compared to just leaving the code as it is).
So I'm tempted to undo the renaming/changing to GetExtensionFromFrameAndWorld
and instead go back to GetExtensionID

[Devlin, 2014/09/04 19:15:10]

Good point, so I don't really feel too strongly either way.

[Marijn Kruisselbrink, 2014/09/04 23:41:11]

Actually I kind of like having this GetExtensionFromFrameAndWorld method; I now
slightly rewrote it to keep the old behavior by first looking up the extension
id.

   if (!extension && !extension_id.empty()) {
@@ -260,15 +264,28 @@ void Dispatcher::DidCreateScriptContext(
     extension_id = "";
   }
 
+  extension_id = GetExtensionID(frame, world_id, true);
+  const Extension* effective_extension = extensions_.GetByID(extension_id);

[Devlin, 2014/09/03 22:09:16]

If I recall correctly, GetEffectiveDocumentURL() only changes from frame_url if
it's a non-sandboxed about:blank. If this is the case, then |extension| on line
252 is almost certainly NULL, right?  (No extension will have an associated url
of about:blank.)  So if that's the case, do we really need both
|effective_extension| and |extension| in the ScriptContext, or could we just
make |extension| the effective extension?  Would that break things elsewhere?

[Marijn Kruisselbrink, 2014/09/03 23:54:07]

Yes, the ideal end state is to get rid of extension and only have
effective_extension. But, quoting kalman:
"Just changing that will change everything (the "ideal" case as I put it in the
bug). But I'm scared, and we'd need to do an audit of everywhere we use [it].

The hackier/conservative fix would be to store 2 Extensions in ScriptContext,
one derived from GetDataSourceURLForFrame and the other from
GetEffectiveDocumentURL, then use the latter in a CanClipboardRead/Write
method:"

[Devlin, 2014/09/04 19:15:10]

Ah.  That's too bad.  Okay.

+
+  GURL frame_url = ScriptContext::GetDataSourceURLForFrame(frame);
   Feature::Context context_type =
       ClassifyJavaScriptContext(extension,
                                 extension_group,
-                                ScriptContext::GetDataSourceURLForFrame(frame),
+                                frame_url,
                                 frame->document().securityOrigin());
+  Feature::Context effective_context_type = ClassifyJavaScriptContext(
+      effective_extension,
+      extension_group,
+      ScriptContext::GetEffectiveDocumentURL(frame, frame_url, true),
+      frame->document().securityOrigin());
 
   ScriptContext* context =
-      delegate_->CreateScriptContext(v8_context, frame, extension, context_type)
-          .release();
+      delegate_->CreateScriptContext(v8_context,
+                                     frame,
+                                     extension,
+                                     context_type,
+                                     effective_extension,
+                                     effective_context_type).release();
   script_context_set_.Add(context);
 
   // Initialize origin permissions for content scripts, which can't be