New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 688214 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature



Sign in to add a comment

Enable RenderViewContextMenu's OpenURL to notify WebContentsObservers

Project Member Reported by pnoland@chromium.org, Feb 3 2017

Issue description

Currently, RenderViewContextMenuBase uses OpenURL, which ultimately doesn't call DidOpenRequestedURL on the source WebContents' observers. It would be nice if WebContentsObsevers were notified in this path, so that we could track "Open link in new tab" actions in the same way that ctrl-click and middle mouse click are tracked. 
 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/48894465b91ec112cbb9bd92e37f4c83010c14be

commit 48894465b91ec112cbb9bd92e37f4c83010c14be
Author: pnoland <pnoland@chromium.org>
Date: Wed Feb 22 18:58:54 2017

Switch RenderViewContextMenu to use RequestOpenURL

Currently, RenderViewContextMenu calls OpenURL on the source WebContents
when "open link in new tab" is activated. This is almost identical to
RequestOpenURL, except that it doesn't notify WebContentsObservers. This
change:

Changes NavigatorDelegate's RequestOpenURL to OpenURL, with a signature
identical to PageNavigator's OpenURL
Adds source_render_frame_host_id and source_render_process_id to OpenURLParams
Populates the above ids in NavigatorImpl's RequestOpenURL
Populates the above ids in RenderViewContextMenu's call to OpenURL
Removes RenderViewContextMenu's NotifyURLOpened

BUG= 688214 
R=nasko@chromium.org, creis@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2666193002
Cr-Commit-Position: refs/heads/master@{#452151}

[modify] https://crrev.com/48894465b91ec112cbb9bd92e37f4c83010c14be/chrome/browser/extensions/api/web_navigation/web_navigation_api.cc
[modify] https://crrev.com/48894465b91ec112cbb9bd92e37f4c83010c14be/chrome/browser/extensions/api/web_navigation/web_navigation_api.h
[modify] https://crrev.com/48894465b91ec112cbb9bd92e37f4c83010c14be/chrome/browser/renderer_context_menu/render_view_context_menu.cc
[modify] https://crrev.com/48894465b91ec112cbb9bd92e37f4c83010c14be/chrome/browser/renderer_context_menu/render_view_context_menu.h
[modify] https://crrev.com/48894465b91ec112cbb9bd92e37f4c83010c14be/components/renderer_context_menu/render_view_context_menu_base.cc
[modify] https://crrev.com/48894465b91ec112cbb9bd92e37f4c83010c14be/components/renderer_context_menu/render_view_context_menu_base.h
[modify] https://crrev.com/48894465b91ec112cbb9bd92e37f4c83010c14be/content/browser/frame_host/interstitial_page_impl.cc
[modify] https://crrev.com/48894465b91ec112cbb9bd92e37f4c83010c14be/content/browser/frame_host/interstitial_page_impl.h
[modify] https://crrev.com/48894465b91ec112cbb9bd92e37f4c83010c14be/content/browser/frame_host/navigator_delegate.h
[modify] https://crrev.com/48894465b91ec112cbb9bd92e37f4c83010c14be/content/browser/frame_host/navigator_impl.cc
[modify] https://crrev.com/48894465b91ec112cbb9bd92e37f4c83010c14be/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/48894465b91ec112cbb9bd92e37f4c83010c14be/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/48894465b91ec112cbb9bd92e37f4c83010c14be/content/public/browser/page_navigator.h
[modify] https://crrev.com/48894465b91ec112cbb9bd92e37f4c83010c14be/content/public/browser/web_contents_observer.h
[modify] https://crrev.com/48894465b91ec112cbb9bd92e37f4c83010c14be/content/test/web_contents_observer_sanity_checker.cc
[modify] https://crrev.com/48894465b91ec112cbb9bd92e37f4c83010c14be/content/test/web_contents_observer_sanity_checker.h

Status: Fixed (was: Started)

Sign in to add a comment