[Extensions OOPI] Update app window bindings for OOPI

extensions/browser/api/app_window/app_window_api.cc

Index: extensions/browser/api/app_window/app_window_api.cc
diff --git a/extensions/browser/api/app_window/app_window_api.cc b/extensions/browser/api/app_window/app_window_api.cc
index c14d3b1b51fd31887105a7ca4cbcca280dec812a..e2cafcc70d56ef1aa17dbe94001a9ed113dfbcf2 100644
--- a/extensions/browser/api/app_window/app_window_api.cc
+++ b/extensions/browser/api/app_window/app_window_api.cc
@@ -13,7 +13,6 @@
 #include "content/public/browser/notification_types.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/web_contents.h"
 #include "content/public/common/url_constants.h"
 #include "extensions/browser/app_window/app_window.h"
@@ -175,12 +174,12 @@ bool AppWindowCreateFunction::RunAsync() {
                                 ->GetAppWindowForAppAndKey(
                                     extension_id(), create_params.window_key);
         if (window) {
-          content::RenderViewHost* created_view =
-              window->web_contents()->GetRenderViewHost();
-          int view_id = MSG_ROUTING_NONE;
+          content::RenderFrameHost* created_frame =

[not at google - send to devlin, 2015/07/08 21:34:29]

"created_frame" is actually a lie. The whole point of this code is that you
*didn't* create any frame, you're getting one that already existed. Maybe call
it "main_frame" or something.

Or "existing_frame". I'd like to see "window" renamed "existing_window". 

[Devlin, 2015/07/08 21:47:01]

Done.

+              window->web_contents()->GetMainFrame();
+          int frame_id = MSG_ROUTING_NONE;
           if (render_frame_host()->GetProcess()->GetID() ==
-              created_view->GetProcess()->GetID()) {
-            view_id = created_view->GetRoutingID();
+              created_frame->GetProcess()->GetID()) {

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

Why does the API do this? Why not unconditionally set the routing ID? Same as
below.

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

My guess is because if they are in different processes (though I'm not sure
when/how that happens), sending over the frame id is wrong.  Frame ids are
unique in a process, but not (I think) across processes.  Since we respond to
render_frame_host(), if it is in a different process, it could (hypothetically)
get a completely wrong frame.

[not at google - send to devlin, 2015/07/08 21:34:29]

Yeah I feel like this should be a CHECK that they're the same process, because
it's an invariant that they are. The only process that can call
app-window-create is the app process itself, which is where the window is
created. It's misleading to do this check.

Or at least a DCHECK.

Also you should be able to compare GetProcess() calls directly (i.e. pointer
equality), no need to grab their IDs.

And, the same deal below. In fact it's sad that this code and the other don't
share much code, because they do basically the same thing.

[Devlin, 2015/07/08 21:47:01]

As discovered offline, this was added because component apps can create app
windows for other apps (in which case this won't be true).

I don't wanna dive into it here, since that's not the point of this CL, but I
added a TODO to look into it (and potentially kill it).

+            frame_id = created_frame->GetRoutingID();
           }
 
           if (!options->hidden.get() || !*options->hidden.get()) {
@@ -191,7 +190,7 @@ bool AppWindowCreateFunction::RunAsync() {
           }
 
           base::DictionaryValue* result = new base::DictionaryValue;
-          result->Set("viewId", new base::FundamentalValue(view_id));
+          result->Set("frameId", new base::FundamentalValue(frame_id));
           window->GetSerializedState(result);
           result->SetBoolean("existingWindow", true);
           // TODO(benwells): Remove HTML titlebar injection.
@@ -344,14 +343,14 @@ bool AppWindowCreateFunction::RunAsync() {
     app_window->ForcedFullscreen();
   }
 
-  content::RenderViewHost* created_view =
-      app_window->web_contents()->GetRenderViewHost();
-  int view_id = MSG_ROUTING_NONE;
-  if (create_params.creator_process_id == created_view->GetProcess()->GetID())
-    view_id = created_view->GetRoutingID();
+  content::RenderFrameHost* created_frame =
+      app_window->web_contents()->GetMainFrame();
+  int frame_id = MSG_ROUTING_NONE;
+  if (create_params.creator_process_id == created_frame->GetProcess()->GetID())
+    frame_id = created_frame->GetRoutingID();
 
   base::DictionaryValue* result = new base::DictionaryValue;
-  result->Set("viewId", new base::FundamentalValue(view_id));
+  result->Set("frameId", new base::FundamentalValue(frame_id));
   result->Set("injectTitlebar",
       new base::FundamentalValue(inject_html_titlebar_));
   result->Set("id", new base::StringValue(app_window->window_key()));
@@ -359,7 +358,7 @@ bool AppWindowCreateFunction::RunAsync() {
   SetResult(result);
 
   if (AppWindowRegistry::Get(browser_context())
-          ->HadDevToolsAttached(created_view)) {
+          ->HadDevToolsAttached(app_window->web_contents())) {
     AppWindowClient::Get()->OpenDevToolsWindow(
         app_window->web_contents(),
         base::Bind(&AppWindowCreateFunction::SendResponse, this, true));