Commit e53980fb authored by John Delaney's avatar John Delaney Committed by Commit Bot

[conversions] Add OT config for ConversionMeasurement

What: Add an origin trial config for the Conversion Measurement
API. This separates the base::Feature kConversionMeasurement from the
runtime enabled feature. This allows the conversion storage layer to be
setup without enabling the features in blink.

The storage layer needs to be setup for all users regardless of the OT
so that reports for conversions can be sent when the user is no longer
on the site.

This also moves the ConversionMeasurement runtime feature to
experimental, and updates the about flags description to account for
this.

Change-Id: I534b6726b80146e90123b3116158d79872c00e55
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2264767
Commit-Queue: John Delaney <johnidel@chromium.org>
Reviewed-by: default avatarCharlie Harrison <csharrison@chromium.org>
Reviewed-by: default avatarNate Chapin <japhet@chromium.org>
Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#782857}
parent cf8e81f1
......@@ -77,7 +77,8 @@ const char kConditionalTabStripAndroidDescription[] =
const char kConversionMeasurementApiName[] = "Conversion Measurement API";
const char kConversionMeasurementApiDescription[] =
"Enables usage of the Conversion Measurement API.";
"Enables usage of the Conversion Measurement API. Requires "
"#enable-experimental-web-platform-features to be enabled.";
const char kConversionMeasurementDebugModeName[] =
"Conversion Measurement Debug Mode";
......
......@@ -11,6 +11,7 @@
#include "content/browser/conversions/conversion_manager_impl.h"
#include "content/browser/web_contents/web_contents_impl.h"
#include "content/public/common/content_features.h"
#include "content/public/common/content_switches.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/content_browser_test.h"
......@@ -76,11 +77,11 @@ class TestConversionHost : public ConversionHost {
base::RunLoop conversion_waiter_;
};
class ConversionRegistrationBrowserTest : public ContentBrowserTest {
class ConversionDisabledBrowserTest : public ContentBrowserTest {
public:
ConversionRegistrationBrowserTest() {
feature_list_.InitAndEnableFeature(features::kConversionMeasurement);
ConversionDisabledBrowserTest() {
ConversionManagerImpl::RunInMemoryForTesting();
feature_list_.InitAndEnableFeature(features::kConversionMeasurement);
}
void SetUpOnMainThread() override {
......@@ -104,11 +105,39 @@ class ConversionRegistrationBrowserTest : public ContentBrowserTest {
net::EmbeddedTestServer* https_server() { return https_server_.get(); }
private:
protected:
base::test::ScopedFeatureList feature_list_;
private:
std::unique_ptr<net::EmbeddedTestServer> https_server_;
};
IN_PROC_BROWSER_TEST_F(
ConversionDisabledBrowserTest,
ConversionRegisteredWithoutOTEnabled_NoConversionDataReceived) {
EXPECT_TRUE(NavigateToURL(
shell(),
embedded_test_server()->GetURL("/page_with_conversion_redirect.html")));
std::unique_ptr<TestConversionHost> host =
TestConversionHost::ReplaceAndGetConversionHost(web_contents());
EXPECT_TRUE(ExecJs(web_contents(), "registerConversion(123)"));
EXPECT_TRUE(NavigateToURL(shell(), GURL("about:blank")));
EXPECT_EQ(0u, host->num_conversions());
}
class ConversionRegistrationBrowserTest : public ConversionDisabledBrowserTest {
public:
ConversionRegistrationBrowserTest() = default;
void SetUpCommandLine(base::CommandLine* command_line) override {
// Sets up the blink runtime feature for ConversionMeasurement.
command_line->AppendSwitch(
switches::kEnableExperimentalWebPlatformFeatures);
}
};
// Test that full conversion path does not cause any failure when a conversion
// registration mojo is received.
IN_PROC_BROWSER_TEST_F(ConversionRegistrationBrowserTest,
......
......@@ -87,6 +87,10 @@ class ConversionsBrowserTest : public ContentBrowserTest {
void SetUpCommandLine(base::CommandLine* command_line) override {
command_line->AppendSwitch(switches::kConversionsDebugMode);
// Sets up the blink runtime feature for ConversionMeasurement.
command_line->AppendSwitch(
switches::kEnableExperimentalWebPlatformFeatures);
}
void SetUpOnMainThread() override {
......
// Copyright 2019 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 <vector>
#include "base/strings/strcat.h"
#include "base/test/bind_test_util.h"
#include "base/test/scoped_feature_list.h"
#include "content/browser/conversions/conversion_manager_impl.h"
#include "content/browser/conversions/storable_impression.h"
#include "content/browser/storage_partition_impl.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/storage_partition.h"
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_contents_observer.h"
#include "content/public/common/content_features.h"
#include "content/public/common/content_switches.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/content_browser_test.h"
#include "content/public/test/content_browser_test_utils.h"
#include "content/public/test/test_navigation_observer.h"
#include "content/public/test/url_loader_interceptor.h"
#include "content/shell/browser/shell.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
namespace content {
namespace {
constexpr char kBaseDataDir[] = "content/test/data/conversions/";
}
class ConversionsOriginTrialBrowserTest : public ContentBrowserTest {
public:
ConversionsOriginTrialBrowserTest() {
feature_list_.InitAndEnableFeature(features::kConversionMeasurement);
}
void SetUpOnMainThread() override {
ContentBrowserTest::SetUpOnMainThread();
// We use a URLLoaderInterceptor, rather than the EmbeddedTestServer, since
// the origin trial token in the response is associated with a fixed
// origin, whereas EmbeddedTestServer serves content on a random port.
url_loader_interceptor_ =
std::make_unique<URLLoaderInterceptor>(base::BindLambdaForTesting(
[&](URLLoaderInterceptor::RequestParams* params) -> bool {
URLLoaderInterceptor::WriteResponse(
base::StrCat(
{kBaseDataDir, params->url_request.url.path_piece()}),
params->client.get());
return true;
}));
}
void TearDownOnMainThread() override { url_loader_interceptor_.reset(); }
WebContents* web_contents() { return shell()->web_contents(); }
private:
base::test::ScopedFeatureList feature_list_;
std::unique_ptr<URLLoaderInterceptor> url_loader_interceptor_;
};
IN_PROC_BROWSER_TEST_F(ConversionsOriginTrialBrowserTest,
OriginTrialEnabled_ImpressionRegistered) {
EXPECT_TRUE(NavigateToURL(
shell(), GURL("https://example.test/impression_with_origin_trial.html")));
EXPECT_TRUE(ExecJs(shell(), R"(
createImpressionTag("link" /* id */,
"https://example.test/page_with_conversion_redirect.html" /* url */,
"1" /* impression data */,
"https://example.test/" /* conversion_destination */);)"));
TestNavigationObserver observer(web_contents());
EXPECT_TRUE(ExecJs(shell(), "simulateClick('link');"));
observer.Wait();
ConversionManagerImpl* conversion_manager =
static_cast<StoragePartitionImpl*>(
BrowserContext::GetDefaultStoragePartition(
web_contents()->GetBrowserContext()))
->GetConversionManager();
base::RunLoop run_loop;
// Verify we have received and logged an impression for the origin trial.
conversion_manager->GetActiveImpressionsForWebUI(base::BindLambdaForTesting(
[&](std::vector<StorableImpression> impressions) -> void {
EXPECT_EQ(1u, impressions.size());
run_loop.Quit();
}));
run_loop.Run();
}
// TODO(johnidel): Add tests that exercise the conversion side logic as well.
// This requires also using an embedded test server because the
// UrlLoadInterceptor cannot properly redirect the conversion pings.
} // namespace content
......@@ -14,6 +14,7 @@
#include "content/browser/web_contents/web_contents_impl.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/common/content_features.h"
#include "content/public/common/content_switches.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/content_browser_test.h"
......@@ -79,9 +80,9 @@ class ImpressionObserver : public TestNavigationObserver {
base::RunLoop impression_loop_;
};
class ImpressionDeclarationBrowserTest : public ContentBrowserTest {
class ImpressionDisabledBrowserTest : public ContentBrowserTest {
public:
ImpressionDeclarationBrowserTest() {
ImpressionDisabledBrowserTest() {
feature_list_.InitAndEnableFeature(features::kConversionMeasurement);
ConversionManagerImpl::RunInMemoryForTesting();
}
......@@ -114,6 +115,37 @@ class ImpressionDeclarationBrowserTest : public ContentBrowserTest {
std::unique_ptr<net::EmbeddedTestServer> https_server_;
};
// Verifies that impressions are not logged when the Runtime feature isn't
// enabled.
IN_PROC_BROWSER_TEST_F(ImpressionDisabledBrowserTest,
ImpressionWithoutFeatureEnabled_NotReceived) {
ImpressionObserver impression_observer(web_contents());
EXPECT_TRUE(NavigateToURL(
web_contents(),
https_server()->GetURL("b.test", "/page_with_impression_creator.html")));
EXPECT_TRUE(ExecJs(web_contents(), R"(
createImpressionTag("link",
"page_with_conversion_redirect.html",
"1" /* impression data */,
"https://a.com" /* conversion_destination */);)"));
EXPECT_TRUE(ExecJs(shell(), "simulateClick(\'link\');"));
// No impression should be observed.
EXPECT_TRUE(impression_observer.WaitForNavigationWithNoImpression());
}
class ImpressionDeclarationBrowserTest : public ImpressionDisabledBrowserTest {
public:
ImpressionDeclarationBrowserTest() = default;
// Sets up the blink runtime feature for ConversionMeasurement.
void SetUpCommandLine(base::CommandLine* command_line) override {
command_line->AppendSwitch(
switches::kEnableExperimentalWebPlatformFeatures);
}
};
IN_PROC_BROWSER_TEST_F(ImpressionDeclarationBrowserTest,
ImpressionTagClicked_ImpressionReceived) {
ImpressionObserver impression_observer(web_contents());
......
......@@ -353,8 +353,6 @@ void SetRuntimeFeaturesFromChromiumFeatures() {
blink::features::kBlockFlowHandlesWebkitLineClamp, kUseFeatureState},
{"BlockHTMLParserOnStyleSheets",
blink::features::kBlockHTMLParserOnStyleSheets, kUseFeatureState},
{"ConversionMeasurement", features::kConversionMeasurement,
kEnableOnly},
{"CSSColorSchemeUARendering", features::kCSSColorSchemeUARendering,
kUseFeatureState},
{"CSSReducedFontLoadingInvalidations",
......
......@@ -931,6 +931,7 @@ test("content_browsertests") {
"../browser/conversions/conversion_internals_browsertest.cc",
"../browser/conversions/conversion_registration_browsertest.cc",
"../browser/conversions/conversions_browsertest.cc",
"../browser/conversions/conversions_origin_trial_browsertest.cc",
"../browser/conversions/impression_declaration_browsertest.cc",
"../browser/cross_origin_opener_policy_browsertest.cc",
"../browser/cross_site_transfer_browsertest.cc",
......
<html>
<head>
<!-- TODO(johnidel): Find a better way to provide this toke, as this has an expiration in 2033.
Generate this token with the command:
generate_token.py https://example.test ConversionMeasurement -expire-timestamp=2000000000 -->
<meta http-equiv="origin-trial" content="AyiEpqRSCsaH3iE8GrZXh/DPVQPQr21JkMfKwjBFQKXHORXnLsT2AVEbhAECynhD0Um/tHnHHgN/mKJ8JWd1lAUAAABgeyJvcmlnaW4iOiAiaHR0cHM6Ly9leGFtcGxlLnRlc3Q6NDQzIiwgImZlYXR1cmUiOiAiQ29udmVyc2lvbk1lYXN1cmVtZW50IiwgImV4cGlyeSI6IDIwMDAwMDAwMDB9">
<script src="register_impression.js"></script>
</head>
</html>
......@@ -541,8 +541,9 @@ void HTMLAnchorElement::HandleClick(Event& event) {
}
// Only attach impressions for main frame navigations.
if (RuntimeEnabledFeatures::ConversionMeasurementEnabled() && target_frame &&
target_frame->IsMainFrame() && request.HasUserGesture() &&
if (RuntimeEnabledFeatures::ConversionMeasurementEnabled(
GetExecutionContext()) &&
target_frame && target_frame->IsMainFrame() && request.HasUserGesture() &&
HasImpression()) {
base::Optional<WebImpression> impression = GetImpressionForNavigation();
if (impression)
......
......@@ -1019,11 +1019,13 @@ bool FrameFetchContext::SendConversionRequestInsteadOfRedirecting(
const KURL& url,
const base::Optional<ResourceRequest::RedirectInfo>& redirect_info,
ReportingDisposition reporting_disposition) const {
if (!RuntimeEnabledFeatures::ConversionMeasurementEnabled())
if (GetResourceFetcherProperties().IsDetached())
return false;
if (GetResourceFetcherProperties().IsDetached())
if (!RuntimeEnabledFeatures::ConversionMeasurementEnabled(
document_->domWindow())) {
return false;
}
LocalFrame* frame = document_->GetFrame();
DCHECK(frame);
......
......@@ -346,7 +346,8 @@
},
{
name: "ConversionMeasurement",
status: "test",
origin_trial_feature_name: "ConversionMeasurement",
status: "experimental",
},
{
name: "CookieDeprecationMessages",
......
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