[Extensions Bindings] Pull v8 frame fetching in ExtensionFrameHelper

The ExtensionFrameHelper already has logic to find extension frames
matching certain criteria, which is used by custom bindings to return
the global objects for those frames. Move the logic to get the v8
global objects from the custom bindings to the ExtensionFrameHelper in
order to allow re-use with other bindings.

Bug: 653596

Change-Id: Ibb7d57e90bdb83cd574a95eb80db222456dd3e79
Reviewed-on: https://chromium-review.googlesource.com/763572
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517151}

extensions/renderer/extension_frame_helper.cc

@@ -130,6 +130,39 @@ std::vector<content::RenderFrame*> ExtensionFrameHelper::GetExtensionFrames(
   return render_frames;
 }
 
+// static
+v8::Local<v8::Array> ExtensionFrameHelper::GetV8MainFrames(
+    v8::Local<v8::Context> context,
+    const std::string& extension_id,
+    int browser_window_id,
+    int tab_id,
+    ViewType view_type) {
+  std::vector<content::RenderFrame*> render_frames =
+      GetExtensionFrames(extension_id, browser_window_id, tab_id, view_type);
+  v8::Local<v8::Array> v8_frames = v8::Array::New(context->GetIsolate());
+
+  int v8_index = 0;
+  for (content::RenderFrame* frame : render_frames) {
+    if (!frame->IsMainFrame())
+      continue;
+
+    blink::WebLocalFrame* web_frame = frame->GetWebFrame();
+    if (!blink::WebFrame::ScriptCanAccess(web_frame))
+      continue;
+
+    v8::Local<v8::Context> frame_context = web_frame->MainWorldScriptContext();
+    if (!frame_context.IsEmpty()) {
+      v8::Local<v8::Value> window = frame_context->Global();
+      CHECK(!window.IsEmpty());
+      v8::Maybe<bool> maybe =
+          v8_frames->CreateDataProperty(context, v8_index++, window);
+      CHECK(maybe.IsJust() && maybe.FromJust());
+    }
+  }
+
+  return v8_frames;
+}
+
 // static
 content::RenderFrame* ExtensionFrameHelper::GetBackgroundPageFrame(
     const std::string& extension_id) {

extensions/renderer/extension_frame_helper.h

@@ -14,6 +14,7 @@
 #include "content/public/renderer/render_frame_observer.h"
 #include "content/public/renderer/render_frame_observer_tracker.h"
 #include "extensions/common/view_type.h"
+#include "v8/include/v8.h"
 
 struct ExtensionMsg_ExternalConnectionInfo;
 struct ExtensionMsg_TabConnectionInfo;
@@ -46,6 +47,14 @@ class ExtensionFrameHelper
       int browser_window_id,
       int tab_id,
       ViewType view_type);
+  // Same as above, but returns a v8::Array of the v8 global objects for those
+  // frames, and only includes main frames.
+  // Returns an empty v8::Array if no frames are found.
+  static v8::Local<v8::Array> GetV8MainFrames(v8::Local<v8::Context> context,
+                                              const std::string& extension_id,
+                                              int browser_window_id,
+                                              int tab_id,
+                                              ViewType view_type);
 
   // Returns the main frame of the extension's background page, or null if there
   // isn't one in this process.

extensions/renderer/runtime_custom_bindings.cc

@@ -17,7 +17,6 @@
 #include "extensions/common/manifest.h"
 #include "extensions/renderer/extension_frame_helper.h"
 #include "extensions/renderer/script_context.h"
-#include "third_party/WebKit/public/web/WebLocalFrame.h"
 
 namespace extensions {
 
@@ -67,33 +66,15 @@ void RuntimeCustomBindings::GetExtensionViews(
   if (extension_id.empty())
     return;
 
-  std::vector<content::RenderFrame*> frames =
-      ExtensionFrameHelper::GetExtensionFrames(extension_id, browser_window_id,
-                                               tab_id, view_type);
   v8::Local<v8::Context> v8_context = args.GetIsolate()->GetCurrentContext();
-  v8::Local<v8::Array> v8_views = v8::Array::New(args.GetIsolate());
-  int v8_index = 0;
-  for (content::RenderFrame* frame : frames) {
-    // We filter out iframes here. GetExtensionViews should only return the
-    // main views, not any subframes. (Returning subframes can cause broken
-    // behavior by treating an app window's iframe as its main frame, and maybe
-    // other nastiness).
-    blink::WebLocalFrame* web_frame = frame->GetWebFrame();
-    if (web_frame->Top() != web_frame)
-      continue;
-
-    if (!blink::WebFrame::ScriptCanAccess(web_frame))
-      continue;
-
-    v8::Local<v8::Context> context = web_frame->MainWorldScriptContext();
-    if (!context.IsEmpty()) {
-      v8::Local<v8::Value> window = context->Global();
-      CHECK(!window.IsEmpty());
-      v8::Maybe<bool> maybe =
-          v8_views->CreateDataProperty(v8_context, v8_index++, window);
-      CHECK(maybe.IsJust() && maybe.FromJust());
-    }
-  }
+  // We ignore iframes here. (Returning subframes can cause broken behavior by
+  // treating an app window's iframe as its main frame, and maybe other
+  // nastiness).
+  // TODO(devlin): Why wouldn't we just account for that? It seems like there
+  // can be reasons to want to access just a frame - especially with isolated
+  // extension frames in web pages.
+  v8::Local<v8::Array> v8_views = ExtensionFrameHelper::GetV8MainFrames(
+      v8_context, extension_id, browser_window_id, tab_id, view_type);
 
   args.GetReturnValue().Set(v8_views);
 }