Skip to content

Commit

Permalink
Move definition of UKM source id out of //base
Browse files Browse the repository at this point in the history
There are currently two definitions of "UKM source id".
One is a feature class base::UkmSourceId (renaming to ukm::SourceIdObj)
that contains source type information.
The other is an alias for the underlying int64_t ukm::SourceId.
Clarify that ukm::SourceId should be used if only an int is required
to identify a source.

TBR=boliu@chromium.org,finnur@chromium.org,mmenke@chromium.org,seantopping@chromium.org,bmcquade@chromium.org,fdoray@chromium.org,nasko@chromium.org,sergeyu@chromium.org,dcheng@chromium.org,nator@chromium.org

Bug: 1046951
Change-Id: I8ee0f3f6c2093d8288aafe1d011a07887bfd4d99
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2472433
Commit-Queue: Yue Ru Sun <yrsun@chromium.org>
Reviewed-by: Sean Topping <seantopping@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Mugdha Lakhani <nator@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#820583}
  • Loading branch information
protostellarsun authored and Commit Bot committed Oct 24, 2020
1 parent acfe242 commit 03a6990
Show file tree
Hide file tree
Showing 86 changed files with 383 additions and 362 deletions.
2 changes: 2 additions & 0 deletions android_webview/browser/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ include_rules = [

"+printing",

"+services/metrics/public/cpp/ukm_source_id.h",

"+services/network/public",
# This is needed for the CookieManager to configure the CookieStore directly.
"+services/network/cookie_access_delegate_impl.h",
Expand Down
3 changes: 2 additions & 1 deletion android_webview/browser/aw_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@
#include "net/net_buildflags.h"
#include "net/ssl/ssl_cert_request_info.h"
#include "net/ssl/ssl_info.h"
#include "services/metrics/public/cpp/ukm_source_id.h"
#include "services/network/network_service.h"
#include "services/network/public/cpp/resource_request.h"
#include "services/network/public/mojom/cookie_manager.mojom-forward.h"
Expand Down Expand Up @@ -907,7 +908,7 @@ bool AwContentBrowserClient::WillCreateURLLoaderFactory(
URLLoaderFactoryType type,
const url::Origin& request_initiator,
base::Optional<int64_t> navigation_id,
base::UkmSourceId ukm_source_id,
ukm::SourceIdObj ukm_source_id,
mojo::PendingReceiver<network::mojom::URLLoaderFactory>* factory_receiver,
mojo::PendingRemote<network::mojom::TrustedURLLoaderHeaderClient>*
header_client,
Expand Down
3 changes: 2 additions & 1 deletion android_webview/browser/aw_content_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "services/metrics/public/cpp/ukm_source_id.h"
#include "storage/browser/quota/quota_settings.h"

namespace content {
Expand Down Expand Up @@ -197,7 +198,7 @@ class AwContentBrowserClient : public content::ContentBrowserClient {
URLLoaderFactoryType type,
const url::Origin& request_initiator,
base::Optional<int64_t> navigation_id,
base::UkmSourceId ukm_source_id,
ukm::SourceIdObj ukm_source_id,
mojo::PendingReceiver<network::mojom::URLLoaderFactory>* factory_receiver,
mojo::PendingRemote<network::mojom::TrustedURLLoaderHeaderClient>*
header_client,
Expand Down
2 changes: 0 additions & 2 deletions base/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -395,8 +395,6 @@ component("base") {
"metrics/sparse_histogram.h",
"metrics/statistics_recorder.cc",
"metrics/statistics_recorder.h",
"metrics/ukm_source_id.cc",
"metrics/ukm_source_id.h",
"metrics/user_metrics.cc",
"metrics/user_metrics.h",
"metrics/user_metrics_action.h",
Expand Down
65 changes: 0 additions & 65 deletions base/metrics/ukm_source_id.cc

This file was deleted.

106 changes: 0 additions & 106 deletions base/metrics/ukm_source_id.h

This file was deleted.

5 changes: 3 additions & 2 deletions chrome/browser/chrome_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@
#include "sandbox/policy/features.h"
#include "sandbox/policy/sandbox_type.h"
#include "sandbox/policy/switches.h"
#include "services/metrics/public/cpp/ukm_source_id.h"
#include "services/network/public/cpp/features.h"
#include "services/network/public/cpp/is_potentially_trustworthy.h"
#include "services/network/public/cpp/network_switches.h"
Expand Down Expand Up @@ -4454,7 +4455,7 @@ ChromeContentBrowserClient::CreateURLLoaderThrottles(

void ChromeContentBrowserClient::RegisterNonNetworkNavigationURLLoaderFactories(
int frame_tree_node_id,
base::UkmSourceId ukm_source_id,
ukm::SourceIdObj ukm_source_id,
NonNetworkURLLoaderFactoryMap* factories) {
#if BUILDFLAG(ENABLE_EXTENSIONS) || defined(OS_CHROMEOS)
content::WebContents* web_contents =
Expand Down Expand Up @@ -4715,7 +4716,7 @@ bool ChromeContentBrowserClient::WillCreateURLLoaderFactory(
URLLoaderFactoryType type,
const url::Origin& request_initiator,
base::Optional<int64_t> navigation_id,
base::UkmSourceId ukm_source_id,
ukm::SourceIdObj ukm_source_id,
mojo::PendingReceiver<network::mojom::URLLoaderFactory>* factory_receiver,
mojo::PendingRemote<network::mojom::TrustedURLLoaderHeaderClient>*
header_client,
Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/chrome_content_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "ppapi/buildflags/buildflags.h"
#include "services/metrics/public/cpp/ukm_source_id.h"
#include "services/network/public/mojom/network_context.mojom-forward.h"
#include "third_party/blink/public/common/loader/previews_state.h"

Expand Down Expand Up @@ -466,7 +467,7 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
int frame_tree_node_id) override;
void RegisterNonNetworkNavigationURLLoaderFactories(
int frame_tree_node_id,
base::UkmSourceId ukm_source_id,
ukm::SourceIdObj ukm_source_id,
NonNetworkURLLoaderFactoryMap* factories) override;
void RegisterNonNetworkWorkerMainResourceURLLoaderFactories(
content::BrowserContext* browser_context,
Expand All @@ -485,7 +486,7 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
URLLoaderFactoryType type,
const url::Origin& request_initiator,
base::Optional<int64_t> navigation_id,
base::UkmSourceId ukm_source_id,
ukm::SourceIdObj ukm_source_id,
mojo::PendingReceiver<network::mojom::URLLoaderFactory>* factory_receiver,
mojo::PendingRemote<network::mojom::TrustedURLLoaderHeaderClient>*
header_client,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@
#include "net/test/embedded_test_server/request_handler_util.h"
#include "net/test/test_data_directory.h"
#include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
#include "services/metrics/public/cpp/ukm_source_id.h"
#include "services/network/public/cpp/features.h"
#include "services/network/public/cpp/resource_request.h"
#include "services/network/public/cpp/simple_url_loader.h"
Expand Down Expand Up @@ -1812,7 +1813,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
frame->GetProcess()->GetBrowserContext(), frame,
frame->GetProcess()->GetID(),
content::ContentBrowserClient::URLLoaderFactoryType::kDocumentSubResource,
base::nullopt, base::kInvalidUkmSourceId, &pending_receiver, nullptr));
base::nullopt, ukm::kInvalidSourceIdObj, &pending_receiver, nullptr));
temp_web_contents.reset();
auto params = network::mojom::URLLoaderFactoryParams::New();
params->process_id = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ class ExtensionNavigationThrottleUnitTest
}

ukm::SourceId source_id = ukm::ConvertToSourceId(
navigation_id, base::UkmSourceId::Type::NAVIGATION_ID);
navigation_id, ukm::SourceIdObj::Type::NAVIGATION_ID);

ASSERT_EQ(1u, entries.size());
EXPECT_EQ(source_id, entries[0].source);
Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/extensions/extension_protocols_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
#include "extensions/test/test_extension_dir.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
#include "services/metrics/public/cpp/ukm_source_id.h"
#include "services/network/test/test_url_loader_client.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -202,7 +203,7 @@ class ExtensionProtocolsTestBase : public testing::Test {
: task_environment_(content::BrowserTaskEnvironment::IO_MAINLOOP),
rvh_test_enabler_(new content::RenderViewHostTestEnabler()),
force_incognito_(force_incognito),
test_ukm_id_(base::UkmSourceId::New()) {}
test_ukm_id_(ukm::SourceIdObj::New()) {}

void SetUp() override {
testing::Test::SetUp();
Expand Down Expand Up @@ -373,7 +374,7 @@ class ExtensionProtocolsTestBase : public testing::Test {
std::unique_ptr<TestingProfile> testing_profile_;
std::unique_ptr<content::WebContents> contents_;
const bool force_incognito_;
const base::UkmSourceId test_ukm_id_;
const ukm::SourceIdObj test_ukm_id_;

// |power_monitor_source_| is owned by the global PowerMonitor.
base::PowerMonitorTestSource* power_monitor_source_ = nullptr;
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/loader/url_loader_factory_proxy_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "extensions/browser/api/web_request/web_request_api.h"
#include "extensions/browser/browser_context_keyed_api_factory.h"
#include "mojo/public/cpp/bindings/self_owned_receiver.h"
#include "services/metrics/public/cpp/ukm_source_id.h"

// static
void UrlLoaderFactoryProxyImpl::Create(
Expand Down Expand Up @@ -43,7 +44,7 @@ void UrlLoaderFactoryProxyImpl::GetProxiedURLLoaderFactory(
browser_context, frame_host, process->GetID(),
content::ContentBrowserClient::URLLoaderFactoryType::kDocumentSubResource,
/*navigation_id=*/base::nullopt,
base::UkmSourceId::FromInt64(frame_host->GetPageUkmSourceId()),
ukm::SourceIdObj::FromInt64(frame_host->GetPageUkmSourceId()),
&proxied_factory,
/*header_client=*/nullptr);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#define CHROME_BROWSER_PAGE_LOAD_METRICS_OBSERVERS_CORE_UKM_PAGE_LOAD_METRICS_OBSERVER_H_

#include "base/macros.h"
#include "base/metrics/ukm_source_id.h"
#include "base/optional.h"
#include "base/time/time.h"
#include "components/page_load_metrics/browser/page_load_metrics_observer.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#define CHROME_BROWSER_PAGE_LOAD_METRICS_OBSERVERS_PORTAL_PAGE_LOAD_METRICS_OBSERVER_H_

#include "base/macros.h"
#include "base/metrics/ukm_source_id.h"
#include "base/optional.h"
#include "base/time/time.h"
#include "components/page_load_metrics/browser/page_load_metrics_observer.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

#include "base/macros.h"
#include "base/metrics/histogram_macros.h"
#include "base/metrics/ukm_source_id.h"
#include "base/time/time.h"
#include "services/metrics/public/cpp/ukm_builders.h"
#include "services/metrics/public/cpp/ukm_source_id.h"
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/profiles/profile_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "net/test/url_request/url_request_failed_job.h"
#include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
#include "services/metrics/public/cpp/ukm_source_id.h"
#include "services/network/public/cpp/features.h"
#include "services/network/public/cpp/simple_url_loader.h"
#include "testing/gmock/include/gmock/gmock.h"
Expand Down Expand Up @@ -628,7 +629,7 @@ IN_PROC_BROWSER_TEST_F(ProfileBrowserTest,
Profile* incognito_profile = incognito_browser->profile();
mojo::Remote<network::mojom::URLLoaderFactory> url_loader_factory;
url_loader_factory.Bind(extensions::CreateExtensionNavigationURLLoaderFactory(
incognito_profile, base::kInvalidUkmSourceId,
incognito_profile, ukm::kInvalidSourceIdObj,
false /* is_web_view_request */));

// Verify that the factory works fine while the profile is still alive.
Expand Down
Loading

0 comments on commit 03a6990

Please sign in to comment.