Switch RenderViewContextMenu to use RequestOpenURL

components/renderer_context_menu/render_view_context_menu_base.cc

 // Copyright 2014 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 "components/renderer_context_menu/render_view_context_menu_base.h"
 
 #include <algorithm>
 #include <utility>
 #include <vector>
 
@@ skipping 369 matching lines @@
     bool started_from_context_menu) {
   content::Referrer referrer = content::Referrer::SanitizeForRequest(
       url,
       content::Referrer(referring_url.GetAsReferrer(),
                         params_.referrer_policy));
 
   if (params_.link_url == url &&
       disposition != WindowOpenDisposition::OFF_THE_RECORD)
     params_.custom_context.link_followed = url;
 
   OpenURLParams open_url_params(url, referrer, disposition, transition, false,

[Charlie Reis, 2017/02/02 23:34:22]

Side note: The "false" parameter here is why this is not the same as a control
click from the renderer.  Context menu navigations are treated as
browser-initiated, and control clicks (etc) are treated as renderer-initiated. 
There's a big difference for security checks and the process model.

That said, I agree that this case should fire the observer method as well.

                                 started_from_context_menu);
   if (!extra_headers.empty())
     open_url_params.extra_headers = extra_headers;
 
-  WebContents* new_contents = source_web_contents_->OpenURL(open_url_params);
-  if (!new_contents)
-    return;
-
-  NotifyURLOpened(url, new_contents);

[Charlie Reis, 2017/02/02 23:34:22]

This looks like it would be dead code if we remove this call.  Does the
NOTIFICATION_RETARGETING happen somewhere else?  Is it safe to remove
NotifyURLOpened itself?

[nasko, 2017/02/07 19:39:34]

I don't think it is safe to remove. Last time I looked, this is needed for
webNavigation API to dispatch events for newly created WebContents.

[Patrick Noland, 2017/02/10 22:42:55]

Yes, NotifyURLOpened would be dead code and safe to remove. However
NOTIFICATION_RETARGETING is still emitted from Browser::WebContentsCreated. That
might be removable with some additional changes to
WebContentsImpl::CreateNewWindow.

+  source_web_contents_->RequestOpenURL(GetRenderFrameHost(), open_url_params);
 }
 
 bool RenderViewContextMenuBase::IsCustomItemChecked(int id) const {
   return IsCustomItemCheckedInternal(params_.custom_items, id);
 }
 
 bool RenderViewContextMenuBase::IsCustomItemEnabled(int id) const {
   return IsCustomItemEnabledInternal(params_.custom_items, id);
 }