Commit 00c4cedd authored by Paul Dyson's avatar Paul Dyson Committed by Commit Bot

Add mechanism for logging App ids in UKM.

Allow a SourceId to be obtained and the url set to
app://[chrome|play]/id.

Add a new SourceType to govern the creation of urls
with scheme "app".

Bug: 841671
Change-Id: I1ed98722cdeb78ffd9944548901089e28f64181a
Reviewed-on: https://chromium-review.googlesource.com/1058736Reviewed-by: default avatarRobert Kaplow <rkaplow@chromium.org>
Reviewed-by: default avatarSteven Holte <holte@chromium.org>
Commit-Queue: Paul Dyson <pdyson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569962}
parent a25cecdf
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
static_library("content") { static_library("content") {
sources = [ sources = [
"app_source_url_recorder.cc",
"app_source_url_recorder.h",
"source_url_recorder.cc", "source_url_recorder.cc",
"source_url_recorder.h", "source_url_recorder.h",
] ]
...@@ -20,6 +22,7 @@ static_library("content") { ...@@ -20,6 +22,7 @@ static_library("content") {
source_set("unit_tests") { source_set("unit_tests") {
testonly = true testonly = true
sources = [ sources = [
"app_source_url_recorder_test.cc",
"source_url_recorder_test.cc", "source_url_recorder_test.cc",
] ]
deps = [ deps = [
......
// Copyright 2018 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 "components/ukm/content/app_source_url_recorder.h"
#include "base/atomic_sequence_num.h"
#include "services/metrics/public/cpp/delegating_ukm_recorder.h"
#include "services/metrics/public/cpp/ukm_recorder.h"
#include "services/metrics/public/cpp/ukm_source_id.h"
#include "url/gurl.h"
namespace ukm {
SourceId AssignNewAppId() {
static base::AtomicSequenceNumber seq;
return ConvertToSourceId(seq.GetNext() + 1, SourceIdType::APP_ID);
}
SourceId AppSourceUrlRecorder::GetSourceIdForApp(AppType type,
const std::string& id) {
GURL url;
if (type == AppType::kArc)
url = GURL("app://play/" + id);
else if (type == AppType::kChromeExtension)
url = GURL("chrome-extension://" + id);
else
return kInvalidSourceId;
return GetSourceIdForUrl(url);
}
SourceId AppSourceUrlRecorder::GetSourceIdForPWA(const GURL& url) {
return GetSourceIdForUrl(url);
}
SourceId AppSourceUrlRecorder::GetSourceIdForUrl(const GURL& url) {
ukm::DelegatingUkmRecorder* const recorder =
ukm::DelegatingUkmRecorder::Get();
if (!recorder)
return kInvalidSourceId;
const SourceId source_id = AssignNewAppId();
if (base::FeatureList::IsEnabled(kUkmAppLogging)) {
recorder->UpdateAppURL(source_id, url);
}
return source_id;
}
} // namespace ukm
// Copyright 2018 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.
#ifndef COMPONENTS_UKM_CONTENT_APP_SOURCE_URL_RECORDER_H_
#define COMPONENTS_UKM_CONTENT_APP_SOURCE_URL_RECORDER_H_
#include "services/metrics/public/cpp/ukm_source_id.h"
#include "base/feature_list.h"
#include <string>
class GURL;
namespace ukm {
const base::Feature kUkmAppLogging{"UkmAppLogging",
base::FEATURE_DISABLED_BY_DEFAULT};
enum class AppType { kArc, kChromeExtension };
class AppSourceUrlRecorder {
private:
friend class AppSourceUrlRecorderTest;
// Get a UKM SourceId for the app.
// Generates a url for the source depending upon AppType:
// kArc: app://play/id
// kChromeExtension: chrome-extension://id/
static SourceId GetSourceIdForApp(AppType type, const std::string& id);
// Get a UKM SourceId for a PWA.
static SourceId GetSourceIdForPWA(const GURL& url);
// For internal use only.
static SourceId GetSourceIdForUrl(const GURL& url);
};
} // namespace ukm
#endif // COMPONENTS_UKM_CONTENT_APP_SOURCE_URL_RECORDER_H_
// Copyright 2018 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 "components/ukm/content/app_source_url_recorder.h"
#include "base/test/scoped_feature_list.h"
#include "components/ukm/test_ukm_recorder.h"
#include "components/ukm/ukm_source.h"
#include "content/public/test/test_renderer_host.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
namespace ukm {
class AppSourceUrlRecorderTest : public content::RenderViewHostTestHarness {
public:
void SetUp() override {
scoped_feature_list_.InitAndEnableFeature(kUkmAppLogging);
content::RenderViewHostTestHarness::SetUp();
}
protected:
SourceId GetSourceIdForApp(AppType type, const std::string& id) {
return AppSourceUrlRecorder::GetSourceIdForApp(type, id);
}
SourceId GetSourceIdForPWA(const GURL& url) {
return AppSourceUrlRecorder::GetSourceIdForPWA(url);
}
base::test::ScopedFeatureList scoped_feature_list_;
TestAutoSetUkmRecorder test_ukm_recorder_;
};
TEST_F(AppSourceUrlRecorderTest, CheckPlay) {
SourceId id_play =
GetSourceIdForApp(AppType::kArc, "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
GURL expected_url_play("app://play/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
const auto& sources = test_ukm_recorder_.GetSources();
EXPECT_EQ(1ul, sources.size());
ASSERT_NE(kInvalidSourceId, id_play);
auto it = sources.find(id_play);
ASSERT_NE(sources.end(), it);
EXPECT_EQ(expected_url_play, it->second->url());
EXPECT_TRUE(it->second->initial_url().is_empty());
}
TEST_F(AppSourceUrlRecorderTest, CheckPWA) {
GURL url("https://pwa_example_url.com");
SourceId id = GetSourceIdForPWA(url);
const auto& sources = test_ukm_recorder_.GetSources();
EXPECT_EQ(1ul, sources.size());
ASSERT_NE(kInvalidSourceId, id);
auto it = sources.find(id);
ASSERT_NE(sources.end(), it);
EXPECT_EQ(url, it->second->url());
EXPECT_TRUE(it->second->initial_url().is_empty());
}
} // namespace ukm
...@@ -48,7 +48,8 @@ std::string GetWhitelistEntries() { ...@@ -48,7 +48,8 @@ std::string GetWhitelistEntries() {
} }
bool IsWhitelistedSourceId(SourceId source_id) { bool IsWhitelistedSourceId(SourceId source_id) {
return GetSourceIdType(source_id) == SourceIdType::NAVIGATION_ID; return GetSourceIdType(source_id) == SourceIdType::NAVIGATION_ID ||
GetSourceIdType(source_id) == SourceIdType::APP_ID;
} }
// Gets the maximum number of Sources we'll keep in memory before discarding any // Gets the maximum number of Sources we'll keep in memory before discarding any
...@@ -399,6 +400,14 @@ void UkmRecorderImpl::UpdateSourceURL(SourceId source_id, ...@@ -399,6 +400,14 @@ void UkmRecorderImpl::UpdateSourceURL(SourceId source_id,
sources_.emplace(source_id, std::make_unique<UkmSource>(source_id, url)); sources_.emplace(source_id, std::make_unique<UkmSource>(source_id, url));
} }
void UkmRecorderImpl::UpdateAppURL(SourceId source_id, const GURL& url) {
if (!extensions_enabled_) {
RecordDroppedSource(DroppedDataReason::EXTENSION_URLS_DISABLED);
return;
}
UpdateSourceURL(source_id, url);
}
void UkmRecorderImpl::AddEntry(mojom::UkmEntryPtr entry) { void UkmRecorderImpl::AddEntry(mojom::UkmEntryPtr entry) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
......
...@@ -70,6 +70,7 @@ class UkmRecorderImpl : public UkmRecorder { ...@@ -70,6 +70,7 @@ class UkmRecorderImpl : public UkmRecorder {
// UkmRecorder: // UkmRecorder:
void UpdateSourceURL(SourceId source_id, const GURL& url) override; void UpdateSourceURL(SourceId source_id, const GURL& url) override;
void UpdateAppURL(SourceId source_id, const GURL& url) override;
virtual bool ShouldRestrictToWhitelistedSourceIds() const; virtual bool ShouldRestrictToWhitelistedSourceIds() const;
......
...@@ -38,8 +38,10 @@ void DelegatingUkmRecorder::RemoveDelegate(UkmRecorder* delegate) { ...@@ -38,8 +38,10 @@ void DelegatingUkmRecorder::RemoveDelegate(UkmRecorder* delegate) {
void DelegatingUkmRecorder::UpdateSourceURL(SourceId source_id, void DelegatingUkmRecorder::UpdateSourceURL(SourceId source_id,
const GURL& url) { const GURL& url) {
if (GetSourceIdType(source_id) == SourceIdType::NAVIGATION_ID) { if (GetSourceIdType(source_id) == SourceIdType::NAVIGATION_ID ||
DLOG(FATAL) << "UpdateSourceURL invoked for NAVIGATION_ID SourceId"; GetSourceIdType(source_id) == SourceIdType::APP_ID) {
DLOG(FATAL)
<< "UpdateSourceURL invoked for NAVIGATION_ID or APP_ID SourceId";
return; return;
} }
UpdateSourceURLImpl(source_id, url); UpdateSourceURLImpl(source_id, url);
...@@ -54,6 +56,16 @@ void DelegatingUkmRecorder::UpdateNavigationURL(SourceId source_id, ...@@ -54,6 +56,16 @@ void DelegatingUkmRecorder::UpdateNavigationURL(SourceId source_id,
UpdateSourceURLImpl(source_id, url); UpdateSourceURLImpl(source_id, url);
} }
void DelegatingUkmRecorder::UpdateAppURL(SourceId source_id, const GURL& url) {
if (GetSourceIdType(source_id) != SourceIdType::APP_ID) {
DLOG(FATAL) << "UpdateAppURL invoked for non-APP_ID SourceId";
return;
}
base::AutoLock auto_lock(lock_);
for (auto& iterator : delegates_)
iterator.second.UpdateAppURL(source_id, url);
}
void DelegatingUkmRecorder::UpdateSourceURLImpl(SourceId source_id, void DelegatingUkmRecorder::UpdateSourceURLImpl(SourceId source_id,
const GURL& url) { const GURL& url) {
base::AutoLock auto_lock(lock_); base::AutoLock auto_lock(lock_);
...@@ -94,6 +106,16 @@ void DelegatingUkmRecorder::Delegate::UpdateSourceURL(ukm::SourceId source_id, ...@@ -94,6 +106,16 @@ void DelegatingUkmRecorder::Delegate::UpdateSourceURL(ukm::SourceId source_id,
base::BindOnce(&UkmRecorder::UpdateSourceURL, ptr_, source_id, url)); base::BindOnce(&UkmRecorder::UpdateSourceURL, ptr_, source_id, url));
} }
void DelegatingUkmRecorder::Delegate::UpdateAppURL(ukm::SourceId source_id,
const GURL& url) {
if (task_runner_->RunsTasksInCurrentSequence()) {
ptr_->UpdateAppURL(source_id, url);
return;
}
task_runner_->PostTask(FROM_HERE, base::BindOnce(&UkmRecorder::UpdateAppURL,
ptr_, source_id, url));
}
void DelegatingUkmRecorder::Delegate::AddEntry(mojom::UkmEntryPtr entry) { void DelegatingUkmRecorder::Delegate::AddEntry(mojom::UkmEntryPtr entry) {
if (task_runner_->RunsTasksInCurrentSequence()) { if (task_runner_->RunsTasksInCurrentSequence()) {
ptr_->AddEntry(std::move(entry)); ptr_->AddEntry(std::move(entry));
......
...@@ -43,11 +43,13 @@ class METRICS_EXPORT DelegatingUkmRecorder : public UkmRecorder { ...@@ -43,11 +43,13 @@ class METRICS_EXPORT DelegatingUkmRecorder : public UkmRecorder {
void RemoveDelegate(UkmRecorder* delegate); void RemoveDelegate(UkmRecorder* delegate);
private: private:
friend class AppSourceUrlRecorder;
friend class internal::SourceUrlRecorderWebContentsObserver; friend class internal::SourceUrlRecorderWebContentsObserver;
friend class internal::SourceUrlRecorderWebStateObserver; friend class internal::SourceUrlRecorderWebStateObserver;
// UkmRecorder: // UkmRecorder:
void UpdateSourceURL(SourceId source_id, const GURL& url) override; void UpdateSourceURL(SourceId source_id, const GURL& url) override;
void UpdateAppURL(SourceId source_id, const GURL& url) override;
void AddEntry(mojom::UkmEntryPtr entry) override; void AddEntry(mojom::UkmEntryPtr entry) override;
void UpdateSourceURLImpl(SourceId source_id, const GURL& url); void UpdateSourceURLImpl(SourceId source_id, const GURL& url);
...@@ -65,6 +67,7 @@ class METRICS_EXPORT DelegatingUkmRecorder : public UkmRecorder { ...@@ -65,6 +67,7 @@ class METRICS_EXPORT DelegatingUkmRecorder : public UkmRecorder {
~Delegate(); ~Delegate();
void UpdateSourceURL(SourceId source_id, const GURL& url); void UpdateSourceURL(SourceId source_id, const GURL& url);
void UpdateAppURL(SourceId source_id, const GURL& url);
void AddEntry(mojom::UkmEntryPtr entry); void AddEntry(mojom::UkmEntryPtr entry);
private: private:
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include <utility> #include <utility>
#include "base/logging.h"
#include "services/metrics/public/mojom/constants.mojom.h" #include "services/metrics/public/mojom/constants.mojom.h"
#include "services/service_manager/public/cpp/connector.h" #include "services/service_manager/public/cpp/connector.h"
...@@ -32,6 +33,10 @@ void MojoUkmRecorder::UpdateSourceURL(SourceId source_id, const GURL& url) { ...@@ -32,6 +33,10 @@ void MojoUkmRecorder::UpdateSourceURL(SourceId source_id, const GURL& url) {
interface_->UpdateSourceURL(source_id, url.spec()); interface_->UpdateSourceURL(source_id, url.spec());
} }
void MojoUkmRecorder::UpdateAppURL(SourceId source_id, const GURL& url) {
NOTREACHED();
}
void MojoUkmRecorder::AddEntry(mojom::UkmEntryPtr entry) { void MojoUkmRecorder::AddEntry(mojom::UkmEntryPtr entry) {
interface_->AddEntry(std::move(entry)); interface_->AddEntry(std::move(entry));
} }
......
...@@ -44,6 +44,7 @@ class METRICS_EXPORT MojoUkmRecorder : public UkmRecorder { ...@@ -44,6 +44,7 @@ class METRICS_EXPORT MojoUkmRecorder : public UkmRecorder {
private: private:
// UkmRecorder: // UkmRecorder:
void UpdateSourceURL(SourceId source_id, const GURL& url) override; void UpdateSourceURL(SourceId source_id, const GURL& url) override;
void UpdateAppURL(SourceId source_id, const GURL& url) override;
void AddEntry(mojom::UkmEntryPtr entry) override; void AddEntry(mojom::UkmEntryPtr entry) override;
mojom::UkmRecorderInterfacePtr interface_; mojom::UkmRecorderInterfacePtr interface_;
......
...@@ -127,6 +127,10 @@ class METRICS_EXPORT UkmRecorder { ...@@ -127,6 +127,10 @@ class METRICS_EXPORT UkmRecorder {
// maintain privacy constraints. // maintain privacy constraints.
virtual void UpdateSourceURL(SourceId source_id, const GURL& url) = 0; virtual void UpdateSourceURL(SourceId source_id, const GURL& url) = 0;
// Associates the SourceId with an app URL for APP_ID sources. This method
// should only be called by AppSourceUrlRecorder and DelegatingUkmRecorder.
virtual void UpdateAppURL(SourceId source_id, const GURL& url) = 0;
DISALLOW_COPY_AND_ASSIGN(UkmRecorder); DISALLOW_COPY_AND_ASSIGN(UkmRecorder);
}; };
......
...@@ -16,6 +16,7 @@ typedef int64_t SourceId; ...@@ -16,6 +16,7 @@ typedef int64_t SourceId;
enum class SourceIdType : int64_t { enum class SourceIdType : int64_t {
UKM = 0, UKM = 0,
NAVIGATION_ID = 1, NAVIGATION_ID = 1,
APP_ID = 2,
}; };
const SourceId kInvalidSourceId = 0; const SourceId kInvalidSourceId = 0;
......
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