OOPIF: Fix window.open to work from frames with remote parent.

content/browser/renderer_host/render_view_host_delegate.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CONTENT_BROWSER_RENDERER_HOST_RENDER_VIEW_HOST_DELEGATE_H_
 #define CONTENT_BROWSER_RENDERER_HOST_RENDER_VIEW_HOST_DELEGATE_H_
 
 #include <string>
 
 #include "base/basictypes.h"
@@ skipping 177 matching lines @@
   // Requests to lock the mouse. Once the request is approved or rejected,
   // GotResponseToLockMouseRequest() will be called on the requesting render
   // view host.
   virtual void RequestToLockMouse(bool user_gesture,
                                   bool last_unlocked_by_target) {}
 
   // Notification that the view has lost the mouse lock.
   virtual void LostMouseLock() {}
 
   // The page is trying to open a new page (e.g. a popup window). The window
-  // should be created associated with the given |route_id| in process
-  // |render_process_id|, but it should not be shown yet. That should happen in
-  // response to ShowCreatedWindow.
-  // |params.window_container_type| describes the type of RenderViewHost
-  // container that is requested -- in particular, the window.open call may
-  // have specified 'background' and 'persistent' in the feature string.
+  // should be created associated with the given |route_id| in SiteInstance

[Charlie Reis, 2015/07/09 23:42:51]

nit: in the process of |source_site_instance|,

(Routing IDs are always process specific, so it's worth spelling out that the
process in this case is the one from the SiteInstance.)

[alexmos, 2015/07/10 18:35:00]

Done.

+  // |source_site_instance|, but it should not be shown yet. That should happen
+  // in response to ShowCreatedWindow. |params.window_container_type| describes
+  // the type of RenderViewHost container that is requested -- in particular,
+  // the window.open call may have specified 'background' and 'persistent' in
+  // the feature string.
   //
   // The passed |params.frame_name| parameter is the name parameter that was
   // passed to window.open(), and will be empty if none was passed.
   //
   // Note: this is not called "CreateWindow" because that will clash with
   // the Windows function which is actually a #define.
   virtual void CreateNewWindow(

[Charlie Reis, 2015/07/09 23:42:51]

General question: does this belong on RenderFrameHost(Delegate) instead?  It
doesn't seem to be particularly view-specific, and the message is going to come
from a frame.

That could be done in a separate CL, but we could leave a TODO here if it's the
right plan.

[alexmos, 2015/07/10 18:35:00]

Yeah, I agree this should be moved to RenderFrameHostDelegate, and the
corresponding ViewHostMsg_CreateWindow should really be a FrameHostMsg sent to
the creator frame's RFH.  Added a TODO.

-      int render_process_id,
+      SiteInstance* source_site_instance,
       int route_id,
       int main_frame_route_id,
       const ViewHostMsg_CreateWindow_Params& params,
       SessionStorageNamespace* session_storage_namespace) {}
 
   // The page is trying to open a new widget (e.g. a select popup). The
   // widget should be created associated with the given |route_id| in the
   // process |render_process_id|, but it should not be shown yet. That should
   // happen in response to ShowCreatedWidget.
   // |popup_type| indicates if the widget is a popup and what kind of popup it
@@ skipping 49 matching lines @@
   virtual void SetIsVirtualKeyboardRequested(bool requested) {}
   virtual bool IsVirtualKeyboardRequested();
 
  protected:
   virtual ~RenderViewHostDelegate() {}
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_RENDERER_HOST_RENDER_VIEW_HOST_DELEGATE_H_