Commit bc5c7fc6 authored by Brandon Wylie's avatar Brandon Wylie Committed by Commit Bot

[EOC] Remove referrer in native reporting

Bug: 875087
Change-Id: I983ea386fa6ea80d46ea0ed732f620003c1c4099
Reviewed-on: https://chromium-review.googlesource.com/1185855Reviewed-by: default avatarFilip Gorski <fgorski@chromium.org>
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585692}
parent e8b1efa3
......@@ -27,7 +27,6 @@
#include "components/policy/core/common/policy_service.h"
#include "components/policy/policy_constants.h"
#include "components/ukm/content/source_url_recorder.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/web_contents.h"
#include "jni/ContextualSuggestionsBridge_jni.h"
#include "ui/gfx/android/java_bitmap.h"
......@@ -40,13 +39,6 @@ using base::android::JavaParamRef;
using base::android::ScopedJavaGlobalRef;
using base::android::ScopedJavaLocalRef;
namespace {
// Keep this in sync with CHROME_CONTEXTUAL_SUGGESTIONS_REFERRER. They should
// together.
const char kContextualSuggestionsReferrerURL[] =
"https://goto.google.com/explore-on-content-viewer";
} // namespace
namespace contextual_suggestions {
// A whitelisted method to inject synthetic field trials to Chrome Metrics.
......@@ -166,19 +158,8 @@ void ContextualSuggestionsBridge::ReportEvent(
static_cast<contextual_suggestions::ContextualSuggestionsEvent>(
j_event_id);
const content::NavigationEntry* navigation_entry =
web_contents->GetController().GetVisibleEntry();
const std::string& referrer_url =
navigation_entry == nullptr ? ""
: navigation_entry->GetReferrer().url.spec();
ArticleSource article_source = ArticleSource::OTHER;
if (referrer_url == kContextualSuggestionsReferrerURL) {
article_source = ArticleSource::CONTEXTUAL_SUGGESTIONS;
}
service_proxy_->ReportEvent(ukm_source_id,
web_contents->GetLastCommittedURL().spec(),
article_source, event);
service_proxy_->ReportEvent(
ukm_source_id, web_contents->GetLastCommittedURL().spec(), event);
}
void ContextualSuggestionsBridge::OnSuggestionsAvailable(
......
......@@ -46,8 +46,7 @@ void ContextualContentSuggestionsServiceProxy::FetchContextualSuggestions(
weak_ptr_factory_.GetWeakPtr(), std::move(callback)),
base::BindRepeating(
&ContextualContentSuggestionsServiceProxy::ReportEvent,
weak_ptr_factory_.GetWeakPtr(), last_ukm_source_id_, url.spec(),
ArticleSource::CONTEXTUAL_SUGGESTIONS));
weak_ptr_factory_.GetWeakPtr(), last_ukm_source_id_, url.spec()));
}
void ContextualContentSuggestionsServiceProxy::FetchContextualSuggestionImage(
......@@ -90,7 +89,6 @@ void ContextualContentSuggestionsServiceProxy::ClearState() {
void ContextualContentSuggestionsServiceProxy::ReportEvent(
ukm::SourceId ukm_source_id,
const std::string& url,
ArticleSource article_source,
ContextualSuggestionsEvent event) {
// TODO(pnoland): investigate how we can get into this state(one known
// example is if we switch tabs and there's no committed navigation in the new
......@@ -105,7 +103,7 @@ void ContextualContentSuggestionsServiceProxy::ReportEvent(
if (last_ukm_source_id_ != ukm::kInvalidSourceId)
reporter_->Flush();
last_ukm_source_id_ = ukm_source_id;
reporter_->SetupForPage(url, article_source, ukm_source_id);
reporter_->SetupForPage(url, ukm_source_id);
}
reporter_->RecordEvent(event);
......
......@@ -52,7 +52,6 @@ class ContextualContentSuggestionsServiceProxy {
// Reports user interface event to the service.
void ReportEvent(ukm::SourceId,
const std::string& url,
ArticleSource article_source,
ContextualSuggestionsEvent event);
// Ensures that all metrics are properly flushed.
......
......@@ -16,10 +16,9 @@ ContextualSuggestionsCompositeReporter::
void ContextualSuggestionsCompositeReporter::SetupForPage(
const std::string& url,
ArticleSource article_source,
ukm::SourceId source_id) {
for (ContextualSuggestionsReporter* reporter : raw_reporters_)
reporter->SetupForPage(url, article_source, source_id);
reporter->SetupForPage(url, source_id);
}
void ContextualSuggestionsCompositeReporter::RecordEvent(
......
......@@ -26,7 +26,6 @@ class ContextualSuggestionsCompositeReporter
// ContextualSuggestionsReporter
void SetupForPage(const std::string& url,
ArticleSource article_source,
ukm::SourceId source_id) override;
void RecordEvent(ContextualSuggestionsEvent event) override;
void Flush() override;
......
......@@ -20,10 +20,8 @@ class TestReporter : public ContextualSuggestionsReporter {
/* ContextualSuggestionsReporter */
void SetupForPage(const std::string& url,
ArticleSource article_source,
ukm::SourceId source_id) override {
this->url_ = url;
this->article_source_ = article_source;
this->source_id_ = source_id;
called_setup_for_page_count_++;
}
......@@ -49,7 +47,6 @@ class TestReporter : public ContextualSuggestionsReporter {
static int reporter_destroy_count_;
std::string url_;
ArticleSource article_source_;
ukm::SourceId source_id_;
int called_setup_for_page_count_ = 0;
int called_record_event_count_ = 0;
......@@ -69,8 +66,7 @@ TEST(ContextualSuggestionsCompositeReporterTest, AddAndReportUnique) {
composite_reporter->AddOwnedReporter(std::move(reporter));
}
composite_reporter->SetupForPage(
kTestUrl, ArticleSource::CONTEXTUAL_SUGGESTIONS, kSourceId);
composite_reporter->SetupForPage(kTestUrl, kSourceId);
composite_reporter->RecordEvent(ContextualSuggestionsEvent::FETCH_REQUESTED);
composite_reporter->Flush();
......@@ -95,8 +91,7 @@ TEST(ContextualSuggestionsCompositeReporterTest, AddAndReportRaw) {
reporters.push_back(std::move(reporter));
}
composite_reporter->SetupForPage(
kTestUrl, ArticleSource::CONTEXTUAL_SUGGESTIONS, kSourceId);
composite_reporter->SetupForPage(kTestUrl, kSourceId);
composite_reporter->RecordEvent(ContextualSuggestionsEvent::FETCH_REQUESTED);
composite_reporter->Flush();
......
......@@ -29,7 +29,6 @@ void ContextualSuggestionsDebuggingReporter::ClearEvents() {
void ContextualSuggestionsDebuggingReporter::SetupForPage(
const std::string& url,
ArticleSource article_source,
ukm::SourceId source_id) {
current_event_ = ContextualSuggestionsDebuggingEvent();
current_event_.url = url;
......
......@@ -55,7 +55,6 @@ class ContextualSuggestionsDebuggingReporter
// ContextualSuggestionsReporter
void SetupForPage(const std::string& url,
ArticleSource article_source,
ukm::SourceId source_id) override;
void RecordEvent(
contextual_suggestions::ContextualSuggestionsEvent event) override;
......
......@@ -27,7 +27,6 @@ ContextualSuggestionsMetricsReporter::~ContextualSuggestionsMetricsReporter() {
void ContextualSuggestionsMetricsReporter::SetupForPage(
const std::string& url,
ArticleSource article_source,
ukm::SourceId source_id) {
DCHECK(!ukm_entry_) << "Flush should be called before SetupForPage!";
DCHECK(source_id != ukm::kInvalidSourceId);
......
......@@ -28,7 +28,6 @@ class ContextualSuggestionsMetricsReporter
// ContextualSuggestionsReporter
void SetupForPage(const std::string& url,
ArticleSource article_source,
ukm::SourceId source_id) override;
void RecordEvent(ContextualSuggestionsEvent event) override;
void Flush() override;
......
......@@ -73,8 +73,7 @@ ukm::SourceId ContextualSuggestionsMetricsReporterTest::GetSourceId() {
TEST_F(ContextualSuggestionsMetricsReporterTest, BaseTest) {
base::HistogramTester histogram_tester;
GetReporter().SetupForPage(
kTestNavigationUrl, ArticleSource::CONTEXTUAL_SUGGESTIONS, GetSourceId());
GetReporter().SetupForPage(kTestNavigationUrl, GetSourceId());
GetReporter().RecordEvent(FETCH_REQUESTED);
GetReporter().RecordEvent(FETCH_COMPLETED);
GetReporter().RecordEvent(UI_PEEK_REVERSE_SCROLL);
......@@ -124,8 +123,7 @@ void ContextualSuggestionsMetricsReporterTest::ExpectMultipleEventsCountOnce(
ContextualSuggestionsEvent event) {
std::unique_ptr<base::HistogramTester> histogram_tester =
std::make_unique<base::HistogramTester>();
GetReporter().SetupForPage(
kTestNavigationUrl, ArticleSource::CONTEXTUAL_SUGGESTIONS, GetSourceId());
GetReporter().SetupForPage(kTestNavigationUrl, GetSourceId());
// Always report a single FETCH_DELAYED event so we ensure there's a
// histogram (otherwise the ExpectBucketCount may crash).
GetReporter().RecordEvent(FETCH_DELAYED);
......@@ -168,8 +166,7 @@ TEST_F(ContextualSuggestionsMetricsReporterTest, SuggestionClickedTest) {
TEST_F(ContextualSuggestionsMetricsReporterTest, MultipleEventsTest) {
std::unique_ptr<base::HistogramTester> histogram_tester =
std::make_unique<base::HistogramTester>();
GetReporter().SetupForPage(
kTestNavigationUrl, ArticleSource::CONTEXTUAL_SUGGESTIONS, GetSourceId());
GetReporter().SetupForPage(kTestNavigationUrl, GetSourceId());
// Test multiple cycles of FETCH_REQUESTED, FETCH_COMPLETED.
GetReporter().RecordEvent(FETCH_REQUESTED);
histogram_tester->ExpectBucketCount(kEventsHistogramName, kFetchRequested, 1);
......@@ -190,8 +187,7 @@ TEST_F(ContextualSuggestionsMetricsReporterTest, MultipleEventsTest) {
TEST_F(ContextualSuggestionsMetricsReporterTest, EnumNotReorderedTest) {
std::unique_ptr<base::HistogramTester> histogram_tester =
std::make_unique<base::HistogramTester>();
GetReporter().SetupForPage(
kTestNavigationUrl, ArticleSource::CONTEXTUAL_SUGGESTIONS, GetSourceId());
GetReporter().SetupForPage(kTestNavigationUrl, GetSourceId());
// Peek the UI, which starts the timer, expected by all the other UI or
// suggestion events.
GetReporter().RecordEvent(UI_PEEK_REVERSE_SCROLL);
......@@ -204,8 +200,7 @@ TEST_F(ContextualSuggestionsMetricsReporterTest, EnumNotReorderedTest) {
TEST_F(ContextualSuggestionsMetricsReporterTest, ButtonTest) {
base::HistogramTester histogram_tester;
GetReporter().SetupForPage(
kTestNavigationUrl, ArticleSource::CONTEXTUAL_SUGGESTIONS, GetSourceId());
GetReporter().SetupForPage(kTestNavigationUrl, GetSourceId());
GetReporter().RecordEvent(FETCH_REQUESTED);
GetReporter().RecordEvent(FETCH_COMPLETED);
GetReporter().RecordEvent(UI_BUTTON_SHOWN);
......
......@@ -13,11 +13,6 @@
namespace contextual_suggestions {
// This represents the source of the navigation to the current page. It's set
// via referrer links. We only need EOC/other since we only want to compare
// metrics across this dimension.
enum class ArticleSource { CONTEXTUAL_SUGGESTIONS, OTHER };
class ContextualSuggestionsDebuggingReporter;
class ContextualSuggestionsReporter;
......@@ -135,7 +130,6 @@ class ContextualSuggestionsReporter {
// Sets up the page with the given |source_id| for event reporting.
// All subsequent RecordEvent calls will apply to this page
virtual void SetupForPage(const std::string& url,
ArticleSource article_source,
ukm::SourceId source_id) = 0;
// Reports that an event occurred for the current page.
......
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