[Extensions] Use document url (not top url) for tab-specific permissions

extensions/browser/extension_web_contents_observer.cc

Index: extensions/browser/extension_web_contents_observer.cc
diff --git a/extensions/browser/extension_web_contents_observer.cc b/extensions/browser/extension_web_contents_observer.cc
index 04335a527b8531cb9c7e3aac456528117b7be9ad..c9c8174d3559a23284206ddf904f4501736f2bf9 100644
--- a/extensions/browser/extension_web_contents_observer.cc
+++ b/extensions/browser/extension_web_contents_observer.cc
@@ -142,10 +142,19 @@ void ExtensionWebContentsObserver::RenderFrameHostChanged(
   if (old_host)
     process_manager->UnregisterRenderFrameHost(old_host);
 
-  const Extension* extension = GetExtension(new_host->GetRenderViewHost());
-  if (extension) {
+  const Extension* frame_extension = GetExtensionForRenderFrame(new_host);
+  if (frame_extension) {
     process_manager->RegisterRenderFrameHost(
-        web_contents(), new_host, extension);
+        web_contents(), new_host, frame_extension);
+  }
+
+  // This can be different from |frame_extension| above in the case of, e.g.,
+  // a non-extension iframe hosted in a chrome-extension:// page.
+  const Extension* tab_extension =
+      GetExtensionForRenderFrame(web_contents()->GetMainFrame());
+  if (tab_extension) {
+    new_host->Send(new ExtensionMsg_SetTabExtensionOwner(

[not at google - send to devlin, 2015/06/03 20:15:44]

creis/nasko -

Firstly, this is basically a side channel for sending down the URL of the top
frame. From a site isolation perspective it seems a slight regression, but
knowing the top URL isn't drastically bad, right?

Secondly, if we need to do this, perhaps other people will, and may it make
sense to include it as part of the content API?

Thirdly, Devlin, you could moot these issues by sending the IDs of the user
scripts to execute rather than the data needed to make that decision on the
renderer, but I think you already know that. Perhaps worth noting in a TODO
somewhere.

[Devlin, 2015/06/03 20:28:47]

A bit more background - we prevent script injection into any frame of an
extension page, and previously did that based on web_frame->top->document->url,
but with OOPI, that obviously fails.  I don't know that I agree that this is a
regression - at least, no more so than something like tab id (they're both
tab-wide attributes that cause interesting cases in OOPI).

Re your third point, yes, and we obviously want to work towards that - but
that's a pretty massive change from this, with its own set of fun craziness
(this is one of the things I wanna talk about up in SEA next week).

[nasko, 2015/06/03 20:31:56]

It isn't obvious to me that you are sending the URL. Can you clarify how that
happens?
Ideally we don't want to replicate the URL down to the renderer. Unfortunately,
we have to do it for the top-level frame due to another issue, so alexmos@ is
working on a CL as we speak to do this (writing the test and sending out for
review). 

Looking over the CL, this looks much cleaner (using tab ids instead of URLs). So
even if you had the URL for the top frame, I'd prefer this.
See above.

[Devlin, 2015/06/03 21:07:42]

If (and only if) this is an extension page (i.e, a page with a
chrome-extension:// scheme), we send down the id of the extension that "owns"
the page. Technically, this isn't sending the top url (and conceptually, it's
quite a bit cleaner), but it's still in the same neighborhood - which is what
Ben was talking about.
 
I'm a little confused on the tab id part, because we don't use them here. :)  We
send down the extension id if this is an extension page, and use that to
determine if we need to block script injection based on the top frame -
something we only do for extension pages.

That said, Ben said he's actually happy enough with this (my explanation
helped), so if your LG stands, I'll go ahead and push it through.

[Charlie Reis, 2015/06/03 21:12:03]

I don't have the full context here, but we do already replicate the origin
(which contains the extension ID) to each renderer process in
FrameReplicationState.  Would that be sufficient?

[Devlin, 2015/06/03 21:23:51]

Now I'm out of context. :)  Looking at FrameReplicationState, I don't think it's
sufficient as-is.  The origin is sufficient, but we need a way of accessing the
main frame's origin from a separate process.  I can't quite follow all of the
happenings, but it looks like FrameReplicationState is for the render frame in
that process, and only accessible at creation?  And, of course, we need access
from the chrome layer, and FrameReplicationState is private to content.

[Charlie Reis, 2015/06/03 21:34:42]

No, it exists on RenderFrameProxy as well, so you can get it even for the main
frame.  It's meant for cases like this, and Alex would be a good person to chat
with about it.
Oh yeah...  We haven't exposed it yet, and RenderFrameProxies aren't exposed
either.  Maybe we should put that idea on hold until your visit next week, and
you can proceed with this in the meantime.

+        new_host->GetRoutingID(), tab_extension->id()));
   }
 }