Switch RenderViewContextMenu to use RequestOpenURL

content/browser/web_contents/web_contents_impl.cc

 // 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.
 
 #include "content/browser/web_contents/web_contents_impl.h"
 
 #include <stddef.h>
 
 #include <cmath>
 #include <utility>
@@ skipping 2557 matching lines @@
 
   if (delegate_)
     delegate_->ResizeDueToAutoResize(this, new_size);
 }
 
 WebContents* WebContentsImpl::OpenURL(const OpenURLParams& params) {
   if (!delegate_)
     return NULL;
 
   WebContents* new_contents = delegate_->OpenURLFromTab(this, params);
+
+  RenderFrameHost* render_frame_host =
+      RenderFrameHost::FromID(params.render_process_id, params.render_frame_id);
+

[nasko, 2017/02/14 01:20:11]

I realized we discussed one thing I forgot to mention in my previous review.

Since we are now supplying a source RenderFrameHost and a source SiteInstance,
we should ensure that they are matching, otherwise mismatches will likely be
problematic. Let's put a DCHECK or a CHECK that the SiteInstance of the source
RenderFrameHost, if supplied, matches the source SiteInstance, if supplied.

[Patrick Noland, 2017/02/14 21:33:59]

Done.

+  if (new_contents && render_frame_host && new_contents != this) {
+    for (auto& observer : observers_) {
+      observer.DidOpenRequestedURL(new_contents, render_frame_host, params.url,
+                                   params.referrer, params.disposition,
+                                   params.transition,
+                                   params.started_from_context_menu);
+    }
+  }
+
   return new_contents;
 }
 
 bool WebContentsImpl::Send(IPC::Message* message) {
   if (!GetRenderViewHost()) {
     delete message;
     return false;
   }
 
   return GetRenderViewHost()->Send(message);
@@ skipping 773 matching lines @@
   NotifyNavigationStateChanged(changed_flags);
 }
 
 void WebContentsImpl::DidStartNavigationToPendingEntry(const GURL& url,
                                                        ReloadType reload_type) {
   // Notify observers about navigation.
   for (auto& observer : observers_)
     observer.DidStartNavigationToPendingEntry(url, reload_type);
 }
 
-void WebContentsImpl::RequestOpenURL(RenderFrameHostImpl* render_frame_host,
-                                     const OpenURLParams& params) {
-  // OpenURL can blow away the source RFH. Use the process/frame routing ID as a
-  // weak pointer of sorts.
-  const int32_t process_id = render_frame_host->GetProcess()->GetID();
-  const int32_t frame_id = render_frame_host->GetRoutingID();
-
-  WebContents* new_contents = OpenURL(params);
-
-  if (new_contents && RenderFrameHost::FromID(process_id, frame_id)) {
-    // Notify observers.
-    for (auto& observer : observers_) {
-      observer.DidOpenRequestedURL(new_contents, render_frame_host, params.url,
-                                   params.referrer, params.disposition,
-                                   params.transition);
-    }
-  }
-}
-
 bool WebContentsImpl::ShouldTransferNavigation(bool is_main_frame_navigation) {
   if (!delegate_)
     return true;
   return delegate_->ShouldTransferNavigation(is_main_frame_navigation);
 }
 
 bool WebContentsImpl::ShouldPreserveAbortedURLs() {
   if (!delegate_)
     return false;
   return delegate_->ShouldPreserveAbortedURLs(this);
@@ skipping 1975 matching lines @@
   GetMainFrame()->AddMessageToConsole(
       content::CONSOLE_MESSAGE_LEVEL_WARNING,
       base::StringPrintf("This site does not have a valid SSL "
                          "certificate! Without SSL, your site's and "
                          "visitors' data is vulnerable to theft and "
                          "tampering. Get a valid SSL certificate before"
                          " releasing your website to the public."));
 }
 
 }  // namespace content