Add frameId to executeScript/insertCSS

chrome/browser/extensions/api/tabs/tabs_api.cc

Index: chrome/browser/extensions/api/tabs/tabs_api.cc
diff --git a/chrome/browser/extensions/api/tabs/tabs_api.cc b/chrome/browser/extensions/api/tabs/tabs_api.cc
index 4816da7e0687d8ce20a9b3f0af4717c47edc2252..01bfda0c61123f5157d34a51404db341e37fe017 100644
--- a/chrome/browser/extensions/api/tabs/tabs_api.cc
+++ b/chrome/browser/extensions/api/tabs/tabs_api.cc
@@ -59,6 +59,7 @@
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
+#include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_widget_host_view.h"
@@ -1244,7 +1245,8 @@ bool TabsUpdateFunction::UpdateURL(const std::string &url_string,
         extension_id(),
         ScriptExecutor::JAVASCRIPT,
         url.GetContent(),
-        ScriptExecutor::TOP_FRAME,
+        ScriptExecutor::ONE_FRAME,
+        0, // Top-level frame
         ScriptExecutor::DONT_MATCH_ABOUT_BLANK,
         UserScript::DOCUMENT_IDLE,
         ScriptExecutor::MAIN_WORLD,
@@ -1756,9 +1758,29 @@ bool ExecuteCodeInTabFunction::CanExecuteScriptOnPage() {
   // NOTE: This can give the wrong answer due to race conditions, but it is OK,
   // we check again in the renderer.
   content::RenderProcessHost* process = contents->GetRenderProcessHost();

[not at google - send to devlin, 2015/02/23 20:41:23]

@anybody on the site isolation team:

How can |process| possibly be null? Surely every WebContents has a process?

+  content::RenderFrameHost* rfh = nullptr;
+  int frame_id = details_->frame_id.get() ? *details_->frame_id : 0;

[dcheng, 2015/02/23 16:16:01]

details_->frame_id ? *details_->frame_id : 0

[not at google - send to devlin, 2015/02/23 20:41:23]

I actually find the ternary-if here looks a bit weird anyway. Can you make the
condition below (process && details_->frame_id && *details_->frame_id > 0)

[robwu, 2015/02/23 21:54:05]

Done.
frameId is used three times in the next few lines, so removing the variable will
reduce the readability of the code.

+  if (process && frame_id > 0) {
+    // TODO(robwu): This method will not work with OOPI, because the frames
+    // in a tab may be hosted within different processes (crbug.com/432875).
+    rfh = content::RenderFrameHost::FromID(process->GetID(), frame_id);
+    if (!rfh) {
+      error_ = ErrorUtils::FormatErrorMessage(
+          keys::kFrameNotFoundError, base::IntToString(frame_id),
+          base::IntToString(execute_tab_id_));
+      return false;
+    }
+  }
+  GURL document_url = rfh ? rfh->GetLastCommittedURL() : contents->GetURL();

[not at google - send to devlin, 2015/02/23 20:41:23]

All of these GetURL() calls should be GetLastCommittedURL().

[robwu, 2015/02/23 21:54:05]

Done (also in a few other places within this file). This change will also fix
https://crbug.com/396830#c21

+  if (document_url.SchemeIs(url::kAboutScheme)) {
+    // Whether access is allowed depends on the frame's effective origin
+    // (see ScriptContext::GetEffectiveDocumentURL). Let the renderer decide
+    // whether to execute the script.
+    return true;
+  }
   if (!extension()->permissions_data()->CanAccessPage(
           extension(),
-          contents->GetURL(),
+          document_url,
           contents->GetURL(),
           execute_tab_id_,
           process ? process->GetID() : -1,