diff options
author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2020-03-11 11:32:04 +0100 |
---|---|---|
committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2020-03-18 13:40:17 +0000 |
commit | 31ccca0778db85c159634478b4ec7997f6704860 (patch) | |
tree | 3d33fc3afd9d5ec95541e1bbe074a9cf8da12a0e /chromium/components/ukm | |
parent | 248b70b82a40964d5594eb04feca0fa36716185d (diff) | |
download | qtwebengine-chromium-31ccca0778db85c159634478b4ec7997f6704860.tar.gz |
BASELINE: Update Chromium to 80.0.3987.136
Change-Id: I98e1649aafae85ba3a83e67af00bb27ef301db7b
Reviewed-by: Jüri Valdmann <juri.valdmann@qt.io>
Diffstat (limited to 'chromium/components/ukm')
26 files changed, 1273 insertions, 901 deletions
diff --git a/chromium/components/ukm/BUILD.gn b/chromium/components/ukm/BUILD.gn index a12c7c33869..9674b5e9afb 100644 --- a/chromium/components/ukm/BUILD.gn +++ b/chromium/components/ukm/BUILD.gn @@ -11,8 +11,8 @@ static_library("ukm") { sources = [ "app_source_url_recorder.cc", "app_source_url_recorder.h", - "unsent_log_store_metrics_impl.cc", - "unsent_log_store_metrics_impl.h", + "scheme_constants.cc", + "scheme_constants.h", "ukm_pref_names.cc", "ukm_pref_names.h", "ukm_recorder_impl.cc", @@ -23,6 +23,8 @@ static_library("ukm") { "ukm_rotation_scheduler.h", "ukm_service.cc", "ukm_service.h", + "unsent_log_store_metrics_impl.cc", + "unsent_log_store_metrics_impl.h", ] public_deps = [ @@ -47,8 +49,8 @@ static_library("observers") { sources = [ "observers/history_delete_observer.cc", "observers/history_delete_observer.h", - "observers/sync_disable_observer.cc", - "observers/sync_disable_observer.h", + "observers/ukm_consent_state_observer.cc", + "observers/ukm_consent_state_observer.h", ] deps = [ @@ -86,7 +88,7 @@ source_set("unit_tests") { testonly = true sources = [ "app_source_url_recorder_test.cc", - "observers/sync_disable_observer_unittest.cc", + "observers/ukm_consent_state_observer_unittest.cc", "ukm_recorder_impl_unittest.cc", "ukm_service_unittest.cc", ] @@ -102,7 +104,6 @@ source_set("unit_tests") { "//components/prefs:test_support", "//components/sync:test_support", "//components/sync_preferences:test_support", - "//components/unified_consent:test_support", "//components/variations", "//net:test_support", "//services/metrics/public/cpp:ukm_builders", diff --git a/chromium/components/ukm/content/source_url_recorder.cc b/chromium/components/ukm/content/source_url_recorder.cc index 7c5facc6b9c..6cbe282ef2b 100644 --- a/chromium/components/ukm/content/source_url_recorder.cc +++ b/chromium/components/ukm/content/source_url_recorder.cc @@ -11,8 +11,8 @@ #include "base/metrics/field_trial_params.h" #include "content/public/browser/navigation_handle.h" #include "content/public/browser/web_contents.h" -#include "content/public/browser/web_contents_binding_set.h" #include "content/public/browser/web_contents_observer.h" +#include "content/public/browser/web_contents_receiver_set.h" #include "content/public/browser/web_contents_user_data.h" #include "services/metrics/public/cpp/delegating_ukm_recorder.h" #include "services/metrics/public/cpp/ukm_builders.h" @@ -80,6 +80,7 @@ class SourceUrlRecorderWebContentsObserver // blink::mojom::UkmSourceIdFrameHost void SetDocumentSourceId(int64_t source_id) override; + void GetNavigationSourceId(GetNavigationSourceIdCallback callback) override; private: explicit SourceUrlRecorderWebContentsObserver( @@ -100,8 +101,8 @@ class SourceUrlRecorderWebContentsObserver const GURL& initial_url); // Receives document source IDs from the renderer. - content::WebContentsFrameBindingSet<blink::mojom::UkmSourceIdFrameHost> - bindings_; + content::WebContentsFrameReceiverSet<blink::mojom::UkmSourceIdFrameHost> + receivers_; // Map from navigation ID to the initial URL for that navigation. base::flat_map<int64_t, GURL> pending_navigations_; @@ -149,7 +150,7 @@ WEB_CONTENTS_USER_DATA_KEY_IMPL(SourceUrlRecorderWebContentsObserver) SourceUrlRecorderWebContentsObserver::SourceUrlRecorderWebContentsObserver( content::WebContents* web_contents) : content::WebContentsObserver(web_contents), - bindings_(web_contents, this), + receivers_(web_contents, this), last_committed_full_navigation_source_id_(ukm::kInvalidSourceId), last_committed_full_navigation_or_same_document_source_id_( ukm::kInvalidSourceId), @@ -302,10 +303,15 @@ ukm::SourceId SourceUrlRecorderWebContentsObserver:: return last_committed_full_navigation_or_same_document_source_id_; } +void SourceUrlRecorderWebContentsObserver::GetNavigationSourceId( + GetNavigationSourceIdCallback callback) { + std::move(callback).Run(last_committed_full_navigation_source_id_); +} + void SourceUrlRecorderWebContentsObserver::SetDocumentSourceId( int64_t source_id) { content::RenderFrameHost* main_frame = web_contents()->GetMainFrame(); - content::RenderFrameHost* current_frame = bindings_.GetCurrentTargetFrame(); + content::RenderFrameHost* current_frame = receivers_.GetCurrentTargetFrame(); bool is_main_frame = main_frame == current_frame; bool is_cross_origin_frame = is_main_frame ? false @@ -313,7 +319,7 @@ void SourceUrlRecorderWebContentsObserver::SetDocumentSourceId( current_frame->GetLastCommittedOrigin()); pending_document_created_events_.emplace_back( - source_id, !bindings_.GetCurrentTargetFrame()->GetParent(), + source_id, !receivers_.GetCurrentTargetFrame()->GetParent(), is_cross_origin_frame); MaybeFlushPendingEvents(); } diff --git a/chromium/components/ukm/debug/.eslintrc.js b/chromium/components/ukm/debug/.eslintrc.js deleted file mode 100644 index baeb4a6ce2e..00000000000 --- a/chromium/components/ukm/debug/.eslintrc.js +++ /dev/null @@ -1,11 +0,0 @@ -// 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. - -module.exports = { - 'env': {'browser': true, 'es6': true,}, - 'rules': { - 'no-var': 'error', - }, -}; - diff --git a/chromium/components/ukm/ios/BUILD.gn b/chromium/components/ukm/ios/BUILD.gn index a836ad661d2..820b704f822 100644 --- a/chromium/components/ukm/ios/BUILD.gn +++ b/chromium/components/ukm/ios/BUILD.gn @@ -4,6 +4,13 @@ assert(target_os == "ios") +group("ios") { + public_deps = [ + ":features", + ":ukm_url_recorder", + ] +} + source_set("features") { sources = [ "features.cc", @@ -13,3 +20,38 @@ source_set("features") { "//base", ] } + +source_set("ukm_url_recorder") { + configs += [ "//build/config/compiler:enable_arc" ] + sources = [ + "ukm_url_recorder.h", + "ukm_url_recorder.mm", + ] + public_deps = [ + "//services/metrics/public/cpp:metrics_cpp", + ] + deps = [ + "//base", + "//ios/web", + "//url", + ] +} + +source_set("unit_tests") { + configs += [ "//build/config/compiler:enable_arc" ] + testonly = true + sources = [ + "ukm_url_recorder_unittest.mm", + ] + deps = [ + ":ukm_url_recorder", + "//base", + "//base/test:test_support", + "//components/ukm:test_support", + "//ios/web/public", + "//ios/web/public/test", + "//net:test_support", + "//services/metrics/public/cpp:metrics_cpp", + "//testing/gtest", + ] +} diff --git a/chromium/components/ukm/ios/DEPS b/chromium/components/ukm/ios/DEPS new file mode 100644 index 00000000000..7a7f9a14514 --- /dev/null +++ b/chromium/components/ukm/ios/DEPS @@ -0,0 +1,4 @@ +include_rules = [ + "+ios/web/public", + "+net/test", +] diff --git a/chromium/components/ukm/ios/ukm_url_recorder.h b/chromium/components/ukm/ios/ukm_url_recorder.h new file mode 100644 index 00000000000..44f218614d8 --- /dev/null +++ b/chromium/components/ukm/ios/ukm_url_recorder.h @@ -0,0 +1,25 @@ +// 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_IOS_UKM_URL_RECORDER_H_ +#define COMPONENTS_UKM_IOS_UKM_URL_RECORDER_H_ + +#include "services/metrics/public/cpp/ukm_source_id.h" + +namespace web { +class WebState; +} // namespace web + +namespace ukm { + +// Initializes recording of UKM source URLs for the given WebState. +void InitializeSourceUrlRecorderForWebState(web::WebState* web_state); + +// Gets a UKM SourceId for the currently committed document of web state. +// Returns kInvalidSourceId if no commit has been observed. +SourceId GetSourceIdForWebStateDocument(web::WebState* web_state); + +} // namespace ukm + +#endif // COMPONENTS_UKM_IOS_UKM_URL_RECORDER_H_ diff --git a/chromium/components/ukm/ios/ukm_url_recorder.mm b/chromium/components/ukm/ios/ukm_url_recorder.mm new file mode 100644 index 00000000000..10527ec6735 --- /dev/null +++ b/chromium/components/ukm/ios/ukm_url_recorder.mm @@ -0,0 +1,169 @@ +// 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/ios/ukm_url_recorder.h" + +#include <utility> + +#include "base/containers/flat_map.h" +#include "base/macros.h" +#import "ios/web/public/navigation/navigation_context.h" +#import "ios/web/public/web_state.h" +#include "ios/web/public/web_state_observer.h" +#import "ios/web/public/web_state_user_data.h" +#include "services/metrics/public/cpp/delegating_ukm_recorder.h" +#include "services/metrics/public/cpp/ukm_source_id.h" +#include "url/gurl.h" + +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + +namespace ukm { + +namespace internal { + +// SourceUrlRecorderWebStateObserver is responsible for recording UKM source +// URLs for all main frame navigations in a given WebState. +// SourceUrlRecorderWebStateObserver records both the final URL for a +// navigation and, if the navigation was redirected, the initial URL as well. +class SourceUrlRecorderWebStateObserver + : public web::WebStateObserver, + public web::WebStateUserData<SourceUrlRecorderWebStateObserver> { + public: + ~SourceUrlRecorderWebStateObserver() override; + + // web::WebStateObserver + void DidStartNavigation(web::WebState* web_state, + web::NavigationContext* navigation_context) override; + void DidFinishNavigation(web::WebState* web_state, + web::NavigationContext* navigation_context) override; + void WebStateDestroyed(web::WebState* web_state) override; + + SourceId GetLastCommittedSourceId() const; + + private: + explicit SourceUrlRecorderWebStateObserver(web::WebState* web_state); + friend class web::WebStateUserData<SourceUrlRecorderWebStateObserver>; + + void MaybeRecordUrl(web::NavigationContext* navigation_context, + const GURL& initial_url); + + // Map from navigation ID to the initial URL for that navigation. + base::flat_map<int64_t, GURL> pending_navigations_; + + SourceId last_committed_source_id_; + + WEB_STATE_USER_DATA_KEY_DECL(); + + DISALLOW_COPY_AND_ASSIGN(SourceUrlRecorderWebStateObserver); +}; + +WEB_STATE_USER_DATA_KEY_IMPL(SourceUrlRecorderWebStateObserver) + +SourceUrlRecorderWebStateObserver::SourceUrlRecorderWebStateObserver( + web::WebState* web_state) + : last_committed_source_id_(kInvalidSourceId) { + web_state->AddObserver(this); +} + +SourceUrlRecorderWebStateObserver::~SourceUrlRecorderWebStateObserver() {} + +void SourceUrlRecorderWebStateObserver::WebStateDestroyed( + web::WebState* web_state) { + web_state->RemoveObserver(this); +} + +void SourceUrlRecorderWebStateObserver::DidStartNavigation( + web::WebState* web_state, + web::NavigationContext* navigation_context) { + // NavigationContexts only exist for main frame navigations, so this will + // never be called for non-main frame navigations and we don't need to filter + // non-main frame navigations out here. + + // Additionally, at least for the time being, we don't track metrics for + // same-document navigations (e.g. changes in URL fragment or URL changes + // due to history.pushState) in UKM. + if (navigation_context->IsSameDocument()) + return; + + // UKM doesn't want to record URLs for downloads. However, at the point a + // navigation is started, we don't yet know if the navigation will result in a + // download. Thus, we store the URL at the time a navigation was initiated + // and only record it later, once we verify that the navigation didn't result + // in a download. + pending_navigations_.insert(std::make_pair( + navigation_context->GetNavigationId(), navigation_context->GetUrl())); +} + +void SourceUrlRecorderWebStateObserver::DidFinishNavigation( + web::WebState* web_state, + web::NavigationContext* navigation_context) { + // NavigationContexts only exist for main frame navigations, so this will + // never be called for non-main frame navigations and we don't need to filter + // non-main frame navigations out here. + + auto it = pending_navigations_.find(navigation_context->GetNavigationId()); + if (it == pending_navigations_.end()) + return; + + DCHECK(!navigation_context->IsSameDocument()); + + if (navigation_context->HasCommitted()) { + last_committed_source_id_ = ConvertToSourceId( + navigation_context->GetNavigationId(), SourceIdType::NAVIGATION_ID); + } + + GURL initial_url = std::move(it->second); + pending_navigations_.erase(it); + + // UKM doesn't want to record URLs for navigations that result in downloads. + if (navigation_context->IsDownload()) + return; + + MaybeRecordUrl(navigation_context, initial_url); +} + +SourceId SourceUrlRecorderWebStateObserver::GetLastCommittedSourceId() const { + return last_committed_source_id_; +} + +void SourceUrlRecorderWebStateObserver::MaybeRecordUrl( + web::NavigationContext* navigation_context, + const GURL& initial_url) { + DCHECK(!navigation_context->IsSameDocument()); + + DelegatingUkmRecorder* ukm_recorder = DelegatingUkmRecorder::Get(); + if (!ukm_recorder) + return; + + const GURL& final_url = navigation_context->GetUrl(); + + UkmSource::NavigationData navigation_data; + // TODO(crbug.com/869123): This check isn't quite correct, as self redirecting + // is possible. This may also be changed to include the entire redirect chain. + if (final_url != initial_url) + navigation_data.urls = {initial_url}; + navigation_data.urls.push_back(final_url); + + // TODO(crbug.com/873316): Fill out the other fields in NavigationData. + + const ukm::SourceId source_id = ukm::ConvertToSourceId( + navigation_context->GetNavigationId(), ukm::SourceIdType::NAVIGATION_ID); + ukm_recorder->RecordNavigation(source_id, navigation_data); +} + +} // namespace internal + +void InitializeSourceUrlRecorderForWebState(web::WebState* web_state) { + internal::SourceUrlRecorderWebStateObserver::CreateForWebState(web_state); +} + +SourceId GetSourceIdForWebStateDocument(web::WebState* web_state) { + internal::SourceUrlRecorderWebStateObserver* obs = + internal::SourceUrlRecorderWebStateObserver::FromWebState(web_state); + return obs ? obs->GetLastCommittedSourceId() : kInvalidSourceId; +} + +} // namespace ukm diff --git a/chromium/components/ukm/ios/ukm_url_recorder_unittest.mm b/chromium/components/ukm/ios/ukm_url_recorder_unittest.mm new file mode 100644 index 00000000000..2d0d434f03c --- /dev/null +++ b/chromium/components/ukm/ios/ukm_url_recorder_unittest.mm @@ -0,0 +1,151 @@ +// 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/ios/ukm_url_recorder.h" + +#include "base/bind.h" +#include "base/optional.h" +#import "base/test/ios/wait_util.h" +#include "components/ukm/test_ukm_recorder.h" +#import "ios/web/public/navigation/navigation_manager.h" +#import "ios/web/public/test/web_test_with_web_state.h" +#import "ios/web/public/web_state.h" +#include "net/test/embedded_test_server/embedded_test_server.h" +#include "net/test/embedded_test_server/http_request.h" +#include "net/test/embedded_test_server/http_response.h" +#include "services/metrics/public/cpp/ukm_source.h" +#include "url/gurl.h" + +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + +namespace { + +std::unique_ptr<net::test_server::HttpResponse> HandleRequest( + const net::test_server::HttpRequest& request) { + if (request.GetURL().path() == "/title1.html") { + auto result = std::make_unique<net::test_server::BasicHttpResponse>(); + result->set_content_type("text/html"); + result->set_content("<html><head></head><body>NoTitle</body></html>"); + return std::move(result); + } + if (request.GetURL().path() == "/page_with_iframe.html") { + auto result = std::make_unique<net::test_server::BasicHttpResponse>(); + result->set_content_type("text/html"); + result->set_content( + "<html><head></head><body><iframe src=\"title1.html\"></body></html>"); + return std::move(result); + } + if (request.GetURL().path() == "/download") { + auto result = std::make_unique<net::test_server::BasicHttpResponse>(); + result->set_content_type("application/vnd.test"); + result->set_content("TestDownloadContent"); + return std::move(result); + } + if (request.GetURL().path() == "/redirect") { + auto result = std::make_unique<net::test_server::BasicHttpResponse>(); + result->set_code(net::HTTP_MOVED_PERMANENTLY); + result->AddCustomHeader("Location", "/title1.html"); + return std::move(result); + } + return nullptr; +} + +} // namespace + +class UkmUrlRecorderTest : public web::WebTestWithWebState { + protected: + UkmUrlRecorderTest() { + server_.RegisterDefaultHandler(base::BindRepeating(&HandleRequest)); + } + + void SetUp() override { + web::WebTestWithWebState::SetUp(); + ASSERT_TRUE(server_.Start()); + ukm::InitializeSourceUrlRecorderForWebState(web_state()); + } + + bool LoadUrlAndWait(const GURL& url) { + web::NavigationManager::WebLoadParams params(url); + web_state()->GetNavigationManager()->LoadURLWithParams(params); + return base::test::ios::WaitUntilConditionOrTimeout( + base::test::ios::kWaitForPageLoadTimeout, ^{ + return !web_state()->IsLoading(); + }); + } + + testing::AssertionResult RecordedUrl( + ukm::SourceId source_id, + GURL expected_url, + base::Optional<GURL> expected_initial_url) { + auto* source = test_ukm_recorder_.GetSourceForSourceId(source_id); + if (!source) + return testing::AssertionFailure() << "No URL recorded"; + if (source->url() != expected_url) + return testing::AssertionFailure() + << "Url was " << source->url() << ", expected: " << expected_url; + base::Optional<GURL> initial_url; + if (source->urls().size() > 1u) + initial_url = source->urls().front(); + if (expected_initial_url != initial_url) { + return testing::AssertionFailure() + << "Initial Url was " << initial_url.value_or(GURL()) + << ", expected: " << expected_initial_url.value_or(GURL()); + } + return testing::AssertionSuccess(); + } + + testing::AssertionResult DidNotRecordUrl(GURL url) { + const auto& sources = test_ukm_recorder_.GetSources(); + for (const auto& kv : sources) { + if (kv.second->url() == url) + return testing::AssertionFailure() + << "Url " << url << " was recorded with SourceId: " << kv.first; + if (kv.second->url() == url) + return testing::AssertionFailure() + << "Url " << url + << " was recorded as an initial URL with SourceId: " << kv.first; + } + return testing::AssertionSuccess(); + } + + protected: + net::EmbeddedTestServer server_; + ukm::TestAutoSetUkmRecorder test_ukm_recorder_; +}; + +// Tests that URLs get recorded for pages visited. +TEST_F(UkmUrlRecorderTest, Basic) { + GURL url = server_.GetURL("/title1.html"); + EXPECT_TRUE(LoadUrlAndWait(url)); + ukm::SourceId source_id = ukm::GetSourceIdForWebStateDocument(web_state()); + EXPECT_TRUE(RecordedUrl(source_id, url, base::nullopt)); +} + +// Tests that subframe URLs do not get recorded. +TEST_F(UkmUrlRecorderTest, IgnoreUrlInSubframe) { + GURL main_url = server_.GetURL("/page_with_iframe.html"); + GURL subframe_url = server_.GetURL("/title1.html"); + EXPECT_TRUE(LoadUrlAndWait(main_url)); + ukm::SourceId source_id = ukm::GetSourceIdForWebStateDocument(web_state()); + EXPECT_TRUE(RecordedUrl(source_id, main_url, base::nullopt)); + EXPECT_TRUE(DidNotRecordUrl(subframe_url)); +} + +// Tests that download URLs do not get recorded. +TEST_F(UkmUrlRecorderTest, IgnoreDownloadUrl) { + GURL url = server_.GetURL("/download"); + EXPECT_TRUE(LoadUrlAndWait(url)); + EXPECT_TRUE(DidNotRecordUrl(url)); +} + +// Tests that redirected URLs record initial and final URL. +TEST_F(UkmUrlRecorderTest, InitialUrl) { + GURL redirect_url = server_.GetURL("/redirect"); + GURL target_url = server_.GetURL("/title1.html"); + EXPECT_TRUE(LoadUrlAndWait(redirect_url)); + ukm::SourceId source_id = ukm::GetSourceIdForWebStateDocument(web_state()); + EXPECT_TRUE(RecordedUrl(source_id, target_url, redirect_url)); +} diff --git a/chromium/components/ukm/observers/sync_disable_observer.cc b/chromium/components/ukm/observers/sync_disable_observer.cc deleted file mode 100644 index f8e428ac146..00000000000 --- a/chromium/components/ukm/observers/sync_disable_observer.cc +++ /dev/null @@ -1,272 +0,0 @@ -// Copyright 2017 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/observers/sync_disable_observer.h" - -#include <memory> -#include <utility> - -#include "base/feature_list.h" -#include "base/metrics/histogram_macros.h" -#include "base/stl_util.h" -#include "components/sync/driver/sync_user_settings.h" -#include "components/unified_consent/feature.h" -#include "components/unified_consent/url_keyed_data_collection_consent_helper.h" -#include "google_apis/gaia/google_service_auth_error.h" - -using unified_consent::UrlKeyedDataCollectionConsentHelper; - -namespace ukm { - -namespace { - -enum DisableInfo { - DISABLED_BY_NONE, - DISABLED_BY_HISTORY, - DISABLED_BY_INITIALIZED, - DISABLED_BY_HISTORY_INITIALIZED, - DISABLED_BY_CONNECTED, - DISABLED_BY_HISTORY_CONNECTED, - DISABLED_BY_INITIALIZED_CONNECTED, - DISABLED_BY_HISTORY_INITIALIZED_CONNECTED, - DISABLED_BY_PASSPHRASE, - DISABLED_BY_HISTORY_PASSPHRASE, - DISABLED_BY_INITIALIZED_PASSPHRASE, - DISABLED_BY_HISTORY_INITIALIZED_PASSPHRASE, - DISABLED_BY_CONNECTED_PASSPHRASE, - DISABLED_BY_HISTORY_CONNECTED_PASSPHRASE, - DISABLED_BY_INITIALIZED_CONNECTED_PASSPHRASE, - DISABLED_BY_HISTORY_INITIALIZED_CONNECTED_PASSPHRASE, - DISABLED_BY_ANONYMIZED_DATA_COLLECTION, - MAX_DISABLE_INFO -}; - -void RecordDisableInfo(DisableInfo info) { - UMA_HISTOGRAM_ENUMERATION("UKM.SyncDisable.Info", info, MAX_DISABLE_INFO); -} - -} // namespace - -SyncDisableObserver::SyncDisableObserver() : sync_observer_(this) {} - -SyncDisableObserver::~SyncDisableObserver() { - for (const auto& entry : consent_helpers_) { - entry.second->RemoveObserver(this); - } -} - -bool SyncDisableObserver::SyncState::AllowsUkm() const { - if (anonymized_data_collection_state == DataCollectionState::kIgnored) - return history_enabled && initialized && connected && !passphrase_protected; - else - return anonymized_data_collection_state == DataCollectionState::kEnabled; -} - -bool SyncDisableObserver::SyncState::AllowsUkmWithExtension() const { - return AllowsUkm() && extensions_enabled && initialized && connected && - !passphrase_protected; -} - -// static -SyncDisableObserver::SyncState SyncDisableObserver::GetSyncState( - syncer::SyncService* sync_service, - UrlKeyedDataCollectionConsentHelper* consent_helper) { - SyncState state; - - // For the following two settings, we want them to match the state of a user - // having history/extensions enabled in their Sync settings. Using it to track - // active connections here is undesirable as changes in these states trigger - // data purges. - state.history_enabled = sync_service->GetPreferredDataTypes().Has( - syncer::HISTORY_DELETE_DIRECTIVES) && - sync_service->IsSyncFeatureEnabled(); - state.extensions_enabled = - sync_service->GetPreferredDataTypes().Has(syncer::EXTENSIONS) && - sync_service->IsSyncFeatureEnabled(); - - state.initialized = sync_service->IsEngineInitialized(); - - // Reasoning for the individual checks: - // - IsSyncFeatureActive() makes sure Sync is enabled and initialized. - // - HasCompletedSyncCycle() makes sure Sync has actually talked to the - // server. Without this, it's possible that there is some auth issue that we - // just haven't detected yet. - // - Finally, GetAuthError() makes sure Sync is not in an auth error state. In - // particular, this includes the "Sync paused" state (which is implemented - // as an auth error). - state.connected = - (sync_service->IsSyncFeatureActive() && - sync_service->HasCompletedSyncCycle() && - sync_service->GetAuthError() == GoogleServiceAuthError::AuthErrorNone()); - - state.passphrase_protected = - state.initialized && - sync_service->GetUserSettings()->IsUsingSecondaryPassphrase(); - if (consent_helper) { - state.anonymized_data_collection_state = - consent_helper->IsEnabled() ? DataCollectionState::kEnabled - : DataCollectionState::kDisabled; - } - return state; -} - -void SyncDisableObserver::ObserveServiceForSyncDisables( - syncer::SyncService* sync_service, - PrefService* prefs) { - std::unique_ptr<UrlKeyedDataCollectionConsentHelper> consent_helper; - if (unified_consent::IsUnifiedConsentFeatureEnabled()) { - consent_helper = UrlKeyedDataCollectionConsentHelper:: - NewAnonymizedDataCollectionConsentHelper(prefs, sync_service); - } - - SyncState state = GetSyncState(sync_service, consent_helper.get()); - previous_states_[sync_service] = state; - - if (consent_helper) { - consent_helper->AddObserver(this); - consent_helpers_[sync_service] = std::move(consent_helper); - } - sync_observer_.Add(sync_service); - UpdateAllProfileEnabled(false); -} - -void SyncDisableObserver::UpdateAllProfileEnabled(bool must_purge) { - bool all_sync_states_allow_ukm = CheckSyncStateOnAllProfiles(); - bool all_sync_states_allow_extension_ukm = - all_sync_states_allow_ukm && CheckSyncStateForExtensionsOnAllProfiles(); - // Any change in sync settings needs to call OnSyncPrefsChanged so that the - // new settings take effect. - if (must_purge || (all_sync_states_allow_ukm != all_sync_states_allow_ukm_) || - (all_sync_states_allow_extension_ukm != - all_sync_states_allow_extension_ukm_)) { - all_sync_states_allow_ukm_ = all_sync_states_allow_ukm; - all_sync_states_allow_extension_ukm_ = all_sync_states_allow_extension_ukm; - OnSyncPrefsChanged(must_purge); - } -} - -bool SyncDisableObserver::CheckSyncStateOnAllProfiles() { - if (previous_states_.empty()) - return false; - for (const auto& kv : previous_states_) { - const SyncDisableObserver::SyncState& state = kv.second; - - if (state.AllowsUkm()) - continue; - - // Used as output for the disable reason UMA metrics. - int disabled_by = 0; - if (state.anonymized_data_collection_state == - DataCollectionState::kIgnored) { - if (!state.history_enabled) - disabled_by |= 1 << 0; - if (!state.initialized) - disabled_by |= 1 << 1; - if (!state.connected) - disabled_by |= 1 << 2; - if (state.passphrase_protected) - disabled_by |= 1 << 3; - } else { - DCHECK_EQ(DataCollectionState::kDisabled, - state.anonymized_data_collection_state); - disabled_by |= 1 << 4; - } - RecordDisableInfo(DisableInfo(disabled_by)); - return false; - } - RecordDisableInfo(DISABLED_BY_NONE); - return true; -} - -bool SyncDisableObserver::CheckSyncStateForExtensionsOnAllProfiles() { - if (previous_states_.empty()) - return false; - for (const auto& kv : previous_states_) { - const SyncDisableObserver::SyncState& state = kv.second; - if (!state.extensions_enabled) - return false; - } - return true; -} - -void SyncDisableObserver::OnStateChanged(syncer::SyncService* sync) { - UrlKeyedDataCollectionConsentHelper* consent_helper = nullptr; - auto found = consent_helpers_.find(sync); - if (found != consent_helpers_.end()) - consent_helper = found->second.get(); - UpdateSyncState(sync, consent_helper); -} - -void SyncDisableObserver::OnUrlKeyedDataCollectionConsentStateChanged( - unified_consent::UrlKeyedDataCollectionConsentHelper* consent_helper) { - DCHECK(consent_helper); - syncer::SyncService* sync_service = nullptr; - for (const auto& entry : consent_helpers_) { - if (consent_helper == entry.second.get()) { - sync_service = entry.first; - break; - } - } - DCHECK(sync_service); - UpdateSyncState(sync_service, consent_helper); -} - -void SyncDisableObserver::UpdateSyncState( - syncer::SyncService* sync, - UrlKeyedDataCollectionConsentHelper* consent_helper) { - DCHECK(base::Contains(previous_states_, sync)); - const SyncDisableObserver::SyncState& previous_state = previous_states_[sync]; - DCHECK(previous_state.anonymized_data_collection_state == - DataCollectionState::kIgnored || - consent_helper); - SyncDisableObserver::SyncState state = GetSyncState(sync, consent_helper); - - // Trigger a purge if the current state no longer allows UKM, ignoring - // connection issues. - bool must_purge; - - if (state.anonymized_data_collection_state != DataCollectionState::kIgnored) { - // Verify that previous_state also uses unified consent. - DCHECK_NE(DataCollectionState::kIgnored, - previous_state.anonymized_data_collection_state); - must_purge = previous_state.AllowsUkm() && !state.AllowsUkm(); - } else { - // This differs from AllowsUkm() as it intentionally ignores connected - // status. - bool previous_state_allowed_ukm = previous_state.history_enabled && - previous_state.initialized && - !previous_state.passphrase_protected; - bool current_state_allows_ukm = state.history_enabled && - state.initialized && - !state.passphrase_protected; - must_purge = previous_state_allowed_ukm && !current_state_allows_ukm; - } - - UMA_HISTOGRAM_BOOLEAN("UKM.SyncDisable.Purge", must_purge); - - previous_states_[sync] = state; - UpdateAllProfileEnabled(must_purge); -} - -void SyncDisableObserver::OnSyncShutdown(syncer::SyncService* sync) { - DCHECK(base::Contains(previous_states_, sync)); - auto found = consent_helpers_.find(sync); - if (found != consent_helpers_.end()) { - found->second->RemoveObserver(this); - consent_helpers_.erase(found); - } - sync_observer_.Remove(sync); - previous_states_.erase(sync); - UpdateAllProfileEnabled(false); -} - -bool SyncDisableObserver::SyncStateAllowsUkm() { - return all_sync_states_allow_ukm_; -} - -bool SyncDisableObserver::SyncStateAllowsExtensionUkm() { - return all_sync_states_allow_extension_ukm_; -} - -} // namespace ukm diff --git a/chromium/components/ukm/observers/sync_disable_observer.h b/chromium/components/ukm/observers/sync_disable_observer.h deleted file mode 100644 index e70f25ae692..00000000000 --- a/chromium/components/ukm/observers/sync_disable_observer.h +++ /dev/null @@ -1,160 +0,0 @@ -// Copyright 2017 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_OBSERVERS_SYNC_DISABLE_OBSERVER_H_ -#define COMPONENTS_UKM_OBSERVERS_SYNC_DISABLE_OBSERVER_H_ - -#include <map> - -#include "base/scoped_observer.h" -#include "components/sync/driver/sync_service.h" -#include "components/sync/driver/sync_service_observer.h" -#include "components/unified_consent/url_keyed_data_collection_consent_helper.h" - -class PrefService; - -namespace ukm { - -// Observer that monitors whether UKM is allowed for all profiles. -// -// For one profile, UKM is allowed under the following conditions: -// * If unified consent is disabled, then UKM is allowed for the profile iff -// sync history is active; -// * If unified consent is enabled, then UKM is allowed for the profile iff -// URL-keyed anonymized data collectiion is enabled. -class SyncDisableObserver - : public syncer::SyncServiceObserver, - public unified_consent::UrlKeyedDataCollectionConsentHelper::Observer { - public: - SyncDisableObserver(); - ~SyncDisableObserver() override; - - // Starts observing a service for sync disables. - void ObserveServiceForSyncDisables(syncer::SyncService* sync_service, - PrefService* pref_service); - - // Returns true iff all sync states alllow UKM to be enabled. This means that - // for all profiles: - // * If unified consent is disabled, then sync is initialized, connected, has - // the HISTORY_DELETE_DIRECTIVES data type enabled, and does not have a - // secondary passphrase enabled. - // * If unified consent is enabled, then URL-keyed anonymized data collection - // is enabled for that profile. - virtual bool SyncStateAllowsUkm(); - - // Returns true iff sync is in a state that allows UKM to capture extensions. - // This means that all profiles have EXTENSIONS data type enabled for syncing. - virtual bool SyncStateAllowsExtensionUkm(); - - protected: - // Called after state changes and some profile has sync disabled. - // If |must_purge| is true, sync was disabled for some profile, and - // local data should be purged. - virtual void OnSyncPrefsChanged(bool must_purge) = 0; - - private: - // syncer::SyncServiceObserver: - void OnStateChanged(syncer::SyncService* sync) override; - void OnSyncShutdown(syncer::SyncService* sync) override; - - // unified_consent::UrlKeyedDataCollectionConsentHelper::Observer: - void OnUrlKeyedDataCollectionConsentStateChanged( - unified_consent::UrlKeyedDataCollectionConsentHelper* consent_helper) - override; - - // Recomputes all_profiles_enabled_ state from previous_states_; - void UpdateAllProfileEnabled(bool must_purge); - - // Returns true iff all sync states in previous_states_ allow UKM. - // If there are no profiles being observed, this returns false. - bool CheckSyncStateOnAllProfiles(); - - // Returns true iff all sync states in previous_states_ allow extension UKM. - // If there are no profiles being observed, this returns false. - bool CheckSyncStateForExtensionsOnAllProfiles(); - - // Tracks observed history services, for cleanup. - ScopedObserver<syncer::SyncService, syncer::SyncServiceObserver> - sync_observer_; - - enum class DataCollectionState { - // Matches the case when unified consent feature is disabled - kIgnored, - // Unified consent feature is enabled and the user has disabled URL-keyed - // anonymized data collection. - kDisabled, - // Unified consent feature is enabled and the user has enabled URL-keyed - // anonymized data collection. - kEnabled - }; - - // State data about sync services that we need to remember. - struct SyncState { - // Returns true if this sync state allows UKM: - // * If unified consent is disabled, then sync is initialized, connected, - // has history data type enabled, and does not have a secondary passphrase - // enabled. - // * If unified consent is enabled, then URL-keyed anonymized data - // collection is enabled. - bool AllowsUkm() const; - // Returns true if |AllowUkm| and if sync extensions are enabled. - bool AllowsUkmWithExtension() const; - - // If the user has history sync enabled. - bool history_enabled = false; - // If the user has extension sync enabled. - bool extensions_enabled = false; - // Whether the sync service has been initialized. - bool initialized = false; - // Whether the sync service is active and operational. - bool connected = false; - // Whether user data is hidden by a secondary passphrase. - // This is not valid if the state is not initialized. - bool passphrase_protected = false; - - // Whether anonymized data collection is enabled. - // Note: This is not managed by sync service. It was added in this enum - // for convenience. - DataCollectionState anonymized_data_collection_state = - DataCollectionState::kIgnored; - }; - - // Updates the sync state for |sync| service. Updates all profiles if needed. - void UpdateSyncState( - syncer::SyncService* sync, - unified_consent::UrlKeyedDataCollectionConsentHelper* consent_helper); - - // Gets the current state of a SyncService. - // A non-null |consent_helper| implies that Unified Consent is enabled. - static SyncState GetSyncState( - syncer::SyncService* sync, - unified_consent::UrlKeyedDataCollectionConsentHelper* consent_helper); - - // The state of the sync services being observed. - std::map<syncer::SyncService*, SyncState> previous_states_; - - // The list of URL-keyed anonymized data collection consent helpers. - // - // Note: UrlKeyedDataCollectionConsentHelper do not rely on sync when - // unified consent feature is enabled but there must be exactly one per - // Chromium profile. As there is a single sync service per profile, it is safe - // to key them by sync service instead of introducing an additional map. - std::map< - syncer::SyncService*, - std::unique_ptr<unified_consent::UrlKeyedDataCollectionConsentHelper>> - consent_helpers_; - - // Tracks if UKM is allowed on all profiles after the last state change. - bool all_sync_states_allow_ukm_ = false; - - // Tracks if extension sync was enabled on all profiles after the last state - // change. - bool all_sync_states_allow_extension_ukm_ = false; - - DISALLOW_COPY_AND_ASSIGN(SyncDisableObserver); -}; - -} // namespace ukm - -#endif // COMPONENTS_UKM_OBSERVERS_SYNC_DISABLE_OBSERVER_H_ diff --git a/chromium/components/ukm/observers/sync_disable_observer_unittest.cc b/chromium/components/ukm/observers/sync_disable_observer_unittest.cc deleted file mode 100644 index 34935a7bc06..00000000000 --- a/chromium/components/ukm/observers/sync_disable_observer_unittest.cc +++ /dev/null @@ -1,402 +0,0 @@ -// Copyright 2017 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/observers/sync_disable_observer.h" - -#include "base/observer_list.h" -#include "components/sync/driver/sync_token_status.h" -#include "components/sync/driver/test_sync_service.h" -#include "components/sync/engine/cycle/sync_cycle_snapshot.h" -#include "components/sync_preferences/testing_pref_service_syncable.h" -#include "components/unified_consent/feature.h" -#include "components/unified_consent/pref_names.h" -#include "components/unified_consent/scoped_unified_consent.h" -#include "components/unified_consent/unified_consent_service.h" -#include "testing/gtest/include/gtest/gtest.h" - -using unified_consent::ScopedUnifiedConsent; -using unified_consent::UnifiedConsentFeatureState; - -namespace ukm { - -namespace { - -class MockSyncService : public syncer::TestSyncService { - public: - MockSyncService() { - SetTransportState(TransportState::INITIALIZING); - SetLastCycleSnapshot(syncer::SyncCycleSnapshot()); - } - ~MockSyncService() override { Shutdown(); } - - void SetStatus(bool has_passphrase, bool history_enabled, bool active) { - SetTransportState(active ? TransportState::ACTIVE - : TransportState::INITIALIZING); - SetIsUsingSecondaryPassphrase(has_passphrase); - SetPreferredDataTypes( - history_enabled - ? syncer::ModelTypeSet(syncer::HISTORY_DELETE_DIRECTIVES) - : syncer::ModelTypeSet()); - - // It doesn't matter what exactly we set here, it's only relevant that the - // SyncCycleSnapshot is initialized at all. - SetLastCycleSnapshot(syncer::SyncCycleSnapshot( - /*birthday=*/std::string(), /*bag_of_chips=*/std::string(), - syncer::ModelNeutralState(), syncer::ProgressMarkerMap(), false, 0, 0, - 0, true, 0, base::Time::Now(), base::Time::Now(), - std::vector<int>(syncer::ModelType::NUM_ENTRIES, 0), - std::vector<int>(syncer::ModelType::NUM_ENTRIES, 0), - sync_pb::SyncEnums::UNKNOWN_ORIGIN, base::TimeDelta::FromMinutes(1), - false)); - - NotifyObserversOfStateChanged(); - } - - void SetAuthError(GoogleServiceAuthError::State error_state) { - syncer::TestSyncService::SetAuthError(GoogleServiceAuthError(error_state)); - NotifyObserversOfStateChanged(); - } - - void Shutdown() override { - for (auto& observer : observers_) { - observer.OnSyncShutdown(this); - } - } - - private: - // syncer::TestSyncService: - void AddObserver(syncer::SyncServiceObserver* observer) override { - observers_.AddObserver(observer); - } - void RemoveObserver(syncer::SyncServiceObserver* observer) override { - observers_.RemoveObserver(observer); - } - - void NotifyObserversOfStateChanged() { - for (auto& observer : observers_) { - observer.OnStateChanged(this); - } - } - - // The list of observers of the SyncService state. - base::ObserverList<syncer::SyncServiceObserver>::Unchecked observers_; - - DISALLOW_COPY_AND_ASSIGN(MockSyncService); -}; - -class TestSyncDisableObserver : public SyncDisableObserver { - public: - TestSyncDisableObserver() : purged_(false), notified_(false) {} - ~TestSyncDisableObserver() override {} - - bool ResetPurged() { - bool was_purged = purged_; - purged_ = false; - return was_purged; - } - - bool ResetNotified() { - bool notified = notified_; - notified_ = false; - return notified; - } - - private: - // SyncDisableObserver: - void OnSyncPrefsChanged(bool must_purge) override { - notified_ = true; - purged_ = purged_ || must_purge; - } - bool purged_; - bool notified_; - DISALLOW_COPY_AND_ASSIGN(TestSyncDisableObserver); -}; - -class SyncDisableObserverTest : public testing::Test { - public: - SyncDisableObserverTest() {} - void RegisterUrlKeyedAnonymizedDataCollectionPref( - sync_preferences::TestingPrefServiceSyncable& prefs) { - unified_consent::UnifiedConsentService::RegisterPrefs(prefs.registry()); - } - - void SetUrlKeyedAnonymizedDataCollectionEnabled(PrefService* prefs, - bool enabled) { - DCHECK(unified_consent::IsUnifiedConsentFeatureEnabled()); - prefs->SetBoolean( - unified_consent::prefs::kUrlKeyedAnonymizedDataCollectionEnabled, - enabled); - } - - private: - DISALLOW_COPY_AND_ASSIGN(SyncDisableObserverTest); -}; - -} // namespace - -TEST_F(SyncDisableObserverTest, NoProfiles) { - TestSyncDisableObserver observer; - EXPECT_FALSE(observer.SyncStateAllowsUkm()); - EXPECT_FALSE(observer.ResetNotified()); - EXPECT_FALSE(observer.ResetPurged()); -} - -TEST_F(SyncDisableObserverTest, NotActive) { - MockSyncService sync; - sync.SetStatus(false, true, false); - sync_preferences::TestingPrefServiceSyncable prefs; - RegisterUrlKeyedAnonymizedDataCollectionPref(prefs); - TestSyncDisableObserver observer; - observer.ObserveServiceForSyncDisables(&sync, &prefs); - EXPECT_FALSE(observer.SyncStateAllowsUkm()); - EXPECT_FALSE(observer.ResetNotified()); - EXPECT_FALSE(observer.ResetPurged()); -} - -TEST_F(SyncDisableObserverTest, OneEnabled_UnifiedConsentDisabled) { - ScopedUnifiedConsent scoped_unified_consent( - UnifiedConsentFeatureState::kDisabled); - TestSyncDisableObserver observer; - MockSyncService sync; - sync.SetStatus(false, true, true); - observer.ObserveServiceForSyncDisables(&sync, nullptr); - EXPECT_TRUE(observer.SyncStateAllowsUkm()); - EXPECT_TRUE(observer.ResetNotified()); - EXPECT_FALSE(observer.ResetPurged()); -} - -TEST_F(SyncDisableObserverTest, OneEnabled_UnifiedConsentEnabled) { - ScopedUnifiedConsent scoped_unified_consent( - UnifiedConsentFeatureState::kEnabled); - sync_preferences::TestingPrefServiceSyncable prefs; - RegisterUrlKeyedAnonymizedDataCollectionPref(prefs); - SetUrlKeyedAnonymizedDataCollectionEnabled(&prefs, true); - MockSyncService sync; - for (bool has_passphrase : {true, false}) { - for (bool history_enabled : {true, false}) { - TestSyncDisableObserver observer; - sync.SetStatus(has_passphrase, history_enabled, /*active=*/true); - observer.ObserveServiceForSyncDisables(&sync, &prefs); - EXPECT_TRUE(observer.SyncStateAllowsUkm()); - EXPECT_TRUE(observer.ResetNotified()); - EXPECT_FALSE(observer.ResetPurged()); - } - } -} - -TEST_F(SyncDisableObserverTest, Passphrased_UnifiedConsentDisabled) { - ScopedUnifiedConsent scoped_unified_consent( - UnifiedConsentFeatureState::kDisabled); - MockSyncService sync; - sync.SetStatus(true, true, true); - TestSyncDisableObserver observer; - observer.ObserveServiceForSyncDisables(&sync, nullptr); - EXPECT_FALSE(observer.SyncStateAllowsUkm()); - EXPECT_FALSE(observer.ResetNotified()); - EXPECT_FALSE(observer.ResetPurged()); -} - -TEST_F(SyncDisableObserverTest, HistoryDisabled_UnifiedConsentDisabled) { - ScopedUnifiedConsent scoped_unified_consent( - UnifiedConsentFeatureState::kDisabled); - TestSyncDisableObserver observer; - MockSyncService sync; - sync.SetStatus(false, false, true); - observer.ObserveServiceForSyncDisables(&sync, nullptr); - EXPECT_FALSE(observer.SyncStateAllowsUkm()); - EXPECT_FALSE(observer.ResetNotified()); - EXPECT_FALSE(observer.ResetPurged()); -} - -TEST_F(SyncDisableObserverTest, AuthError_UnifiedConsentDisabled) { - ScopedUnifiedConsent scoped_unified_consent( - UnifiedConsentFeatureState::kDisabled); - TestSyncDisableObserver observer; - MockSyncService sync; - sync.SetStatus(false, true, true); - observer.ObserveServiceForSyncDisables(&sync, nullptr); - EXPECT_TRUE(observer.SyncStateAllowsUkm()); - sync.SetAuthError(GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS); - EXPECT_FALSE(observer.SyncStateAllowsUkm()); - sync.SetAuthError(GoogleServiceAuthError::NONE); - EXPECT_TRUE(observer.SyncStateAllowsUkm()); -} - -TEST_F(SyncDisableObserverTest, MixedProfiles1_UnifiedConsentDisabled) { - ScopedUnifiedConsent scoped_unified_consent( - UnifiedConsentFeatureState::kDisabled); - TestSyncDisableObserver observer; - MockSyncService sync1; - sync1.SetStatus(false, false, true); - observer.ObserveServiceForSyncDisables(&sync1, nullptr); - MockSyncService sync2; - sync2.SetStatus(false, true, true); - observer.ObserveServiceForSyncDisables(&sync2, nullptr); - EXPECT_FALSE(observer.SyncStateAllowsUkm()); - EXPECT_FALSE(observer.ResetNotified()); - EXPECT_FALSE(observer.ResetPurged()); -} - -TEST_F(SyncDisableObserverTest, MixedProfiles2_UnifiedConsentDisabled) { - ScopedUnifiedConsent scoped_unified_consent( - UnifiedConsentFeatureState::kDisabled); - TestSyncDisableObserver observer; - MockSyncService sync1; - sync1.SetStatus(false, true, true); - observer.ObserveServiceForSyncDisables(&sync1, nullptr); - EXPECT_TRUE(observer.ResetNotified()); - - MockSyncService sync2; - sync2.SetStatus(false, false, true); - observer.ObserveServiceForSyncDisables(&sync2, nullptr); - EXPECT_FALSE(observer.SyncStateAllowsUkm()); - EXPECT_TRUE(observer.ResetNotified()); - EXPECT_FALSE(observer.ResetPurged()); - sync2.Shutdown(); - EXPECT_TRUE(observer.SyncStateAllowsUkm()); - EXPECT_TRUE(observer.ResetNotified()); - EXPECT_FALSE(observer.ResetPurged()); -} - -TEST_F(SyncDisableObserverTest, MixedProfiles_UnifiedConsentEnabled) { - ScopedUnifiedConsent scoped_unified_consent( - UnifiedConsentFeatureState::kEnabled); - sync_preferences::TestingPrefServiceSyncable prefs1; - RegisterUrlKeyedAnonymizedDataCollectionPref(prefs1); - sync_preferences::TestingPrefServiceSyncable prefs2; - RegisterUrlKeyedAnonymizedDataCollectionPref(prefs2); - - TestSyncDisableObserver observer; - MockSyncService sync1; - SetUrlKeyedAnonymizedDataCollectionEnabled(&prefs1, false); - observer.ObserveServiceForSyncDisables(&sync1, &prefs1); - MockSyncService sync2; - SetUrlKeyedAnonymizedDataCollectionEnabled(&prefs2, true); - observer.ObserveServiceForSyncDisables(&sync2, &prefs2); - EXPECT_FALSE(observer.SyncStateAllowsUkm()); - EXPECT_FALSE(observer.ResetNotified()); - EXPECT_FALSE(observer.ResetPurged()); -} - -TEST_F(SyncDisableObserverTest, TwoEnabled_UnifiedConsentDisabled) { - ScopedUnifiedConsent scoped_unified_consent( - UnifiedConsentFeatureState::kDisabled); - TestSyncDisableObserver observer; - MockSyncService sync1; - sync1.SetStatus(false, true, true); - observer.ObserveServiceForSyncDisables(&sync1, nullptr); - EXPECT_TRUE(observer.ResetNotified()); - MockSyncService sync2; - sync2.SetStatus(false, true, true); - observer.ObserveServiceForSyncDisables(&sync2, nullptr); - EXPECT_TRUE(observer.SyncStateAllowsUkm()); - EXPECT_FALSE(observer.ResetNotified()); - EXPECT_FALSE(observer.ResetPurged()); -} - -TEST_F(SyncDisableObserverTest, TwoEnabled_UnifiedConsentEnabled) { - ScopedUnifiedConsent scoped_unified_consent( - UnifiedConsentFeatureState::kEnabled); - sync_preferences::TestingPrefServiceSyncable prefs1; - RegisterUrlKeyedAnonymizedDataCollectionPref(prefs1); - sync_preferences::TestingPrefServiceSyncable prefs2; - RegisterUrlKeyedAnonymizedDataCollectionPref(prefs2); - - TestSyncDisableObserver observer; - MockSyncService sync1; - SetUrlKeyedAnonymizedDataCollectionEnabled(&prefs1, true); - observer.ObserveServiceForSyncDisables(&sync1, &prefs1); - EXPECT_TRUE(observer.ResetNotified()); - MockSyncService sync2; - SetUrlKeyedAnonymizedDataCollectionEnabled(&prefs2, true); - observer.ObserveServiceForSyncDisables(&sync2, &prefs2); - EXPECT_TRUE(observer.SyncStateAllowsUkm()); - EXPECT_FALSE(observer.ResetNotified()); - EXPECT_FALSE(observer.ResetPurged()); -} - -TEST_F(SyncDisableObserverTest, OneAddRemove_UnifiedConsentDisabled) { - ScopedUnifiedConsent scoped_unified_consent( - UnifiedConsentFeatureState::kDisabled); - TestSyncDisableObserver observer; - MockSyncService sync; - observer.ObserveServiceForSyncDisables(&sync, nullptr); - EXPECT_FALSE(observer.SyncStateAllowsUkm()); - EXPECT_FALSE(observer.ResetNotified()); - EXPECT_FALSE(observer.ResetPurged()); - sync.SetStatus(false, true, true); - EXPECT_TRUE(observer.SyncStateAllowsUkm()); - EXPECT_TRUE(observer.ResetNotified()); - EXPECT_FALSE(observer.ResetPurged()); - sync.Shutdown(); - EXPECT_FALSE(observer.SyncStateAllowsUkm()); - EXPECT_TRUE(observer.ResetNotified()); - EXPECT_FALSE(observer.ResetPurged()); -} - -TEST_F(SyncDisableObserverTest, OneAddRemove_UnifiedConsentEnabled) { - ScopedUnifiedConsent scoped_unified_consent( - UnifiedConsentFeatureState::kEnabled); - sync_preferences::TestingPrefServiceSyncable prefs; - RegisterUrlKeyedAnonymizedDataCollectionPref(prefs); - TestSyncDisableObserver observer; - MockSyncService sync; - observer.ObserveServiceForSyncDisables(&sync, &prefs); - EXPECT_FALSE(observer.SyncStateAllowsUkm()); - EXPECT_FALSE(observer.ResetNotified()); - EXPECT_FALSE(observer.ResetPurged()); - SetUrlKeyedAnonymizedDataCollectionEnabled(&prefs, true); - EXPECT_TRUE(observer.SyncStateAllowsUkm()); - EXPECT_TRUE(observer.ResetNotified()); - EXPECT_FALSE(observer.ResetPurged()); - sync.Shutdown(); - EXPECT_FALSE(observer.SyncStateAllowsUkm()); - EXPECT_TRUE(observer.ResetNotified()); - EXPECT_FALSE(observer.ResetPurged()); -} - -TEST_F(SyncDisableObserverTest, PurgeOnDisable_UnifiedConsentDisabled) { - ScopedUnifiedConsent scoped_unified_consent( - UnifiedConsentFeatureState::kDisabled); - TestSyncDisableObserver observer; - MockSyncService sync; - sync.SetStatus(false, true, true); - observer.ObserveServiceForSyncDisables(&sync, nullptr); - EXPECT_TRUE(observer.SyncStateAllowsUkm()); - EXPECT_TRUE(observer.ResetNotified()); - EXPECT_FALSE(observer.ResetPurged()); - sync.SetStatus(false, false, true); - EXPECT_FALSE(observer.SyncStateAllowsUkm()); - EXPECT_TRUE(observer.ResetNotified()); - EXPECT_TRUE(observer.ResetPurged()); - sync.Shutdown(); - EXPECT_FALSE(observer.SyncStateAllowsUkm()); - EXPECT_FALSE(observer.ResetNotified()); - EXPECT_FALSE(observer.ResetPurged()); -} - -TEST_F(SyncDisableObserverTest, PurgeOnDisable_UnifiedConsentEnabled) { - ScopedUnifiedConsent scoped_unified_consent( - UnifiedConsentFeatureState::kEnabled); - - sync_preferences::TestingPrefServiceSyncable prefs; - RegisterUrlKeyedAnonymizedDataCollectionPref(prefs); - TestSyncDisableObserver observer; - MockSyncService sync; - SetUrlKeyedAnonymizedDataCollectionEnabled(&prefs, true); - observer.ObserveServiceForSyncDisables(&sync, &prefs); - EXPECT_TRUE(observer.SyncStateAllowsUkm()); - EXPECT_TRUE(observer.ResetNotified()); - EXPECT_FALSE(observer.ResetPurged()); - SetUrlKeyedAnonymizedDataCollectionEnabled(&prefs, false); - EXPECT_FALSE(observer.SyncStateAllowsUkm()); - EXPECT_TRUE(observer.ResetNotified()); - EXPECT_TRUE(observer.ResetPurged()); - sync.Shutdown(); - EXPECT_FALSE(observer.SyncStateAllowsUkm()); - EXPECT_FALSE(observer.ResetNotified()); - EXPECT_FALSE(observer.ResetPurged()); -} - -} // namespace ukm diff --git a/chromium/components/ukm/observers/ukm_consent_state_observer.cc b/chromium/components/ukm/observers/ukm_consent_state_observer.cc new file mode 100644 index 00000000000..96bfed06bfc --- /dev/null +++ b/chromium/components/ukm/observers/ukm_consent_state_observer.cc @@ -0,0 +1,165 @@ +// Copyright 2017 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/observers/ukm_consent_state_observer.h" + +#include <memory> +#include <utility> + +#include "base/feature_list.h" +#include "base/metrics/histogram_macros.h" +#include "base/stl_util.h" +#include "components/sync/driver/sync_user_settings.h" +#include "components/unified_consent/url_keyed_data_collection_consent_helper.h" +#include "google_apis/gaia/google_service_auth_error.h" + +using unified_consent::UrlKeyedDataCollectionConsentHelper; + +namespace ukm { + +UkmConsentStateObserver::UkmConsentStateObserver() : sync_observer_(this) {} + +UkmConsentStateObserver::~UkmConsentStateObserver() { + for (const auto& entry : consent_helpers_) { + entry.second->RemoveObserver(this); + } +} + +bool UkmConsentStateObserver::ProfileState::AllowsUkm() const { + return anonymized_data_collection_enabled; +} + +// static +UkmConsentStateObserver::ProfileState UkmConsentStateObserver::GetProfileState( + syncer::SyncService* sync_service, + UrlKeyedDataCollectionConsentHelper* consent_helper) { + DCHECK(sync_service); + DCHECK(consent_helper); + ProfileState state; + state.anonymized_data_collection_enabled = consent_helper->IsEnabled(); + state.extensions_enabled = + sync_service->GetPreferredDataTypes().Has(syncer::EXTENSIONS) && + sync_service->IsSyncFeatureEnabled(); + return state; +} + +void UkmConsentStateObserver::StartObserving(syncer::SyncService* sync_service, + PrefService* prefs) { + std::unique_ptr<UrlKeyedDataCollectionConsentHelper> consent_helper = + UrlKeyedDataCollectionConsentHelper:: + NewAnonymizedDataCollectionConsentHelper(prefs, sync_service); + + ProfileState state = GetProfileState(sync_service, consent_helper.get()); + previous_states_[sync_service] = state; + + consent_helper->AddObserver(this); + consent_helpers_[sync_service] = std::move(consent_helper); + sync_observer_.Add(sync_service); + UpdateUkmAllowedForAllProfiles(false); +} + +void UkmConsentStateObserver::UpdateUkmAllowedForAllProfiles(bool must_purge) { + bool all_profile_states_allow_ukm = CheckPreviousStatesAllowUkm(); + bool all_profile_states_allow_extension_ukm = + all_profile_states_allow_ukm && CheckPreviousStatesAllowExtensionUkm(); + + UMA_HISTOGRAM_BOOLEAN("UKM.ConsentObserver.AllowedForAllProfiles", + all_profile_states_allow_ukm); + + // Any change in profile states needs to call OnUkmAllowedStateChanged so that + // the new settings take effect. + if (must_purge || + (all_profile_states_allow_ukm != ukm_allowed_for_all_profiles_) || + (all_profile_states_allow_extension_ukm != + ukm_allowed_with_extensions_for_all_profiles_)) { + ukm_allowed_for_all_profiles_ = all_profile_states_allow_ukm; + ukm_allowed_with_extensions_for_all_profiles_ = + all_profile_states_allow_extension_ukm; + OnUkmAllowedStateChanged(must_purge); + } +} + +bool UkmConsentStateObserver::CheckPreviousStatesAllowUkm() { + if (previous_states_.empty()) + return false; + for (const auto& kv : previous_states_) { + const ProfileState& state = kv.second; + if (!state.AllowsUkm()) + return false; + } + + return true; +} + +bool UkmConsentStateObserver::CheckPreviousStatesAllowExtensionUkm() { + if (previous_states_.empty()) + return false; + for (const auto& kv : previous_states_) { + const ProfileState& state = kv.second; + if (!state.extensions_enabled) + return false; + } + return true; +} + +void UkmConsentStateObserver::OnStateChanged(syncer::SyncService* sync) { + UrlKeyedDataCollectionConsentHelper* consent_helper = nullptr; + auto found = consent_helpers_.find(sync); + if (found != consent_helpers_.end()) + consent_helper = found->second.get(); + UpdateProfileState(sync, consent_helper); +} + +void UkmConsentStateObserver::OnUrlKeyedDataCollectionConsentStateChanged( + unified_consent::UrlKeyedDataCollectionConsentHelper* consent_helper) { + DCHECK(consent_helper); + syncer::SyncService* sync_service = nullptr; + for (const auto& entry : consent_helpers_) { + if (consent_helper == entry.second.get()) { + sync_service = entry.first; + break; + } + } + DCHECK(sync_service); + UpdateProfileState(sync_service, consent_helper); +} + +void UkmConsentStateObserver::UpdateProfileState( + syncer::SyncService* sync, + UrlKeyedDataCollectionConsentHelper* consent_helper) { + DCHECK(base::Contains(previous_states_, sync)); + const ProfileState& previous_state = previous_states_[sync]; + DCHECK(consent_helper); + ProfileState state = GetProfileState(sync, consent_helper); + + // Trigger a purge if the current state no longer allows UKM. + bool must_purge = previous_state.AllowsUkm() && !state.AllowsUkm(); + + UMA_HISTOGRAM_BOOLEAN("UKM.ConsentObserver.Purge", must_purge); + + previous_states_[sync] = state; + UpdateUkmAllowedForAllProfiles(must_purge); +} + +void UkmConsentStateObserver::OnSyncShutdown(syncer::SyncService* sync) { + DCHECK(base::Contains(previous_states_, sync)); + auto found = consent_helpers_.find(sync); + if (found != consent_helpers_.end()) { + found->second->RemoveObserver(this); + consent_helpers_.erase(found); + } + sync_observer_.Remove(sync); + previous_states_.erase(sync); + UpdateUkmAllowedForAllProfiles(/*must_purge=*/false); +} + +bool UkmConsentStateObserver::IsUkmAllowedForAllProfiles() { + return ukm_allowed_for_all_profiles_; +} + +bool UkmConsentStateObserver::IsUkmAllowedWithExtensionsForAllProfiles() { + return ukm_allowed_with_extensions_for_all_profiles_; +} + +} // namespace ukm diff --git a/chromium/components/ukm/observers/ukm_consent_state_observer.h b/chromium/components/ukm/observers/ukm_consent_state_observer.h new file mode 100644 index 00000000000..4d214765b1c --- /dev/null +++ b/chromium/components/ukm/observers/ukm_consent_state_observer.h @@ -0,0 +1,134 @@ +// Copyright 2017 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_OBSERVERS_UKM_CONSENT_STATE_OBSERVER_H_ +#define COMPONENTS_UKM_OBSERVERS_UKM_CONSENT_STATE_OBSERVER_H_ + +#include <map> + +#include "base/scoped_observer.h" +#include "components/sync/driver/sync_service.h" +#include "components/sync/driver/sync_service_observer.h" +#include "components/unified_consent/url_keyed_data_collection_consent_helper.h" + +class PrefService; + +namespace ukm { + +// Observer that monitors whether UKM is allowed for all profiles. +// +// For one profile, UKM is allowed iff URL-keyed anonymized data collection is +// enabled. +class UkmConsentStateObserver + : public syncer::SyncServiceObserver, + public unified_consent::UrlKeyedDataCollectionConsentHelper::Observer { + public: + UkmConsentStateObserver(); + ~UkmConsentStateObserver() override; + + // Starts observing whether UKM is allowed for a profile. + // |pref_service| is the pref service of a profile. + void StartObserving(syncer::SyncService* sync_service, + PrefService* pref_service); + + // Returns true iff all UKM is allowed for all profile states. This means that + // URL-keyed anonymized data collection is enabled for all profiles. + virtual bool IsUkmAllowedForAllProfiles(); + + // Returns true iff sync is in a state that allows UKM to capture extensions. + // This means that all profiles have EXTENSIONS data type enabled for syncing. + virtual bool IsUkmAllowedWithExtensionsForAllProfiles(); + + protected: + // Called after UKM consent state changed. + // If |must_purge| is true, the UKM is not allowed for some profile, and local + // data must be purged. + virtual void OnUkmAllowedStateChanged(bool must_purge) = 0; + + private: + // syncer::SyncServiceObserver: + void OnStateChanged(syncer::SyncService* sync) override; + void OnSyncShutdown(syncer::SyncService* sync) override; + + // unified_consent::UrlKeyedDataCollectionConsentHelper::Observer: + void OnUrlKeyedDataCollectionConsentStateChanged( + unified_consent::UrlKeyedDataCollectionConsentHelper* consent_helper) + override; + + // Recomputes |ukm_allowed_for_all_profiles_| and + // |ukm_allowed_with_extensions_for_all_profiles_| from |previous_states_|; + void UpdateUkmAllowedForAllProfiles(bool must_purge); + + // Returns true iff all profile states in |previous_states_| allow UKM. + // If there are no profiles being observed, this returns false. + bool CheckPreviousStatesAllowUkm(); + + // Returns true iff all profile states in |previous_states_| allow extension + // UKM. If there are no profiles are being observed, this returns false. + bool CheckPreviousStatesAllowExtensionUkm(); + + // Tracks observed sync services, for cleanup. + ScopedObserver<syncer::SyncService, syncer::SyncServiceObserver> + sync_observer_; + + // State data about profiles that we need to remember. + struct ProfileState { + // Returns true if this state allows UKM (i.e. URL-keyed anonymized + // data collection is enabled). + bool AllowsUkm() const; + + // Returns true if |AllowsUkm| and if sync extensions are enabled. + bool AllowsUkmWithExtension() const; + + // Whether anonymized data collection is enabled. + bool anonymized_data_collection_enabled = false; + + // If the user has extension sync enabled. + bool extensions_enabled = false; + }; + + // Updates the UKM enabled state for a profile and then triggers an update of + // the state for all profiles. + // |sync| and |consent_helper| must not be null. + void UpdateProfileState( + syncer::SyncService* sync, + unified_consent::UrlKeyedDataCollectionConsentHelper* consent_helper); + + // Gets the current state of a profile. + // |sync| and |consent_helper| must not be null. + static ProfileState GetProfileState( + syncer::SyncService* sync, + unified_consent::UrlKeyedDataCollectionConsentHelper* consent_helper); + + // The state of the profile being observed. + // + // Note: UKM consent does not rely on sync but there must be exactly one + // sync service per profile, so it is safe to key the profile states by the + // sync service. + std::map<syncer::SyncService*, ProfileState> previous_states_; + + // The list of URL-keyed anonymized data collection consent helpers. + // + // Note: UrlKeyedDataCollectionConsentHelper does not rely on sync but there + // must be exactly one per Chromium profile. As there is a single sync service + // per profile, it is safe to key them by sync service instead of introducing + // an additional map. + std::map< + syncer::SyncService*, + std::unique_ptr<unified_consent::UrlKeyedDataCollectionConsentHelper>> + consent_helpers_; + + // Tracks if UKM is allowed on all profiles after the last state change. + bool ukm_allowed_for_all_profiles_ = false; + + // Tracks if extension sync was enabled on all profiles after the last state + // change. + bool ukm_allowed_with_extensions_for_all_profiles_ = false; + + DISALLOW_COPY_AND_ASSIGN(UkmConsentStateObserver); +}; + +} // namespace ukm + +#endif // COMPONENTS_UKM_OBSERVERS_UKM_CONSENT_STATE_OBSERVER_H_ diff --git a/chromium/components/ukm/observers/ukm_consent_state_observer_unittest.cc b/chromium/components/ukm/observers/ukm_consent_state_observer_unittest.cc new file mode 100644 index 00000000000..d9faae1ec90 --- /dev/null +++ b/chromium/components/ukm/observers/ukm_consent_state_observer_unittest.cc @@ -0,0 +1,239 @@ +// Copyright 2017 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/observers/ukm_consent_state_observer.h" + +#include "base/observer_list.h" +#include "components/sync/driver/sync_token_status.h" +#include "components/sync/driver/test_sync_service.h" +#include "components/sync/engine/cycle/sync_cycle_snapshot.h" +#include "components/sync_preferences/testing_pref_service_syncable.h" +#include "components/unified_consent/pref_names.h" +#include "components/unified_consent/unified_consent_service.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace ukm { + +namespace { + +class MockSyncService : public syncer::TestSyncService { + public: + MockSyncService() { + SetTransportState(TransportState::INITIALIZING); + SetLastCycleSnapshot(syncer::SyncCycleSnapshot()); + } + ~MockSyncService() override { Shutdown(); } + + void SetStatus(bool has_passphrase, bool history_enabled, bool active) { + SetTransportState(active ? TransportState::ACTIVE + : TransportState::INITIALIZING); + SetIsUsingSecondaryPassphrase(has_passphrase); + SetPreferredDataTypes( + history_enabled + ? syncer::ModelTypeSet(syncer::HISTORY_DELETE_DIRECTIVES) + : syncer::ModelTypeSet()); + + // It doesn't matter what exactly we set here, it's only relevant that the + // SyncCycleSnapshot is initialized at all. + SetLastCycleSnapshot(syncer::SyncCycleSnapshot( + /*birthday=*/std::string(), /*bag_of_chips=*/std::string(), + syncer::ModelNeutralState(), syncer::ProgressMarkerMap(), false, 0, 0, + 0, true, 0, base::Time::Now(), base::Time::Now(), + std::vector<int>(syncer::ModelType::NUM_ENTRIES, 0), + std::vector<int>(syncer::ModelType::NUM_ENTRIES, 0), + sync_pb::SyncEnums::UNKNOWN_ORIGIN, base::TimeDelta::FromMinutes(1), + false)); + + NotifyObserversOfStateChanged(); + } + + void SetAuthError(GoogleServiceAuthError::State error_state) { + syncer::TestSyncService::SetAuthError(GoogleServiceAuthError(error_state)); + NotifyObserversOfStateChanged(); + } + + void Shutdown() override { + for (auto& observer : observers_) { + observer.OnSyncShutdown(this); + } + } + + private: + // syncer::TestSyncService: + void AddObserver(syncer::SyncServiceObserver* observer) override { + observers_.AddObserver(observer); + } + void RemoveObserver(syncer::SyncServiceObserver* observer) override { + observers_.RemoveObserver(observer); + } + + void NotifyObserversOfStateChanged() { + for (auto& observer : observers_) { + observer.OnStateChanged(this); + } + } + + // The list of observers of the SyncService state. + base::ObserverList<syncer::SyncServiceObserver>::Unchecked observers_; + + DISALLOW_COPY_AND_ASSIGN(MockSyncService); +}; + +class TestUkmConsentStateObserver : public UkmConsentStateObserver { + public: + TestUkmConsentStateObserver() : purged_(false), notified_(false) {} + ~TestUkmConsentStateObserver() override {} + + bool ResetPurged() { + bool was_purged = purged_; + purged_ = false; + return was_purged; + } + + bool ResetNotified() { + bool notified = notified_; + notified_ = false; + return notified; + } + + private: + // UkmConsentStateObserver: + void OnUkmAllowedStateChanged(bool must_purge) override { + notified_ = true; + purged_ = purged_ || must_purge; + } + bool purged_; + bool notified_; + DISALLOW_COPY_AND_ASSIGN(TestUkmConsentStateObserver); +}; + +class UkmConsentStateObserverTest : public testing::Test { + public: + UkmConsentStateObserverTest() {} + void RegisterUrlKeyedAnonymizedDataCollectionPref( + sync_preferences::TestingPrefServiceSyncable& prefs) { + unified_consent::UnifiedConsentService::RegisterPrefs(prefs.registry()); + } + + void SetUrlKeyedAnonymizedDataCollectionEnabled(PrefService* prefs, + bool enabled) { + prefs->SetBoolean( + unified_consent::prefs::kUrlKeyedAnonymizedDataCollectionEnabled, + enabled); + } + + private: + DISALLOW_COPY_AND_ASSIGN(UkmConsentStateObserverTest); +}; + +} // namespace + +TEST_F(UkmConsentStateObserverTest, NoProfiles) { + TestUkmConsentStateObserver observer; + EXPECT_FALSE(observer.IsUkmAllowedForAllProfiles()); + EXPECT_FALSE(observer.ResetNotified()); + EXPECT_FALSE(observer.ResetPurged()); +} + +TEST_F(UkmConsentStateObserverTest, NotActive) { + MockSyncService sync; + sync.SetStatus(false, true, false); + sync_preferences::TestingPrefServiceSyncable prefs; + RegisterUrlKeyedAnonymizedDataCollectionPref(prefs); + TestUkmConsentStateObserver observer; + observer.StartObserving(&sync, &prefs); + EXPECT_FALSE(observer.IsUkmAllowedForAllProfiles()); + EXPECT_FALSE(observer.ResetNotified()); + EXPECT_FALSE(observer.ResetPurged()); +} + +TEST_F(UkmConsentStateObserverTest, OneEnabled) { + MockSyncService sync; + sync_preferences::TestingPrefServiceSyncable prefs; + RegisterUrlKeyedAnonymizedDataCollectionPref(prefs); + SetUrlKeyedAnonymizedDataCollectionEnabled(&prefs, true); + TestUkmConsentStateObserver observer; + observer.StartObserving(&sync, &prefs); + EXPECT_TRUE(observer.IsUkmAllowedForAllProfiles()); + EXPECT_TRUE(observer.ResetNotified()); + EXPECT_FALSE(observer.ResetPurged()); +} + +TEST_F(UkmConsentStateObserverTest, MixedProfiles) { + sync_preferences::TestingPrefServiceSyncable prefs1; + RegisterUrlKeyedAnonymizedDataCollectionPref(prefs1); + sync_preferences::TestingPrefServiceSyncable prefs2; + RegisterUrlKeyedAnonymizedDataCollectionPref(prefs2); + + TestUkmConsentStateObserver observer; + MockSyncService sync1; + SetUrlKeyedAnonymizedDataCollectionEnabled(&prefs1, false); + observer.StartObserving(&sync1, &prefs1); + MockSyncService sync2; + SetUrlKeyedAnonymizedDataCollectionEnabled(&prefs2, true); + observer.StartObserving(&sync2, &prefs2); + EXPECT_FALSE(observer.IsUkmAllowedForAllProfiles()); + EXPECT_FALSE(observer.ResetNotified()); + EXPECT_FALSE(observer.ResetPurged()); +} + +TEST_F(UkmConsentStateObserverTest, TwoEnabled) { + sync_preferences::TestingPrefServiceSyncable prefs1; + RegisterUrlKeyedAnonymizedDataCollectionPref(prefs1); + sync_preferences::TestingPrefServiceSyncable prefs2; + RegisterUrlKeyedAnonymizedDataCollectionPref(prefs2); + + TestUkmConsentStateObserver observer; + MockSyncService sync1; + SetUrlKeyedAnonymizedDataCollectionEnabled(&prefs1, true); + observer.StartObserving(&sync1, &prefs1); + EXPECT_TRUE(observer.ResetNotified()); + MockSyncService sync2; + SetUrlKeyedAnonymizedDataCollectionEnabled(&prefs2, true); + observer.StartObserving(&sync2, &prefs2); + EXPECT_TRUE(observer.IsUkmAllowedForAllProfiles()); + EXPECT_FALSE(observer.ResetNotified()); + EXPECT_FALSE(observer.ResetPurged()); +} + +TEST_F(UkmConsentStateObserverTest, OneAddRemove) { + sync_preferences::TestingPrefServiceSyncable prefs; + RegisterUrlKeyedAnonymizedDataCollectionPref(prefs); + TestUkmConsentStateObserver observer; + MockSyncService sync; + observer.StartObserving(&sync, &prefs); + EXPECT_FALSE(observer.IsUkmAllowedForAllProfiles()); + EXPECT_FALSE(observer.ResetNotified()); + EXPECT_FALSE(observer.ResetPurged()); + SetUrlKeyedAnonymizedDataCollectionEnabled(&prefs, true); + EXPECT_TRUE(observer.IsUkmAllowedForAllProfiles()); + EXPECT_TRUE(observer.ResetNotified()); + EXPECT_FALSE(observer.ResetPurged()); + sync.Shutdown(); + EXPECT_FALSE(observer.IsUkmAllowedForAllProfiles()); + EXPECT_TRUE(observer.ResetNotified()); + EXPECT_FALSE(observer.ResetPurged()); +} + +TEST_F(UkmConsentStateObserverTest, PurgeOnDisable) { + sync_preferences::TestingPrefServiceSyncable prefs; + RegisterUrlKeyedAnonymizedDataCollectionPref(prefs); + TestUkmConsentStateObserver observer; + MockSyncService sync; + SetUrlKeyedAnonymizedDataCollectionEnabled(&prefs, true); + observer.StartObserving(&sync, &prefs); + EXPECT_TRUE(observer.IsUkmAllowedForAllProfiles()); + EXPECT_TRUE(observer.ResetNotified()); + EXPECT_FALSE(observer.ResetPurged()); + SetUrlKeyedAnonymizedDataCollectionEnabled(&prefs, false); + EXPECT_FALSE(observer.IsUkmAllowedForAllProfiles()); + EXPECT_TRUE(observer.ResetNotified()); + EXPECT_TRUE(observer.ResetPurged()); + sync.Shutdown(); + EXPECT_FALSE(observer.IsUkmAllowedForAllProfiles()); + EXPECT_FALSE(observer.ResetNotified()); + EXPECT_FALSE(observer.ResetPurged()); +} + +} // namespace ukm diff --git a/chromium/components/ukm/scheme_constants.cc b/chromium/components/ukm/scheme_constants.cc new file mode 100644 index 00000000000..543a7170640 --- /dev/null +++ b/chromium/components/ukm/scheme_constants.cc @@ -0,0 +1,13 @@ +// 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 "components/ukm/scheme_constants.h" + +namespace ukm { + +const char kAppScheme[] = "app"; +const char kChromeUIScheme[] = "chrome"; +const char kExtensionScheme[] = "chrome-extension"; + +} // namespace ukm diff --git a/chromium/components/ukm/scheme_constants.h b/chromium/components/ukm/scheme_constants.h new file mode 100644 index 00000000000..d9a3af6004f --- /dev/null +++ b/chromium/components/ukm/scheme_constants.h @@ -0,0 +1,21 @@ +// 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. + +#ifndef COMPONENTS_UKM_SCHEME_CONSTANTS_H_ +#define COMPONENTS_UKM_SCHEME_CONSTANTS_H_ + +namespace ukm { + +// Defines several URL scheme constants to avoid dependencies. +// kAppScheme will be defined in code that isn't available here. +extern const char kAppScheme[]; +// kChromeUIScheme is defined in content, which this code can't depend on +// since it's used by iOS too. +extern const char kChromeUIScheme[]; +// kExtensionScheme is defined in extensions which also isn't available here. +extern const char kExtensionScheme[]; + +} // namespace ukm + +#endif // COMPONENTS_UKM_SCHEME_CONSTANTS_H_ diff --git a/chromium/components/ukm/test_ukm_recorder.h b/chromium/components/ukm/test_ukm_recorder.h index 15f87aa5f5b..1ddb5e373ae 100644 --- a/chromium/components/ukm/test_ukm_recorder.h +++ b/chromium/components/ukm/test_ukm_recorder.h @@ -7,6 +7,7 @@ #include <stddef.h> +#include <map> #include <memory> #include <set> #include <utility> diff --git a/chromium/components/ukm/ukm_recorder_impl.cc b/chromium/components/ukm/ukm_recorder_impl.cc index cced2e48d0a..35b98693fe1 100644 --- a/chromium/components/ukm/ukm_recorder_impl.cc +++ b/chromium/components/ukm/ukm_recorder_impl.cc @@ -18,6 +18,7 @@ #include "base/rand_util.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_split.h" +#include "components/ukm/scheme_constants.h" #include "components/variations/variations_associated_data.h" #include "services/metrics/public/cpp/ukm_builders.h" #include "services/metrics/public/cpp/ukm_decode.h" @@ -33,14 +34,6 @@ namespace ukm { namespace { -// Note: kChromeUIScheme is defined in content, which this code can't -// depend on - since it's used by iOS too. kExtensionScheme is defined -// in extensions which also isn't always available here. kAppScheme -// will be defined in code that isn't available here. -const char kChromeUIScheme[] = "chrome"; -const char kExtensionScheme[] = "chrome-extension"; -const char kAppScheme[] = "app"; - const base::Feature kUkmSamplingRateFeature{"UkmSamplingRate", base::FEATURE_DISABLED_BY_DEFAULT}; @@ -159,7 +152,7 @@ void AppendWhitelistedUrls( } } -bool HasUnknownMetrics(const ukm::builders::DecodeMap& decode_map, +bool HasUnknownMetrics(const builders::DecodeMap& decode_map, const mojom::UkmEntry& entry) { const auto it = decode_map.find(entry.event_hash); if (it == decode_map.end()) @@ -260,6 +253,32 @@ void UkmRecorderImpl::Purge() { recording_is_continuous_ = false; } +void UkmRecorderImpl::PurgeExtensionRecordings() { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + // Discard all sources that have an extension URL as well as all the entries + // related to any of these sources. + std::unordered_set<SourceId> extension_source_ids; + for (const auto& kv : recordings_.sources) { + if (kv.second->url().SchemeIs(kExtensionScheme)) { + extension_source_ids.insert(kv.first); + } + } + for (const auto source_id : extension_source_ids) { + recordings_.sources.erase(source_id); + } + + std::vector<mojom::UkmEntryPtr>& events = recordings_.entries; + + events.erase( + std::remove_if(events.begin(), events.end(), + [&](const auto& event) { + return extension_source_ids.count(event->source_id); + }), + events.end()); + + recording_is_continuous_ = false; +} + void UkmRecorderImpl::MarkSourceForDeletion(SourceId source_id) { if (source_id == kInvalidSourceId) return; @@ -295,7 +314,7 @@ void UkmRecorderImpl::StoreRecordingsInReport(Report* report) { // Number of sources discarded due to not matching a navigation URL. int num_sources_unmatched = 0; - std::unordered_map<ukm::SourceIdType, int> serialized_source_type_counts; + std::unordered_map<SourceIdType, int> serialized_source_type_counts; for (const auto& kv : recordings_.sources) { // Don't keep sources of these types after current report because their @@ -385,15 +404,14 @@ void UkmRecorderImpl::StoreRecordingsInReport(Report* report) { UMA_HISTOGRAM_COUNTS_1000("UKM.Sources.UnmatchedSourcesCount", num_sources_unmatched); - UMA_HISTOGRAM_COUNTS_1000( - "UKM.Sources.SerializedCount2.Ukm", - serialized_source_type_counts[ukm::SourceIdType::UKM]); + UMA_HISTOGRAM_COUNTS_1000("UKM.Sources.SerializedCount2.Ukm", + serialized_source_type_counts[SourceIdType::UKM]); UMA_HISTOGRAM_COUNTS_1000( "UKM.Sources.SerializedCount2.Navigation", - serialized_source_type_counts[ukm::SourceIdType::NAVIGATION_ID]); + serialized_source_type_counts[SourceIdType::NAVIGATION_ID]); UMA_HISTOGRAM_COUNTS_1000( "UKM.Sources.SerializedCount2.App", - serialized_source_type_counts[ukm::SourceIdType::APP_ID]); + serialized_source_type_counts[SourceIdType::APP_ID]); // We record a UMA metric specifically for the number of serialized events // with the FCP metric. This is for data quality verification. @@ -482,12 +500,10 @@ int UkmRecorderImpl::PruneOldSources(size_t max_kept_sources) { if (recordings_.sources.size() <= max_kept_sources) return 0; - std::vector<std::pair<base::TimeTicks, ukm::SourceId>> - timestamp_source_id_pairs; + std::vector<std::pair<base::TimeTicks, SourceId>> timestamp_source_id_pairs; for (const auto& kv : recordings_.sources) { timestamp_source_id_pairs.push_back( - std::pair<base::TimeTicks, ukm::SourceId>(kv.second->creation_time(), - kv.first)); + std::make_pair(kv.second->creation_time(), kv.first)); } // Partially sort so that the last |max_kept_sources| elements are the // newest. @@ -639,7 +655,7 @@ void UkmRecorderImpl::AddEntry(mojom::UkmEntryPtr entry) { } if (IsSamplingEnabled()) { - if (default_sampling_rate_ == 0) { + if (default_sampling_rate_ < 0) { LoadExperimentSamplingInfo(); } @@ -671,9 +687,10 @@ void UkmRecorderImpl::AddEntry(mojom::UkmEntryPtr entry) { } void UkmRecorderImpl::LoadExperimentSamplingInfo() { - DCHECK_EQ(0, default_sampling_rate_); + // This should be called only if a sampling rate hasn't been loaded. + DCHECK_LT(default_sampling_rate_, 0); - // Default rate must be > 0 to indicate that load is complete. + // Default rate must be >= 0 to indicate that load is complete. default_sampling_rate_ = 1; // If we don't have the feature, no parameters to load. @@ -693,9 +710,8 @@ void UkmRecorderImpl::LoadExperimentSamplingInfo() { if (key.at(0) == '_') { if (key == "_default_sampling") { int sampling; - // We only load positive global sampling rates. If the global sampling - // is 0, then we stick to the default rate of '1' (unsampled). - if (base::StringToInt(kv.second, &sampling) && sampling > 0) + // We only load non-negative global sampling rates. + if (base::StringToInt(kv.second, &sampling) && sampling >= 0) default_sampling_rate_ = sampling; } continue; @@ -714,8 +730,9 @@ bool UkmRecorderImpl::IsSampledIn(int64_t source_id, int sampling_rate) { // A sampling rate of 0 is "never"; everything else is 1-in-N but calculated // deterministically based on a seed, the source-id, and the event-id. Skip - // the calculation, though, if N==1 because it will always be true. - if (sampling_rate == 0) + // the calculation, though, if N==1 because it will always be true. A negative + // rate means "unset"; treat it like "never". + if (sampling_rate <= 0) return false; if (sampling_rate == 1) return true; @@ -740,7 +757,7 @@ void UkmRecorderImpl::StoreWhitelistedEntries() { base::SPLIT_WANT_NONEMPTY); for (const auto& entry_string : entries) whitelisted_entry_hashes_.insert(base::HashMetricName(entry_string)); - decode_map_ = ::ukm::builders::CreateDecodeMap(); + decode_map_ = builders::CreateDecodeMap(); } } // namespace ukm diff --git a/chromium/components/ukm/ukm_recorder_impl.h b/chromium/components/ukm/ukm_recorder_impl.h index 83f9dabd157..a0db836d635 100644 --- a/chromium/components/ukm/ukm_recorder_impl.h +++ b/chromium/components/ukm/ukm_recorder_impl.h @@ -68,6 +68,9 @@ class UkmRecorderImpl : public UkmRecorder { // Deletes stored recordings. void Purge(); + // Deletes stored recordings related to Chrome extensions. + void PurgeExtensionRecordings(); + // Marks a source as no longer needed to be kept alive in memory. The source // with given id will be removed from in-memory recordings at the next // reporting cycle. @@ -131,6 +134,7 @@ class UkmRecorderImpl : public UkmRecorder { friend ::ukm::UkmRecorderImplTest; friend ::ukm::UkmUtilsForTest; FRIEND_TEST_ALL_PREFIXES(UkmRecorderImplTest, IsSampledIn); + FRIEND_TEST_ALL_PREFIXES(UkmRecorderImplTest, PurgeExtensionRecordings); struct MetricAggregate { uint64_t total_count = 0; @@ -189,7 +193,7 @@ class UkmRecorderImpl : public UkmRecorder { std::set<uint64_t> whitelisted_entry_hashes_; // Sampling configurations, loaded from a field-trial. - int default_sampling_rate_ = 0; + int default_sampling_rate_ = -1; // -1 == not yet loaded base::flat_map<uint64_t, int> event_sampling_rates_; // Contains data from various recordings which periodically get serialized diff --git a/chromium/components/ukm/ukm_recorder_impl_unittest.cc b/chromium/components/ukm/ukm_recorder_impl_unittest.cc index add9f220bb2..cc129a0cb5e 100644 --- a/chromium/components/ukm/ukm_recorder_impl_unittest.cc +++ b/chromium/components/ukm/ukm_recorder_impl_unittest.cc @@ -7,14 +7,18 @@ #include "base/bind.h" #include "base/metrics/ukm_source_id.h" #include "base/test/scoped_feature_list.h" +#include "components/ukm/test_ukm_recorder.h" #include "services/metrics/public/cpp/ukm_builders.h" #include "services/metrics/public/cpp/ukm_entry_builder.h" #include "services/metrics/public/cpp/ukm_source.h" #include "testing/gtest/include/gtest/gtest.h" #include "third_party/metrics_proto/ukm/report.pb.h" +#include "url/gurl.h" namespace ukm { +using TestEvent1 = builders::PageLoad; + TEST(UkmRecorderImplTest, IsSampledIn) { UkmRecorderImpl impl; @@ -56,4 +60,50 @@ TEST(UkmRecorderImplTest, IsSampledIn) { EXPECT_FALSE(impl.IsSampledIn(4, 2, 2)); } +TEST(UkmRecorderImplTest, PurgeExtensionRecordings) { + TestUkmRecorder recorder; + // Enable extension sync. + recorder.SetIsWebstoreExtensionCallback( + base::BindRepeating([](base::StringPiece) { return true; })); + + // Record some sources and events. + SourceId id1 = ConvertToSourceId(1, SourceIdType::NAVIGATION_ID); + recorder.UpdateSourceURL(id1, GURL("https://www.google.ca")); + SourceId id2 = ConvertToSourceId(2, SourceIdType::NAVIGATION_ID); + recorder.UpdateSourceURL(id2, GURL("chrome-extension://abc/manifest.json")); + SourceId id3 = ConvertToSourceId(3, SourceIdType::NAVIGATION_ID); + recorder.UpdateSourceURL(id3, GURL("http://www.wikipedia.org")); + SourceId id4 = ConvertToSourceId(4, SourceIdType::NAVIGATION_ID); + recorder.UpdateSourceURL(id4, GURL("chrome-extension://abc/index.html")); + + TestEvent1(id1).Record(&recorder); + TestEvent1(id2).Record(&recorder); + + // All sources and events have been recorded. + EXPECT_TRUE(recorder.extensions_enabled_); + EXPECT_TRUE(recorder.recording_is_continuous_); + EXPECT_EQ(4U, recorder.sources().size()); + EXPECT_EQ(2U, recorder.entries().size()); + + recorder.PurgeExtensionRecordings(); + + // Recorded sources of extension scheme and related events have been cleared. + EXPECT_EQ(2U, recorder.sources().size()); + EXPECT_EQ(1U, recorder.sources().count(id1)); + EXPECT_EQ(0U, recorder.sources().count(id2)); + EXPECT_EQ(1U, recorder.sources().count(id3)); + EXPECT_EQ(0U, recorder.sources().count(id4)); + + EXPECT_FALSE(recorder.recording_is_continuous_); + EXPECT_EQ(1U, recorder.entries().size()); + EXPECT_EQ(id1, recorder.entries()[0]->source_id); + + // Recording is disabled for extensions, thus new extension URL will not be + // recorded. + recorder.EnableRecording(/* extensions = */ false); + recorder.UpdateSourceURL(id4, GURL("chrome-extension://abc/index.html")); + EXPECT_FALSE(recorder.extensions_enabled_); + EXPECT_EQ(2U, recorder.sources().size()); +} + } // namespace ukm diff --git a/chromium/components/ukm/ukm_reporting_service.cc b/chromium/components/ukm/ukm_reporting_service.cc index c059bd6541f..0b23f3a1421 100644 --- a/chromium/components/ukm/ukm_reporting_service.cc +++ b/chromium/components/ukm/ukm_reporting_service.cc @@ -111,4 +111,4 @@ void UkmReportingService::LogSuccess(size_t log_size) { void UkmReportingService::LogLargeRejection(size_t log_size) {} -} // namespace metrics +} // namespace ukm diff --git a/chromium/components/ukm/ukm_rotation_scheduler.cc b/chromium/components/ukm/ukm_rotation_scheduler.cc index 787ed1abad1..89c8c97078a 100644 --- a/chromium/components/ukm/ukm_rotation_scheduler.cc +++ b/chromium/components/ukm/ukm_rotation_scheduler.cc @@ -9,10 +9,13 @@ namespace ukm { UkmRotationScheduler::UkmRotationScheduler( - const base::Closure& upload_callback, - const base::Callback<base::TimeDelta(void)>& upload_interval_callback) + const base::RepeatingClosure& upload_callback, + bool fast_startup_for_testing, + const base::RepeatingCallback<base::TimeDelta(void)>& + upload_interval_callback) : metrics::MetricsRotationScheduler(upload_callback, - upload_interval_callback) {} + upload_interval_callback, + fast_startup_for_testing) {} UkmRotationScheduler::~UkmRotationScheduler() = default; diff --git a/chromium/components/ukm/ukm_rotation_scheduler.h b/chromium/components/ukm/ukm_rotation_scheduler.h index 5c835277ea8..2e0f99e23af 100644 --- a/chromium/components/ukm/ukm_rotation_scheduler.h +++ b/chromium/components/ukm/ukm_rotation_scheduler.h @@ -17,8 +17,9 @@ class UkmRotationScheduler : public metrics::MetricsRotationScheduler { // callback to call when log rotation should happen and |interval_callback| // to determine the interval between rotations in steady state. UkmRotationScheduler( - const base::Closure& rotation_callback, - const base::Callback<base::TimeDelta(void)>& interval_callback); + const base::RepeatingClosure& rotation_callback, + bool fast_startup_for_testing, + const base::RepeatingCallback<base::TimeDelta(void)>& interval_callback); ~UkmRotationScheduler() override; private: diff --git a/chromium/components/ukm/ukm_service.cc b/chromium/components/ukm/ukm_service.cc index 1d2fc16c73c..561aaaebd73 100644 --- a/chromium/components/ukm/ukm_service.cc +++ b/chromium/components/ukm/ukm_service.cc @@ -6,6 +6,7 @@ #include <memory> #include <string> +#include <unordered_set> #include <utility> #include "base/bind.h" @@ -21,11 +22,13 @@ #include "components/metrics/ukm_demographic_metrics_provider.h" #include "components/prefs/pref_registry_simple.h" #include "components/prefs/pref_service.h" +#include "components/ukm/scheme_constants.h" #include "components/ukm/ukm_pref_names.h" #include "components/ukm/ukm_rotation_scheduler.h" #include "services/metrics/public/cpp/delegating_ukm_recorder.h" #include "third_party/metrics_proto/ukm/report.pb.h" #include "third_party/metrics_proto/user_demographics.pb.h" +#include "third_party/zlib/google/compression_utils.h" namespace ukm { @@ -73,6 +76,94 @@ int32_t LoadAndIncrementSessionId(PrefService* pref_service) { return session_id; } +// Remove elements satisfying the predicate by moving them to the end of the +// list then truncate. +template <typename Predicate, typename ReadElements, typename WriteElements> +void FilterReportElements(Predicate predicate, + const ReadElements& elements, + WriteElements* mutable_elements) { + if (elements.empty()) + return; + + int entries_size = elements.size(); + int start = 0; + int end = entries_size - 1; + while (start < end) { + while (start < entries_size && !predicate(elements.Get(start))) { + start++; + } + while (end >= 0 && predicate(elements.Get(end))) { + end--; + } + if (start < end) { + mutable_elements->SwapElements(start, end); + start++; + end--; + } + } + mutable_elements->DeleteSubrange(start, entries_size - start); +} + +void PurgeExtensionDataFromUnsentLogStore( + metrics::UnsentLogStore* ukm_log_store) { + for (size_t index = 0; index < ukm_log_store->size(); index++) { + // Uncompress log data from store back into a Report. + const std::string& compressed_log_data = + ukm_log_store->GetLogAtIndex(index); + std::string uncompressed_log_data; + const bool uncompress_successful = compression::GzipUncompress( + compressed_log_data, &uncompressed_log_data); + DCHECK(uncompress_successful); + Report report; + + const bool report_parse_successful = + report.ParseFromString(uncompressed_log_data); + DCHECK(report_parse_successful); + + std::unordered_set<SourceId> extension_source_ids; + + // Grab all extension-related source ids. + for (const auto& source : report.sources()) { + // Check if any URL on the source has extension scheme. It is possible + // that only one of multiple URLs does due to redirect, in this case, we + // should still purge the source. + for (const auto& url_info : source.urls()) { + if (GURL(url_info.url()).SchemeIs(kExtensionScheme)) { + extension_source_ids.insert(source.id()); + break; + } + } + } + if (extension_source_ids.empty()) + continue; + + // Remove all extension-related sources from the report. + FilterReportElements( + [&](const Source& element) { + return extension_source_ids.count(element.id()); + }, + report.sources(), report.mutable_sources()); + + // Remove all entries originating from extension-related sources. + FilterReportElements( + [&](const Entry& element) { + return extension_source_ids.count(element.source_id()); + }, + report.entries(), report.mutable_entries()); + + std::string reserialized_log_data; + report.SerializeToString(&reserialized_log_data); + + // Replace the compressed log in the store by its filtered version. + const std::string old_compressed_log_data = + ukm_log_store->ReplaceLogAtIndex(index, reserialized_log_data); + + // Reached here only if extensions were found in the log, so data should now + // be different after filtering. + DCHECK(ukm_log_store->GetLogAtIndex(index) != old_compressed_log_data); + } +} + } // namespace UkmService::UkmService(PrefService* pref_service, @@ -92,16 +183,17 @@ UkmService::UkmService(PrefService* pref_service, reporting_service_.Initialize(); - base::Closure rotate_callback = - base::Bind(&UkmService::RotateLog, self_ptr_factory_.GetWeakPtr()); + base::RepeatingClosure rotate_callback = base::BindRepeating( + &UkmService::RotateLog, self_ptr_factory_.GetWeakPtr()); // MetricsServiceClient outlives UkmService, and // MetricsReportingScheduler is tied to the lifetime of |this|. - const base::Callback<base::TimeDelta(void)>& get_upload_interval_callback = - base::Bind(&metrics::MetricsServiceClient::GetUploadInterval, - base::Unretained(client_)); - scheduler_.reset(new ukm::UkmRotationScheduler(rotate_callback, - get_upload_interval_callback)); - + const base::RepeatingCallback<base::TimeDelta(void)>& + get_upload_interval_callback = + base::BindRepeating(&metrics::MetricsServiceClient::GetUploadInterval, + base::Unretained(client_)); + bool fast_startup_for_testing = client_->ShouldStartUpFastForTesting(); + scheduler_.reset(new UkmRotationScheduler( + rotate_callback, fast_startup_for_testing, get_upload_interval_callback)); StoreWhitelistedEntries(); DelegatingUkmRecorder::Get()->AddDelegate(self_ptr_factory_.GetWeakPtr()); @@ -196,6 +288,16 @@ void UkmService::Purge() { UkmRecorderImpl::Purge(); } +void UkmService::PurgeExtensions() { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + DVLOG(1) << "UkmService::PurgeExtensions"; + // Filter out any extension-related data from the serialized logs in the + // UnsentLogStore for uploading. + PurgeExtensionDataFromUnsentLogStore(reporting_service_.ukm_log_store()); + // Purge data currently in the recordings intended for the next ukm::Report. + UkmRecorderImpl::PurgeExtensionRecordings(); +} + void UkmService::ResetClientState(ResetReason reason) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); diff --git a/chromium/components/ukm/ukm_service.h b/chromium/components/ukm/ukm_service.h index d2a3a7265dc..08fea5f287b 100644 --- a/chromium/components/ukm/ukm_service.h +++ b/chromium/components/ukm/ukm_service.h @@ -48,7 +48,7 @@ class UkmDebugDataExtractor; // These values are persisted to logs. Entries should not be renumbered and // numeric values should never be reused. This maps to the enum UkmResetReason. enum class ResetReason { - kOnSyncPrefsChanged = 0, + kOnUkmAllowedStateChanged = 0, kUpdatePermissions = 1, kMaxValue = kUpdatePermissions, }; @@ -88,6 +88,9 @@ class UkmService : public UkmRecorderImpl { // Deletes any unsent local data. void Purge(); + // Deletes any unsent local data related to Chrome extensions. + void PurgeExtensions(); + // Resets the client prefs (client_id/session_id). |reason| should be passed // to provide the reason of the reset - this is only used for UMA logging. void ResetClientState(ResetReason reason); @@ -121,6 +124,8 @@ class UkmService : public UkmRecorderImpl { TestRegisterUKMProvidersWhenForceMetricsReporting); FRIEND_TEST_ALL_PREFIXES(::IOSChromeMetricsServiceClientTest, TestRegisterUKMProvidersWhenUKMFeatureEnabled); + FRIEND_TEST_ALL_PREFIXES(UkmServiceTest, + PurgeExtensionDataFromUnsentLogStore); // Starts metrics client initialization. void StartInitTask(); diff --git a/chromium/components/ukm/ukm_service_unittest.cc b/chromium/components/ukm/ukm_service_unittest.cc index edea585bca1..c46772215a5 100644 --- a/chromium/components/ukm/ukm_service_unittest.cc +++ b/chromium/components/ukm/ukm_service_unittest.cc @@ -249,6 +249,70 @@ TEST_F(UkmServiceTest, Purge) { EXPECT_EQ(0, GetPersistedLogCount()); } +TEST_F(UkmServiceTest, PurgeExtensionDataFromUnsentLogStore) { + UkmService service(&prefs_, &client_, + true /* restrict_to_whitelisted_entries */, + std::make_unique<MockDemographicMetricsProvider>()); + auto* unsent_log_store = service.reporting_service_.ukm_log_store(); + + // Initialize a Report to be saved to the log store. + ukm::Report report; + report.set_client_id(1); + report.set_session_id(1); + report.set_report_id(1); + + std::string non_extension_url = "https://www.google.ca"; + std::string extension_url = + "chrome-extension://bmnlcjabgnpnenekpadlanbbkooimhnj/manifest.json"; + + // Add both extension- and non-extension-related sources to the Report. + ukm::Source* proto_source_1 = report.add_sources(); + ukm::SourceId source_id_1 = + ukm::ConvertToSourceId(1, ukm::SourceIdType::NAVIGATION_ID); + proto_source_1->set_id(source_id_1); + proto_source_1->add_urls()->set_url(non_extension_url); + ukm::Source* proto_source_2 = report.add_sources(); + ukm::SourceId source_id_2 = + ukm::ConvertToSourceId(2, ukm::SourceIdType::NAVIGATION_ID); + proto_source_2->set_id(source_id_2); + proto_source_2->add_urls()->set_url(extension_url); + + // Add some entries for both sources. + ukm::Entry* entry_1 = report.add_entries(); + entry_1->set_source_id(source_id_2); + ukm::Entry* entry_2 = report.add_entries(); + entry_2->set_source_id(source_id_1); + ukm::Entry* entry_3 = report.add_entries(); + entry_3->set_source_id(source_id_2); + + // Save the Report to the store. + std::string serialized_log; + report.SerializeToString(&serialized_log); + unsent_log_store->StoreLog(serialized_log); + + // Do extension purging. + service.PurgeExtensions(); + + // Get the Report in the log store and verify extension-related data have been + // filtered. + unsent_log_store->StageNextLog(); + const std::string& compressed_log_data = unsent_log_store->staged_log(); + + std::string uncompressed_log_data; + compression::GzipUncompress(compressed_log_data, &uncompressed_log_data); + ukm::Report filtered_report; + filtered_report.ParseFromString(uncompressed_log_data); + + // Only proto_source_1 with non-extension URL is kept. + EXPECT_EQ(1, filtered_report.sources_size()); + EXPECT_EQ(source_id_1, filtered_report.sources(0).id()); + EXPECT_EQ(non_extension_url, filtered_report.sources(0).urls(0).url()); + + // Only entry_2 from the non-extension source is kept. + EXPECT_EQ(1, filtered_report.entries_size()); + EXPECT_EQ(source_id_1, filtered_report.entries(0).source_id()); +} + TEST_F(UkmServiceTest, SourceSerialization) { UkmService service(&prefs_, &client_, true /* restrict_to_whitelisted_entries */, |