[Extensions OOPI] Update app window bindings for OOPI

extensions/renderer/app_window_custom_bindings.cc

Index: extensions/renderer/app_window_custom_bindings.cc
diff --git a/extensions/renderer/app_window_custom_bindings.cc b/extensions/renderer/app_window_custom_bindings.cc
index fa5b3dc3e61665467fc75226ae5a779125bc0d85..97406fc3ec0b2586611449ed118539692f81b071 100644
--- a/extensions/renderer/app_window_custom_bindings.cc
+++ b/extensions/renderer/app_window_custom_bindings.cc
@@ -9,10 +9,9 @@
 #include "base/command_line.h"
 #include "content/public/child/v8_value_converter.h"
 #include "content/public/renderer/render_frame.h"
+#include "content/public/renderer/render_frame_observer.h"
 #include "content/public/renderer/render_thread.h"
 #include "content/public/renderer/render_view.h"
-#include "content/public/renderer/render_view_observer.h"
-#include "content/public/renderer/render_view_visitor.h"
 #include "extensions/common/extension_messages.h"
 #include "extensions/common/switches.h"
 #include "extensions/renderer/dispatcher.h"
@@ -26,21 +25,22 @@
 
 namespace extensions {
 
-class DidCreateDocumentElementObserver : public content::RenderViewObserver {
+class DidCreateDocumentElementObserver : public content::RenderFrameObserver {
  public:
-  DidCreateDocumentElementObserver(content::RenderView* view,
+  DidCreateDocumentElementObserver(content::RenderFrame* frame,
                                    Dispatcher* dispatcher)

[not at google - send to devlin, 2015/07/08 20:22:20]

Can you change this to just pass in the ScriptContextSet, not the whole
Dispatcher? That way you can also remove the Dispatcher dependency entirely. You
only need the ScriptContextSet.

[Devlin, 2015/07/08 21:08:46]

Sure, done.

-      : content::RenderViewObserver(view), dispatcher_(dispatcher) {}
-
-  void DidCreateDocumentElement(blink::WebLocalFrame* frame) override {
-    DCHECK(frame);
+      : content::RenderFrameObserver(frame), dispatcher_(dispatcher) {
     DCHECK(dispatcher_);
+  }
+
+  void DidCreateDocumentElement() override {
+    blink::WebLocalFrame* web_frame = render_frame()->GetWebFrame();
     // Don't attempt to inject the titlebar into iframes.
-    if (frame->parent())
+    if (web_frame->parent())
       return;
     ScriptContext* script_context =
         dispatcher_->script_context_set().GetByV8Context(
-            frame->mainWorldScriptContext());
+            web_frame->mainWorldScriptContext());
     if (!script_context)
       return;
     script_context->module_system()->CallModuleMethod(
@@ -49,21 +49,22 @@ class DidCreateDocumentElementObserver : public content::RenderViewObserver {
 
  private:
   Dispatcher* dispatcher_;
+
+  DISALLOW_COPY_AND_ASSIGN(DidCreateDocumentElementObserver);
 };
 
 AppWindowCustomBindings::AppWindowCustomBindings(Dispatcher* dispatcher,
                                                  ScriptContext* context)
     : ObjectBackedNativeHandler(context), dispatcher_(dispatcher) {
-  RouteFunction("GetView",
-      base::Bind(&AppWindowCustomBindings::GetView,
-                 base::Unretained(this)));
+  RouteFunction("GetFrame", base::Bind(&AppWindowCustomBindings::GetFrame,
+                                       base::Unretained(this)));
 
   RouteFunction("GetWindowControlsHtmlTemplate",
       base::Bind(&AppWindowCustomBindings::GetWindowControlsHtmlTemplate,
                  base::Unretained(this)));
 }
 
-void AppWindowCustomBindings::GetView(
+void AppWindowCustomBindings::GetFrame(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
   // TODO(jeremya): convert this to IDL nocompile to get validation, and turn
   // these argument checks into CHECK().
@@ -76,36 +77,38 @@ void AppWindowCustomBindings::GetView(
   if (!args[1]->IsBoolean())
     return;
 
-  int view_id = args[0]->Int32Value();
+  int frame_id = args[0]->Int32Value();
 
   bool inject_titlebar = args[1]->BooleanValue();
 
-  if (view_id == MSG_ROUTING_NONE)
+  if (frame_id == MSG_ROUTING_NONE)
     return;
 
-  content::RenderView* view = content::RenderView::FromRoutingID(view_id);
-  if (!view)
+  content::RenderFrame* app_frame =
+      content::RenderFrame::FromRoutingID(frame_id);
+  if (!app_frame)
     return;
 
   if (inject_titlebar)
-    new DidCreateDocumentElementObserver(view, dispatcher_);
+    new DidCreateDocumentElementObserver(app_frame, dispatcher_);

[not at google - send to devlin, 2015/07/08 20:22:20]

I feel like this shouldn't happen until we know for sure the window is actually
returned to the caller. Unnecessary/wasteful otherwise.

[Devlin, 2015/07/08 21:08:46]

Done.

 
   // TODO(jeremya): it doesn't really make sense to set the opener here, but we
   // need to make sure the security origin is set up before returning the DOM
   // reference. A better way to do this would be to have the browser pass the
   // opener through so opener_id is set in RenderViewImpl's constructor.
-  content::RenderFrame* render_frame = context()->GetRenderFrame();
-  content::RenderView* render_view =
-      render_frame ? render_frame->GetRenderView() : nullptr;
-  if (!render_view)
+  content::RenderFrame* context_render_frame = context()->GetRenderFrame();
+  content::RenderView* context_render_view =
+      context_render_frame ? context_render_frame->GetRenderView() : nullptr;
+  if (!context_render_view)
     return;
-  blink::WebFrame* opener = render_view->GetWebView()->mainFrame();
-  blink::WebFrame* frame = view->GetWebView()->mainFrame();
-  frame->setOpener(opener);
+  blink::WebFrame* opener = context_render_view->GetWebView()->mainFrame();

[not at google - send to devlin, 2015/07/08 20:22:20]

I think the "opener" is equally correctly just
context()->GetRenderFrame()->GetWebFrame(). No need to dig into the RenderView.

[Devlin, 2015/07/08 21:08:46]

It is potentially a behavior change.  But let's try it and see what happens.

+  blink::WebLocalFrame* app_web_frame = app_frame->GetWebFrame();
+  app_web_frame->setOpener(opener);
   content::RenderThread::Get()->Send(
-      new ExtensionHostMsg_ResumeRequests(view->GetRoutingID()));
+      new ExtensionHostMsg_ResumeRequests(app_frame->GetRoutingID()));
 
-  v8::Local<v8::Value> window = frame->mainWorldScriptContext()->Global();
+  v8::Local<v8::Value> window =
+      app_web_frame->mainWorldScriptContext()->Global();
   args.GetReturnValue().Set(window);
 }