Commit 2083e8bc authored by Steven Holte's avatar Steven Holte Committed by Commit Bot

Use shared UKM source id for JourneyLogger.

Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I33d2163e45e439d1ce109f17afeded9c11171381
Reviewed-on: https://chromium-review.googlesource.com/1136004Reviewed-by: default avatarMathieu Perreault <mathp@chromium.org>
Reviewed-by: default avatarMoe Ahmadi <mahmadi@chromium.org>
Commit-Queue: Steven Holte <holte@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575485}
parent 7b8b4f58
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
package org.chromium.chrome.browser.payments; package org.chromium.chrome.browser.payments;
import org.chromium.base.annotations.JNINamespace; import org.chromium.base.annotations.JNINamespace;
import org.chromium.content_public.browser.WebContents;
/** /**
* A class used to record journey metrics for the Payment Request feature. * A class used to record journey metrics for the Payment Request feature.
...@@ -19,10 +20,10 @@ public class JourneyLogger { ...@@ -19,10 +20,10 @@ public class JourneyLogger {
private boolean mWasPaymentRequestTriggered; private boolean mWasPaymentRequestTriggered;
private boolean mHasRecorded; private boolean mHasRecorded;
public JourneyLogger(boolean isIncognito, String url) { public JourneyLogger(boolean isIncognito, WebContents webContents) {
// Note that this pointer could leak the native object. The called must call destroy() to // Note that this pointer could leak the native object. The called must call destroy() to
// ensure that the native object is destroyed. // ensure that the native object is destroyed.
mJourneyLoggerAndroid = nativeInitJourneyLoggerAndroid(isIncognito, url); mJourneyLoggerAndroid = nativeInitJourneyLoggerAndroid(isIncognito, webContents);
} }
/** Will destroy the native object. This class shouldn't be used afterwards. */ /** Will destroy the native object. This class shouldn't be used afterwards. */
...@@ -176,7 +177,8 @@ public class JourneyLogger { ...@@ -176,7 +177,8 @@ public class JourneyLogger {
} }
} }
private native long nativeInitJourneyLoggerAndroid(boolean isIncognito, String url); private native long nativeInitJourneyLoggerAndroid(
boolean isIncognito, WebContents webContents);
private native void nativeDestroy(long nativeJourneyLoggerAndroid); private native void nativeDestroy(long nativeJourneyLoggerAndroid);
private native void nativeSetNumberOfSuggestionsShown(long nativeJourneyLoggerAndroid, private native void nativeSetNumberOfSuggestionsShown(long nativeJourneyLoggerAndroid,
int section, int number, boolean hasCompleteSuggestion); int section, int number, boolean hasCompleteSuggestion);
......
...@@ -404,7 +404,7 @@ public class PaymentRequestImpl ...@@ -404,7 +404,7 @@ public class PaymentRequestImpl
new AddressEditor(/*emailFieldIncluded=*/false, /*saveToDisk=*/!mIsIncognito); new AddressEditor(/*emailFieldIncluded=*/false, /*saveToDisk=*/!mIsIncognito);
mCardEditor = new CardEditor(mWebContents, mAddressEditor, sObserverForTest); mCardEditor = new CardEditor(mWebContents, mAddressEditor, sObserverForTest);
mJourneyLogger = new JourneyLogger(mIsIncognito, mWebContents.getLastCommittedUrl()); mJourneyLogger = new JourneyLogger(mIsIncognito, mWebContents);
if (sCanMakePaymentQueries == null) sCanMakePaymentQueries = new ArrayMap<>(); if (sCanMakePaymentQueries == null) sCanMakePaymentQueries = new ArrayMap<>();
......
...@@ -5,9 +5,9 @@ ...@@ -5,9 +5,9 @@
#include "chrome/browser/payments/android/journey_logger_android.h" #include "chrome/browser/payments/android/journey_logger_android.h"
#include "base/android/jni_string.h" #include "base/android/jni_string.h"
#include "components/ukm/content/source_url_recorder.h"
#include "content/public/browser/web_contents.h"
#include "jni/JourneyLogger_jni.h" #include "jni/JourneyLogger_jni.h"
#include "services/metrics/public/cpp/ukm_recorder.h"
#include "url/gurl.h"
namespace payments { namespace payments {
namespace { namespace {
...@@ -18,8 +18,8 @@ using ::base::android::ConvertJavaStringToUTF8; ...@@ -18,8 +18,8 @@ using ::base::android::ConvertJavaStringToUTF8;
} // namespace } // namespace
JourneyLoggerAndroid::JourneyLoggerAndroid(bool is_incognito, JourneyLoggerAndroid::JourneyLoggerAndroid(bool is_incognito,
const std::string& url) ukm::SourceId source_id)
: journey_logger_(is_incognito, GURL(url), ukm::UkmRecorder::Get()) {} : journey_logger_(is_incognito, source_id) {}
JourneyLoggerAndroid::~JourneyLoggerAndroid() {} JourneyLoggerAndroid::~JourneyLoggerAndroid() {}
...@@ -137,9 +137,11 @@ static jlong JNI_JourneyLogger_InitJourneyLoggerAndroid( ...@@ -137,9 +137,11 @@ static jlong JNI_JourneyLogger_InitJourneyLoggerAndroid(
JNIEnv* env, JNIEnv* env,
const JavaParamRef<jobject>& jcaller, const JavaParamRef<jobject>& jcaller,
jboolean jis_incognito, jboolean jis_incognito,
const base::android::JavaParamRef<jstring>& jurl) { const JavaParamRef<jobject>& jweb_contents) {
content::WebContents* web_contents =
content::WebContents::FromJavaWebContents(jweb_contents);
return reinterpret_cast<jlong>(new JourneyLoggerAndroid( return reinterpret_cast<jlong>(new JourneyLoggerAndroid(
jis_incognito, ConvertJavaStringToUTF8(env, jurl))); jis_incognito, ukm::GetSourceIdForWebContentsDocument(web_contents)));
} }
} // namespace payments } // namespace payments
...@@ -8,13 +8,14 @@ ...@@ -8,13 +8,14 @@
#include "base/android/scoped_java_ref.h" #include "base/android/scoped_java_ref.h"
#include "base/macros.h" #include "base/macros.h"
#include "components/payments/core/journey_logger.h" #include "components/payments/core/journey_logger.h"
#include "services/metrics/public/cpp/ukm_source_id.h"
namespace payments { namespace payments {
// Forwarding calls to payments::JourneyLogger. // Forwarding calls to payments::JourneyLogger.
class JourneyLoggerAndroid { class JourneyLoggerAndroid {
public: public:
JourneyLoggerAndroid(bool is_incognito, const std::string& url); JourneyLoggerAndroid(bool is_incognito, ukm::SourceId source_id);
~JourneyLoggerAndroid(); ~JourneyLoggerAndroid();
// Message from Java to destroy this object. // Message from Java to destroy this object.
......
...@@ -36,6 +36,7 @@ static_library("content") { ...@@ -36,6 +36,7 @@ static_library("content") {
"//components/payments/mojom", "//components/payments/mojom",
"//components/prefs", "//components/prefs",
"//components/strings:components_strings_grit", "//components/strings:components_strings_grit",
"//components/ukm/content",
"//components/url_formatter", "//components/url_formatter",
"//content/public/browser", "//content/public/browser",
"//third_party/blink/public:blink_headers", "//third_party/blink/public:blink_headers",
...@@ -119,6 +120,7 @@ source_set("unit_tests") { ...@@ -119,6 +120,7 @@ source_set("unit_tests") {
"//components/strings:components_strings_grit", "//components/strings:components_strings_grit",
"//components/webdata/common", "//components/webdata/common",
"//content/test:test_support", "//content/test:test_support",
"//services/metrics/public/cpp:metrics_cpp",
"//sql", "//sql",
"//testing/gtest", "//testing/gtest",
"//third_party/blink/public:blink_headers", "//third_party/blink/public:blink_headers",
......
...@@ -5,12 +5,14 @@ include_rules = [ ...@@ -5,12 +5,14 @@ include_rules = [
"+components/keyed_service/core", "+components/keyed_service/core",
"+components/prefs", "+components/prefs",
"+components/strings", "+components/strings",
"+components/ukm/content",
"+components/url_formatter", "+components/url_formatter",
"+components/webdata/common", "+components/webdata/common",
"+content/public", "+content/public",
"+mojo/public/cpp", "+mojo/public/cpp",
"+net", "+net",
"+services/data_decoder/public/cpp", "+services/data_decoder/public/cpp",
"+services/metrics/public/cpp",
"+sql", "+sql",
"+third_party/blink/public/common", "+third_party/blink/public/common",
"+third_party/blink/public/platform/modules/payments", "+third_party/blink/public/platform/modules/payments",
......
...@@ -21,6 +21,7 @@ ...@@ -21,6 +21,7 @@
#include "components/payments/core/payment_instrument.h" #include "components/payments/core/payment_instrument.h"
#include "components/payments/core/payment_prefs.h" #include "components/payments/core/payment_prefs.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "components/ukm/content/source_url_recorder.h"
#include "components/url_formatter/elide_url.h" #include "components/url_formatter/elide_url.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_frame_host.h"
...@@ -49,8 +50,7 @@ PaymentRequest::PaymentRequest( ...@@ -49,8 +50,7 @@ PaymentRequest::PaymentRequest(
render_frame_host->GetLastCommittedURL())), render_frame_host->GetLastCommittedURL())),
observer_for_testing_(observer_for_testing), observer_for_testing_(observer_for_testing),
journey_logger_(delegate_->IsIncognito(), journey_logger_(delegate_->IsIncognito(),
web_contents_->GetLastCommittedURL(), ukm::GetSourceIdForWebContentsDocument(web_contents)),
delegate_->GetUkmRecorder()),
weak_ptr_factory_(this) { weak_ptr_factory_(this) {
// OnConnectionTerminated will be called when the Mojo pipe is closed. This // OnConnectionTerminated will be called when the Mojo pipe is closed. This
// will happen as a result of many renderer-side events (both successful and // will happen as a result of many renderer-side events (both successful and
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include "components/payments/content/payment_request_spec.h" #include "components/payments/content/payment_request_spec.h"
#include "components/payments/content/test_content_payment_request_delegate.h" #include "components/payments/content/test_content_payment_request_delegate.h"
#include "components/payments/core/journey_logger.h" #include "components/payments/core/journey_logger.h"
#include "services/metrics/public/cpp/ukm_recorder.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/platform/modules/payments/payment_request.mojom.h" #include "third_party/blink/public/platform/modules/payments/payment_request.mojom.h"
...@@ -29,8 +30,7 @@ class PaymentRequestStateTest : public testing::Test, ...@@ -29,8 +30,7 @@ class PaymentRequestStateTest : public testing::Test,
: num_on_selected_information_changed_called_(0), : num_on_selected_information_changed_called_(0),
test_payment_request_delegate_(&test_personal_data_manager_), test_payment_request_delegate_(&test_personal_data_manager_),
journey_logger_(test_payment_request_delegate_.IsIncognito(), journey_logger_(test_payment_request_delegate_.IsIncognito(),
GURL("http://www.test.com"), ukm::UkmRecorder::GetNewSourceID()),
test_payment_request_delegate_.GetUkmRecorder()),
address_(autofill::test::GetFullProfile()), address_(autofill::test::GetFullProfile()),
credit_card_visa_(autofill::test::GetCreditCard()) { credit_card_visa_(autofill::test::GetCreditCard()) {
test_personal_data_manager_.SetAutofillCreditCardEnabled(true); test_personal_data_manager_.SetAutofillCreditCardEnabled(true);
......
...@@ -55,13 +55,10 @@ std::string GetHistogramNameSuffix( ...@@ -55,13 +55,10 @@ std::string GetHistogramNameSuffix(
} // namespace } // namespace
JourneyLogger::JourneyLogger(bool is_incognito, JourneyLogger::JourneyLogger(bool is_incognito, ukm::SourceId source_id)
const GURL& url,
ukm::UkmRecorder* ukm_recorder)
: is_incognito_(is_incognito), : is_incognito_(is_incognito),
events_(EVENT_INITIATED), events_(EVENT_INITIATED),
url_(url), source_id_(source_id) {}
ukm_recorder_(ukm_recorder) {}
JourneyLogger::~JourneyLogger() { JourneyLogger::~JourneyLogger() {
if (WasPaymentRequestTriggered()) if (WasPaymentRequestTriggered())
...@@ -241,16 +238,14 @@ void JourneyLogger::RecordEventsMetric(CompletionStatus completion_status) { ...@@ -241,16 +238,14 @@ void JourneyLogger::RecordEventsMetric(CompletionStatus completion_status) {
// Record the events in UMA. // Record the events in UMA.
base::UmaHistogramSparse("PaymentRequest.Events", events_); base::UmaHistogramSparse("PaymentRequest.Events", events_);
if (!ukm_recorder_ || !url_.is_valid()) if (source_id_ == ukm::kInvalidSourceId)
return; return;
// Record the events in UKM. // Record the events in UKM.
ukm::SourceId source_id = ukm_recorder_->GetNewSourceID(); ukm::builders::PaymentRequest_CheckoutEvents(source_id_)
ukm_recorder_->UpdateSourceURL(source_id, url_);
ukm::builders::PaymentRequest_CheckoutEvents(source_id)
.SetCompletionStatus(completion_status) .SetCompletionStatus(completion_status)
.SetEvents(events_) .SetEvents(events_)
.Record(ukm_recorder_); .Record(ukm::UkmRecorder::Get());
} }
bool JourneyLogger::WasPaymentRequestTriggered() { bool JourneyLogger::WasPaymentRequestTriggered() {
......
...@@ -8,11 +8,7 @@ ...@@ -8,11 +8,7 @@
#include <string> #include <string>
#include "base/macros.h" #include "base/macros.h"
#include "url/gurl.h" #include "services/metrics/public/cpp/ukm_source_id.h"
namespace ukm {
class UkmRecorder;
}
namespace payments { namespace payments {
...@@ -127,9 +123,7 @@ class JourneyLogger { ...@@ -127,9 +123,7 @@ class JourneyLogger {
NOT_SHOWN_REASON_MAX = 4, NOT_SHOWN_REASON_MAX = 4,
}; };
JourneyLogger(bool is_incognito, JourneyLogger(bool is_incognito, ukm::SourceId source_id);
const GURL& url,
ukm::UkmRecorder* ukm_recorder);
~JourneyLogger(); ~JourneyLogger();
// Increments the number of selection adds for the specified section. // Increments the number of selection adds for the specified section.
...@@ -232,10 +226,7 @@ class JourneyLogger { ...@@ -232,10 +226,7 @@ class JourneyLogger {
// Accumulates the many events that have happened during the Payment Request. // Accumulates the many events that have happened during the Payment Request.
int events_; int events_;
const GURL url_; ukm::SourceId source_id_;
// Not owned, will outlive this object.
ukm::UkmRecorder* ukm_recorder_;
DISALLOW_COPY_AND_ASSIGN(JourneyLogger); DISALLOW_COPY_AND_ASSIGN(JourneyLogger);
}; };
......
...@@ -44,6 +44,7 @@ source_set("payments") { ...@@ -44,6 +44,7 @@ source_set("payments") {
"//ios/chrome/browser", "//ios/chrome/browser",
"//ios/chrome/browser/autofill", "//ios/chrome/browser/autofill",
"//ios/chrome/browser/browser_state", "//ios/chrome/browser/browser_state",
"//ios/chrome/browser/metrics",
"//ios/chrome/browser/signin", "//ios/chrome/browser/signin",
"//ios/web", "//ios/web",
"//net", "//net",
......
...@@ -31,6 +31,7 @@ ...@@ -31,6 +31,7 @@
#include "ios/chrome/browser/autofill/address_normalizer_factory.h" #include "ios/chrome/browser/autofill/address_normalizer_factory.h"
#include "ios/chrome/browser/autofill/validation_rules_storage_factory.h" #include "ios/chrome/browser/autofill/validation_rules_storage_factory.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h" #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
#import "ios/chrome/browser/metrics/ukm_url_recorder.h"
#import "ios/chrome/browser/payments/ios_payment_instrument.h" #import "ios/chrome/browser/payments/ios_payment_instrument.h"
#import "ios/chrome/browser/payments/payment_request_util.h" #import "ios/chrome/browser/payments/payment_request_util.h"
#include "ios/chrome/browser/signin/signin_manager_factory.h" #include "ios/chrome/browser/signin/signin_manager_factory.h"
...@@ -96,7 +97,8 @@ PaymentRequest::PaymentRequest( ...@@ -96,7 +97,8 @@ PaymentRequest::PaymentRequest(
selected_payment_method_(nullptr), selected_payment_method_(nullptr),
selected_shipping_option_(nullptr), selected_shipping_option_(nullptr),
profile_comparator_(GetApplicationLocale(), *this), profile_comparator_(GetApplicationLocale(), *this),
journey_logger_(IsIncognito(), GetLastCommittedURL(), GetUkmRecorder()), journey_logger_(IsIncognito(),
ukm::GetSourceIdForWebStateDocument(web_state)),
payment_instruments_ready_(false), payment_instruments_ready_(false),
ios_instrument_finder_( ios_instrument_finder_(
GetApplicationContext()->GetSharedURLLoaderFactory(), GetApplicationContext()->GetSharedURLLoaderFactory(),
......
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