Commit 865f085a authored by John Delaney's avatar John Delaney Committed by Commit Bot

[conversions] Support impressions on context-menu navs

This CL attaches impressions to context menu navigations that occur
from anchor tags with impression attributes.

This also declares the frame the context menu occurred in as the
initiator routing frame for the navigation, which was previously null.

Bug: 1049674
Change-Id: I7a2031b41865d44bc926c87af1fba86aa72048f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2240198
Commit-Queue: John Delaney <johnidel@chromium.org>
Reviewed-by: default avatarKen Buchanan <kenrb@chromium.org>
Reviewed-by: default avatarNate Chapin <japhet@chromium.org>
Reviewed-by: default avatarCharlie Harrison <csharrison@chromium.org>
Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790625}
parent fcf7ad0c
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "chrome/browser/renderer_context_menu/render_view_context_menu.h" #include "chrome/browser/renderer_context_menu/render_view_context_menu.h"
#include "base/bind.h" #include "base/bind.h"
#include "base/optional.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
...@@ -27,7 +28,12 @@ ...@@ -27,7 +28,12 @@
#include "components/prefs/pref_registry_simple.h" #include "components/prefs/pref_registry_simple.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "components/proxy_config/proxy_config_pref_names.h" #include "components/proxy_config/proxy_config_pref_names.h"
#include "content/public/browser/global_routing_id.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "content/public/browser/web_contents_delegate.h"
#include "content/public/common/impression.h"
#include "content/public/test/test_renderer_host.h" #include "content/public/test/test_renderer_host.h"
#include "content/public/test/web_contents_tester.h" #include "content/public/test/web_contents_tester.h"
#include "extensions/browser/extension_prefs.h" #include "extensions/browser/extension_prefs.h"
...@@ -97,6 +103,26 @@ std::unique_ptr<TestRenderViewContextMenu> CreateContextMenu( ...@@ -97,6 +103,26 @@ std::unique_ptr<TestRenderViewContextMenu> CreateContextMenu(
return menu; return menu;
} }
class TestNavigationDelegate : public content::WebContentsDelegate {
public:
TestNavigationDelegate() = default;
~TestNavigationDelegate() override = default;
content::WebContents* OpenURLFromTab(
content::WebContents* source,
const content::OpenURLParams& params) override {
last_navigation_params_ = params;
return nullptr;
}
const base::Optional<content::OpenURLParams>& last_navigation_params() {
return last_navigation_params_;
}
private:
base::Optional<content::OpenURLParams> last_navigation_params_;
};
} // namespace } // namespace
class RenderViewContextMenuTest : public testing::Test { class RenderViewContextMenuTest : public testing::Test {
...@@ -518,6 +544,32 @@ TEST_F(RenderViewContextMenuPrefsTest, SaveMediaSuggestedFileName) { ...@@ -518,6 +544,32 @@ TEST_F(RenderViewContextMenuPrefsTest, SaveMediaSuggestedFileName) {
EXPECT_EQ(kTestSuggestedFileName, suggested_filename); EXPECT_EQ(kTestSuggestedFileName, suggested_filename);
} }
// Verify ContextMenu navigations properly set the initiator routing id for a
// frame.
TEST_F(RenderViewContextMenuPrefsTest, OpenLinkNavigationParamsSet) {
TestNavigationDelegate delegate;
web_contents()->SetDelegate(&delegate);
content::RenderFrameHost* main_frame = web_contents()->GetMainFrame();
content::ContextMenuParams params = CreateParams(MenuItem::LINK);
params.unfiltered_link_url = params.link_url;
params.link_url = params.link_url;
params.impression = content::Impression();
auto menu = std::make_unique<TestRenderViewContextMenu>(main_frame, params);
menu->ExecuteCommand(IDC_CONTENT_CONTEXT_OPENLINKNEWTAB, 0);
EXPECT_TRUE(delegate.last_navigation_params());
// Verify that the ContextMenu source frame is set as the navigation
// initiator.
auto main_frame_id = content::GlobalFrameRoutingId(
main_frame->GetProcess()->GetID(), main_frame->GetFrameTreeNodeId());
EXPECT_EQ(main_frame_id,
delegate.last_navigation_params()->initiator_routing_id);
// Verify that the impression is attached to the navigation.
EXPECT_TRUE(delegate.last_navigation_params()->impression);
}
// Verify that "Show all passwords" is displayed on a password field. // Verify that "Show all passwords" is displayed on a password field.
TEST_F(RenderViewContextMenuPrefsTest, ShowAllPasswords) { TEST_F(RenderViewContextMenuPrefsTest, ShowAllPasswords) {
// Set up password manager stuff. // Set up password manager stuff.
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "base/command_line.h" #include "base/command_line.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "content/public/browser/global_routing_id.h"
#include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_frame_host.h"
#include "content/public/browser/render_process_host.h" #include "content/public/browser/render_process_host.h"
#include "content/public/browser/render_view_host.h" #include "content/public/browser/render_view_host.h"
...@@ -23,6 +24,7 @@ ...@@ -23,6 +24,7 @@
using blink::WebString; using blink::WebString;
using blink::WebURL; using blink::WebURL;
using content::BrowserContext; using content::BrowserContext;
using content::GlobalFrameRoutingId;
using content::OpenURLParams; using content::OpenURLParams;
using content::RenderFrameHost; using content::RenderFrameHost;
using content::RenderViewHost; using content::RenderViewHost;
...@@ -465,6 +467,12 @@ void RenderViewContextMenuBase::OpenURLWithExtraHeaders( ...@@ -465,6 +467,12 @@ void RenderViewContextMenuBase::OpenURLWithExtraHeaders(
open_url_params.source_render_process_id = render_process_id_; open_url_params.source_render_process_id = render_process_id_;
open_url_params.source_render_frame_id = render_frame_id_; open_url_params.source_render_frame_id = render_frame_id_;
open_url_params.initiator_routing_id =
GlobalFrameRoutingId(render_process_id_, render_frame_id_);
if (disposition != WindowOpenDisposition::OFF_THE_RECORD)
open_url_params.impression = params_.impression;
source_web_contents_->OpenURL(open_url_params); source_web_contents_->OpenURL(open_url_params);
} }
......
...@@ -19,11 +19,14 @@ ...@@ -19,11 +19,14 @@
#include "content/public/test/browser_test_utils.h" #include "content/public/test/browser_test_utils.h"
#include "content/public/test/content_browser_test.h" #include "content/public/test/content_browser_test.h"
#include "content/public/test/content_browser_test_utils.h" #include "content/public/test/content_browser_test_utils.h"
#include "content/public/test/hit_test_region_observer.h"
#include "content/public/test/test_navigation_observer.h" #include "content/public/test/test_navigation_observer.h"
#include "content/shell/browser/shell.h" #include "content/shell/browser/shell.h"
#include "net/dns/mock_host_resolver.h" #include "net/dns/mock_host_resolver.h"
#include "net/test/embedded_test_server/default_handlers.h" #include "net/test/embedded_test_server/default_handlers.h"
#include "net/test/embedded_test_server/embedded_test_server.h" #include "net/test/embedded_test_server/embedded_test_server.h"
#include "third_party/blink/public/common/input/web_mouse_event.h"
#include "ui/gfx/geometry/point.h"
#include "url/gurl.h" #include "url/gurl.h"
namespace content { namespace content {
...@@ -447,6 +450,45 @@ IN_PROC_BROWSER_TEST_F(ImpressionDeclarationBrowserTest, ...@@ -447,6 +450,45 @@ IN_PROC_BROWSER_TEST_F(ImpressionDeclarationBrowserTest,
EXPECT_EQ(1u, impression_observer.Wait().impression_data); EXPECT_EQ(1u, impression_observer.Wait().impression_data);
} }
// Tests that when a context menu is shown, there is an impression attached to
// the ContextMenu data forwarded to the browser process.
// TODO(johnidel): SimulateMouseClickAt() does not work on Android, find a
// different way to invoke the context menu that works on Android.
#if !defined(OS_ANDROID)
IN_PROC_BROWSER_TEST_F(ImpressionDeclarationBrowserTest,
ContextMenuShownForImpression_ImpressionSet) {
// Navigate to a page with the non-https server.
EXPECT_TRUE(NavigateToURL(
web_contents(),
https_server()->GetURL("b.test", "/page_with_impression_creator.html")));
EXPECT_TRUE(ExecJs(web_contents(), R"(
createImpressionTagAtLocation("link",
"page_with_conversion_redirect.html",
"10" /* impression data */,
"https://dest.com" /* conversion_destination */,
100 /* left */, 100 /* top */);)"));
content::RenderProcessHost* render_process_host =
web_contents()->GetMainFrame()->GetProcess();
auto context_menu_filter = base::MakeRefCounted<content::ContextMenuFilter>();
render_process_host->AddFilter(context_menu_filter.get());
content::SimulateMouseClickAt(web_contents(), 0,
blink::WebMouseEvent::Button::kRight,
gfx::Point(100, 100));
context_menu_filter->Wait();
content::UntrustworthyContextMenuParams params =
context_menu_filter->get_params();
EXPECT_TRUE(params.impression);
EXPECT_EQ(16UL, params.impression->impression_data);
EXPECT_EQ(url::Origin::Create(GURL("https://dest.com")),
params.impression->conversion_destination);
}
#endif // !defined(OS_ANDROID)
IN_PROC_BROWSER_TEST_F(ImpressionDeclarationBrowserTest, IN_PROC_BROWSER_TEST_F(ImpressionDeclarationBrowserTest,
ImpressionNavigationReloads_NoImpression) { ImpressionNavigationReloads_NoImpression) {
EXPECT_TRUE(NavigateToURL( EXPECT_TRUE(NavigateToURL(
......
...@@ -152,6 +152,7 @@ IPC_STRUCT_TRAITS_BEGIN(content::UntrustworthyContextMenuParams) ...@@ -152,6 +152,7 @@ IPC_STRUCT_TRAITS_BEGIN(content::UntrustworthyContextMenuParams)
IPC_STRUCT_TRAITS_MEMBER(y) IPC_STRUCT_TRAITS_MEMBER(y)
IPC_STRUCT_TRAITS_MEMBER(link_url) IPC_STRUCT_TRAITS_MEMBER(link_url)
IPC_STRUCT_TRAITS_MEMBER(link_text) IPC_STRUCT_TRAITS_MEMBER(link_text)
IPC_STRUCT_TRAITS_MEMBER(impression)
IPC_STRUCT_TRAITS_MEMBER(unfiltered_link_url) IPC_STRUCT_TRAITS_MEMBER(unfiltered_link_url)
IPC_STRUCT_TRAITS_MEMBER(src_url) IPC_STRUCT_TRAITS_MEMBER(src_url)
IPC_STRUCT_TRAITS_MEMBER(has_image_contents) IPC_STRUCT_TRAITS_MEMBER(has_image_contents)
......
...@@ -11,9 +11,11 @@ ...@@ -11,9 +11,11 @@
#include <string> #include <string>
#include <vector> #include <vector>
#include "base/optional.h"
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "content/common/content_export.h" #include "content/common/content_export.h"
#include "content/public/common/impression.h"
#include "content/public/common/menu_item.h" #include "content/public/common/menu_item.h"
#include "services/network/public/mojom/referrer_policy.mojom.h" #include "services/network/public/mojom/referrer_policy.mojom.h"
#include "third_party/blink/public/web/web_context_menu_data.h" #include "third_party/blink/public/web/web_context_menu_data.h"
...@@ -65,15 +67,19 @@ struct CONTENT_EXPORT UntrustworthyContextMenuParams { ...@@ -65,15 +67,19 @@ struct CONTENT_EXPORT UntrustworthyContextMenuParams {
// The text associated with the link. May be an empty string if the contents // The text associated with the link. May be an empty string if the contents
// of the link are an image. // of the link are an image.
// Will be empty if link_url is empty. // Will be empty if |link_url| is empty.
base::string16 link_text; base::string16 link_text;
// The impression declared by the link. May be base::nullopt even if
// |link_url| is non-empty.
base::Optional<Impression> impression;
// The link URL to be used ONLY for "copy link address". We don't validate // The link URL to be used ONLY for "copy link address". We don't validate
// this field in the frontend process. // this field in the frontend process.
GURL unfiltered_link_url; GURL unfiltered_link_url;
// This is the source URL for the element that the context menu was // This is the source URL for the element that the context menu was
// invoked on. Example of elements with source URLs are img, audio, and // invoked on. Example of elements with source URLs are img, audio, and
// video. // video.
GURL src_url; GURL src_url;
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "content/public/common/untrustworthy_context_menu_params.h" #include "content/public/common/untrustworthy_context_menu_params.h"
#include "content/public/renderer/content_renderer_client.h" #include "content/public/renderer/content_renderer_client.h"
#include "content/renderer/history_serialization.h" #include "content/renderer/history_serialization.h"
#include "content/renderer/impression_conversions.h"
#include "content/renderer/menu_item_builder.h" #include "content/renderer/menu_item_builder.h"
namespace content { namespace content {
...@@ -50,6 +51,10 @@ UntrustworthyContextMenuParams ContextMenuParamsBuilder::Build( ...@@ -50,6 +51,10 @@ UntrustworthyContextMenuParams ContextMenuParamsBuilder::Build(
params.custom_items.push_back(MenuItemBuilder::Build(data.custom_items[i])); params.custom_items.push_back(MenuItemBuilder::Build(data.custom_items[i]));
params.link_text = data.link_text.Utf16(); params.link_text = data.link_text.Utf16();
if (data.impression)
params.impression = ConvertWebImpressionToImpression(*data.impression);
params.source_type = static_cast<ui::MenuSourceType>(data.source_type); params.source_type = static_cast<ui::MenuSourceType>(data.source_type);
return params; return params;
......
...@@ -20,6 +20,14 @@ function createImpressionTag(id, url, data, destination) { ...@@ -20,6 +20,14 @@ function createImpressionTag(id, url, data, destination) {
createImpressionTagWithTarget(id, url, data, destination, "_top"); createImpressionTagWithTarget(id, url, data, destination, "_top");
} }
function createImpressionTagAtLocation(id, url, data, destination, left, top) {
let anchor =
createImpressionTagWithTarget(id, url, data, destination, "_top");
const style = "position: absolute; left: " + (left - 10) + "px; top: " +
(top - 10) + "px; width: 20px; height: 20px;";
anchor.setAttribute("style", style);
}
function createImpressionTagWithReportingAndExpiry( function createImpressionTagWithReportingAndExpiry(
id, url, data, destination, report_origin, expiry) { id, url, data, destination, report_origin, expiry) {
let anchor = createImpressionTagWithTarget( let anchor = createImpressionTagWithTarget(
......
...@@ -35,6 +35,7 @@ ...@@ -35,6 +35,7 @@
#include "third_party/blink/public/common/context_menu_data/input_field_type.h" #include "third_party/blink/public/common/context_menu_data/input_field_type.h"
#include "third_party/blink/public/common/context_menu_data/media_type.h" #include "third_party/blink/public/common/context_menu_data/media_type.h"
#include "third_party/blink/public/common/input/web_menu_source_type.h" #include "third_party/blink/public/common/input/web_menu_source_type.h"
#include "third_party/blink/public/platform/web_impression.h"
#include "third_party/blink/public/platform/web_rect.h" #include "third_party/blink/public/platform/web_rect.h"
#include "third_party/blink/public/platform/web_string.h" #include "third_party/blink/public/platform/web_string.h"
#include "third_party/blink/public/platform/web_url.h" #include "third_party/blink/public/platform/web_url.h"
...@@ -88,6 +89,10 @@ struct WebContextMenuData { ...@@ -88,6 +89,10 @@ struct WebContextMenuData {
// The text of the link that is in the context. // The text of the link that is in the context.
WebString link_text; WebString link_text;
// If the node is a link, the impression declared by the link's conversion
// measurement attributes.
base::Optional<WebImpression> impression;
// The raw text of the selection in context. // The raw text of the selection in context.
WebString selected_text; WebString selected_text;
......
...@@ -352,6 +352,10 @@ base::Optional<WebImpression> HTMLAnchorElement::GetImpressionForNavigation() ...@@ -352,6 +352,10 @@ base::Optional<WebImpression> HTMLAnchorElement::GetImpressionForNavigation()
const { const {
DCHECK(HasImpression()); DCHECK(HasImpression());
if (!RuntimeEnabledFeatures::ConversionMeasurementEnabled(
GetExecutionContext()))
return base::nullopt;
if (!GetExecutionContext()->IsFeatureEnabled( if (!GetExecutionContext()->IsFeatureEnabled(
mojom::blink::FeaturePolicyFeature::kConversionMeasurement)) { mojom::blink::FeaturePolicyFeature::kConversionMeasurement)) {
String message = String message =
...@@ -551,9 +555,7 @@ void HTMLAnchorElement::HandleClick(Event& event) { ...@@ -551,9 +555,7 @@ void HTMLAnchorElement::HandleClick(Event& event) {
} }
// Only attach impressions for main frame navigations. // Only attach impressions for main frame navigations.
if (RuntimeEnabledFeatures::ConversionMeasurementEnabled( if (target_frame && target_frame->IsMainFrame() && request.HasUserGesture() &&
GetExecutionContext()) &&
target_frame && target_frame->IsMainFrame() && request.HasUserGesture() &&
HasImpression()) { HasImpression()) {
base::Optional<WebImpression> impression = GetImpressionForNavigation(); base::Optional<WebImpression> impression = GetImpressionForNavigation();
if (impression) if (impression)
......
...@@ -454,6 +454,9 @@ bool ContextMenuController::ShowContextMenu(LocalFrame* frame, ...@@ -454,6 +454,9 @@ bool ContextMenuController::ShowContextMenu(LocalFrame* frame,
data.referrer_policy = network::mojom::ReferrerPolicy::kNever; data.referrer_policy = network::mojom::ReferrerPolicy::kNever;
data.link_text = anchor->innerText(); data.link_text = anchor->innerText();
if (anchor->HasImpression())
data.impression = anchor->GetImpressionForNavigation();
} }
data.input_field_type = ComputeInputFieldType(result); data.input_field_type = ComputeInputFieldType(result);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment