From 512e75d251000d6394da35c5ef8cc74d6d6b35e5 Mon Sep 17 00:00:00 2001 From: Joel Martinez Date: Thu, 22 Jun 2023 14:36:43 -0600 Subject: [PATCH] Fix threading-related crash in Cobalt telemetry. (#689) The Chromium metrics libraries require any calls to their public APIs to be invoked on the same thread in which they were created. Similarly, H5vcc JS callback need to be invoked on the V8 thread. This was being violated as the H5vcc callbacks were being invoked by a non-V8 thread and H5vccMetrics was trying to call metrics APIs from a V8 thread. The fix is to store the task runners for the CobaltMetricsServicesManager and H5vccMetrics instances and use those to run any tasks that must be run in the targeted thread. This approach required a slight refactoring to allow the top level CobaltMetricsServicesManager to provide its own public API to interact with the underlying metrics client on a targeted thread. I also found this approach allowed me to just pass around a RepeatingCallback for the upload handler and remove the abstract class that was being used before (CobaltH5vccMetricsUploaderCallback). Added demo to verify everything is working. b/288252273 Change-Id: I8243b27a1853c65ac9d4fe11adc95a7f4da9ed51 (cherry picked from commit a89e9fb307daf68fdae75018b2a4e434b45a4b50) --- cobalt/browser/application.cc | 3 +- cobalt/browser/application.h | 4 +- cobalt/browser/metrics/BUILD.gn | 4 +- .../cobalt_h5vcc_metrics_uploader_callback.cc | 28 ------ .../cobalt_h5vcc_metrics_uploader_callback.h | 48 --------- .../metrics/cobalt_metrics_log_uploader.cc | 5 +- .../metrics/cobalt_metrics_log_uploader.h | 5 +- .../cobalt_metrics_log_uploader_test.cc | 19 ++-- .../metrics/cobalt_metrics_service_client.cc | 2 +- .../metrics/cobalt_metrics_service_client.h | 5 +- .../cobalt_metrics_services_manager.cc | 98 +++++++++++++++++++ .../metrics/cobalt_metrics_services_manager.h | 60 +++++++++--- .../cobalt_metrics_uploader_callback.h | 14 +-- cobalt/demos/content/telemetry/index.html | 63 ++++++++++++ cobalt/h5vcc/h5vcc_metrics.cc | 59 ++++++----- cobalt/h5vcc/h5vcc_metrics.h | 24 ++++- 16 files changed, 285 insertions(+), 156 deletions(-) delete mode 100644 cobalt/browser/metrics/cobalt_h5vcc_metrics_uploader_callback.cc delete mode 100644 cobalt/browser/metrics/cobalt_h5vcc_metrics_uploader_callback.h create mode 100644 cobalt/browser/metrics/cobalt_metrics_services_manager.cc create mode 100644 cobalt/demos/content/telemetry/index.html diff --git a/cobalt/browser/application.cc b/cobalt/browser/application.cc index a775896947c9..dab5cf5e97a8 100644 --- a/cobalt/browser/application.cc +++ b/cobalt/browser/application.cc @@ -1061,8 +1061,7 @@ Application::~Application() { // Explicitly delete the global metrics services manager here to give it // an opportunity to clean up late logs and persist metrics. - metrics::CobaltMetricsServicesManager::DeleteInstance( - metrics_services_manager_); + metrics::CobaltMetricsServicesManager::DeleteInstance(); #if defined(ENABLE_DEBUGGER) && defined(STARBOARD_ALLOWS_MEMORY_TRACKING) memory_tracker_tool_.reset(NULL); diff --git a/cobalt/browser/application.h b/cobalt/browser/application.h index 23b9088d4a0b..6a25f0c10f07 100644 --- a/cobalt/browser/application.h +++ b/cobalt/browser/application.h @@ -27,10 +27,10 @@ #include "cobalt/base/event_dispatcher.h" #include "cobalt/browser/browser_module.h" #include "cobalt/browser/memory_tracker/tool.h" +#include "cobalt/browser/metrics/cobalt_metrics_services_manager.h" #include "cobalt/network/network_module.h" #include "cobalt/persistent_storage/persistent_settings.h" #include "cobalt/system_window/system_window.h" -#include "components/metrics_services_manager/metrics_services_manager.h" #include "starboard/time.h" #if SB_IS(EVERGREEN) #include "cobalt/updater/updater_module.h" @@ -225,7 +225,7 @@ class Application { void DispatchDeepLink(const char* link, SbTimeMonotonic timestamp); void DispatchDeepLinkIfNotConsumed(); - metrics_services_manager::MetricsServicesManager* metrics_services_manager_; + metrics::CobaltMetricsServicesManager* metrics_services_manager_; DISALLOW_COPY_AND_ASSIGN(Application); }; diff --git a/cobalt/browser/metrics/BUILD.gn b/cobalt/browser/metrics/BUILD.gn index dc7c577f3167..3f25f1a9fa85 100644 --- a/cobalt/browser/metrics/BUILD.gn +++ b/cobalt/browser/metrics/BUILD.gn @@ -16,12 +16,12 @@ static_library("metrics") { sources = [ "cobalt_enabled_state_provider.cc", "cobalt_enabled_state_provider.h", - "cobalt_h5vcc_metrics_uploader_callback.cc", - "cobalt_h5vcc_metrics_uploader_callback.h", "cobalt_metrics_log_uploader.cc", "cobalt_metrics_log_uploader.h", "cobalt_metrics_service_client.cc", "cobalt_metrics_service_client.h", + "cobalt_metrics_services_manager.cc", + "cobalt_metrics_services_manager.h", "cobalt_metrics_services_manager_client.cc", "cobalt_metrics_services_manager_client.h", "cobalt_metrics_uploader_callback.h", diff --git a/cobalt/browser/metrics/cobalt_h5vcc_metrics_uploader_callback.cc b/cobalt/browser/metrics/cobalt_h5vcc_metrics_uploader_callback.cc deleted file mode 100644 index b7779e757950..000000000000 --- a/cobalt/browser/metrics/cobalt_h5vcc_metrics_uploader_callback.cc +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright 2023 The Cobalt Authors. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "cobalt/browser/metrics/cobalt_h5vcc_metrics_uploader_callback.h" - -namespace cobalt { -namespace browser { -namespace metrics { - -void CobaltH5vccMetricsUploaderCallback::Run( - const cobalt::h5vcc::H5vccMetricType& type, const std::string& payload) { - event_handler_->callback.value().Run(type, payload); -} - -} // namespace metrics -} // namespace browser -} // namespace cobalt diff --git a/cobalt/browser/metrics/cobalt_h5vcc_metrics_uploader_callback.h b/cobalt/browser/metrics/cobalt_h5vcc_metrics_uploader_callback.h deleted file mode 100644 index 87df7fa04153..000000000000 --- a/cobalt/browser/metrics/cobalt_h5vcc_metrics_uploader_callback.h +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright 2023 The Cobalt Authors. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef COBALT_BROWSER_METRICS_COBALT_H5VCC_METRICS_UPLOADER_CALLBACK_H_ -#define COBALT_BROWSER_METRICS_COBALT_H5VCC_METRICS_UPLOADER_CALLBACK_H_ - -#include - -#include "cobalt/browser/metrics/cobalt_metrics_uploader_callback.h" -#include "cobalt/h5vcc/h5vcc_metric_type.h" -#include "cobalt/h5vcc/metric_event_handler_wrapper.h" - -namespace cobalt { -namespace browser { -namespace metrics { - -// An implementation of CobaltMetricsUploaderCallback to support binding an -// H5vcc (JS) callback to be called with a metric payload. -class CobaltH5vccMetricsUploaderCallback - : public CobaltMetricsUploaderCallback { - public: - CobaltH5vccMetricsUploaderCallback( - h5vcc::MetricEventHandlerWrapper* event_handler) - : event_handler_(event_handler) {} - - // Runs the h5vcc event_handler callback for the given type and payload. - void Run(const cobalt::h5vcc::H5vccMetricType& type, - const std::string& payload) override; - - private: - h5vcc::MetricEventHandlerWrapper* event_handler_; -}; -} // namespace metrics -} // namespace browser -} // namespace cobalt - -#endif // COBALT_BROWSER_METRICS_COBALT_H5VCC_METRICS_UPLOADER_CALLBACK_H_ diff --git a/cobalt/browser/metrics/cobalt_metrics_log_uploader.cc b/cobalt/browser/metrics/cobalt_metrics_log_uploader.cc index 523558b04faf..2076f6bd19dd 100644 --- a/cobalt/browser/metrics/cobalt_metrics_log_uploader.cc +++ b/cobalt/browser/metrics/cobalt_metrics_log_uploader.cc @@ -14,13 +14,14 @@ #include "cobalt/browser/metrics/cobalt_metrics_log_uploader.h" +#include "base/logging.h" #include "cobalt/browser/metrics/cobalt_metrics_uploader_callback.h" #include "cobalt/h5vcc/h5vcc_metric_type.h" #include "components/metrics/log_decoder.h" #include "components/metrics/metrics_log_uploader.h" +#include "third_party/metrics_proto/chrome_user_metrics_extension.pb.h" #include "third_party/metrics_proto/reporting_info.pb.h" - namespace cobalt { namespace browser { namespace metrics { @@ -51,7 +52,7 @@ void CobaltMetricsLogUploader::UploadLog( } void CobaltMetricsLogUploader::SetOnUploadHandler( - CobaltMetricsUploaderCallback* upload_handler) { + const CobaltMetricsUploaderCallback* upload_handler) { upload_handler_ = upload_handler; } diff --git a/cobalt/browser/metrics/cobalt_metrics_log_uploader.h b/cobalt/browser/metrics/cobalt_metrics_log_uploader.h index 298e80e6b8eb..fa71662ab5e6 100644 --- a/cobalt/browser/metrics/cobalt_metrics_log_uploader.h +++ b/cobalt/browser/metrics/cobalt_metrics_log_uploader.h @@ -52,12 +52,13 @@ class CobaltMetricsLogUploader : public ::metrics::MetricsLogUploader { // Sets the event handler wrapper to be called when metrics are ready for // upload. This should be the JavaScript H5vcc callback implementation. - void SetOnUploadHandler(CobaltMetricsUploaderCallback* metric_event_handler); + void SetOnUploadHandler( + const CobaltMetricsUploaderCallback* metric_event_handler); private: const ::metrics::MetricsLogUploader::MetricServiceType service_type_; const ::metrics::MetricsLogUploader::UploadCallback on_upload_complete_; - CobaltMetricsUploaderCallback* upload_handler_ = nullptr; + const CobaltMetricsUploaderCallback* upload_handler_ = nullptr; }; } // namespace metrics diff --git a/cobalt/browser/metrics/cobalt_metrics_log_uploader_test.cc b/cobalt/browser/metrics/cobalt_metrics_log_uploader_test.cc index 8baaa8e85b22..21f27709b4b8 100644 --- a/cobalt/browser/metrics/cobalt_metrics_log_uploader_test.cc +++ b/cobalt/browser/metrics/cobalt_metrics_log_uploader_test.cc @@ -16,6 +16,7 @@ #include +#include "base/test/mock_callback.h" #include "cobalt/browser/metrics/cobalt_metrics_uploader_callback.h" #include "cobalt/h5vcc/h5vcc_metrics.h" #include "testing/gmock/include/gmock/gmock.h" @@ -24,7 +25,6 @@ #include "third_party/metrics_proto/reporting_info.pb.h" #include "third_party/zlib/google/compression_utils.h" - namespace cobalt { namespace browser { namespace metrics { @@ -37,6 +37,7 @@ using ::testing::Eq; using ::testing::StrEq; using ::testing::StrictMock; + class CobaltMetricsLogUploaderTest : public ::testing::Test { public: void SetUp() override { @@ -58,15 +59,10 @@ class CobaltMetricsLogUploaderTest : public ::testing::Test { int callback_count_ = 0; }; -class MockMetricsUploaderCallback : public CobaltMetricsUploaderCallback { - public: - MOCK_METHOD2(Run, void(const cobalt::h5vcc::H5vccMetricType& type, - const std::string& payload)); -}; - TEST_F(CobaltMetricsLogUploaderTest, TriggersUploadHandler) { - StrictMock mock_upload_handler; - uploader_->SetOnUploadHandler(&mock_upload_handler); + base::MockCallback mock_upload_handler; + const auto cb = mock_upload_handler.Get(); + uploader_->SetOnUploadHandler(&cb); ::metrics::ReportingInfo dummy_reporting_info; ::metrics::ChromeUserMetricsExtension uma_log; uma_log.set_session_id(1234); @@ -98,8 +94,9 @@ TEST_F(CobaltMetricsLogUploaderTest, UnknownMetricTypeDoesntTriggerUpload) { ::metrics::MetricsLogUploader::MetricServiceType::UKM, base::Bind(&CobaltMetricsLogUploaderTest::UploadCompleteCallback, base::Unretained(this)))); - StrictMock mock_upload_handler; - uploader_->SetOnUploadHandler(&mock_upload_handler); + base::MockCallback mock_upload_handler; + const auto cb = mock_upload_handler.Get(); + uploader_->SetOnUploadHandler(&cb); ::metrics::ReportingInfo dummy_reporting_info; ::metrics::ChromeUserMetricsExtension uma_log; uma_log.set_session_id(1234); diff --git a/cobalt/browser/metrics/cobalt_metrics_service_client.cc b/cobalt/browser/metrics/cobalt_metrics_service_client.cc index f81987f08e63..e60a0306542e 100644 --- a/cobalt/browser/metrics/cobalt_metrics_service_client.cc +++ b/cobalt/browser/metrics/cobalt_metrics_service_client.cc @@ -51,7 +51,7 @@ namespace metrics { const int kStandardUploadIntervalSeconds = 5 * 60; // 5 minutes. void CobaltMetricsServiceClient::SetOnUploadHandler( - CobaltMetricsUploaderCallback* uploader_callback) { + const CobaltMetricsUploaderCallback* uploader_callback) { upload_handler_ = uploader_callback; if (log_uploader_) { log_uploader_->SetOnUploadHandler(upload_handler_); diff --git a/cobalt/browser/metrics/cobalt_metrics_service_client.h b/cobalt/browser/metrics/cobalt_metrics_service_client.h index c0505e605683..c7860f0c57b5 100644 --- a/cobalt/browser/metrics/cobalt_metrics_service_client.h +++ b/cobalt/browser/metrics/cobalt_metrics_service_client.h @@ -50,7 +50,8 @@ class CobaltMetricsServiceClient : public ::metrics::MetricsServiceClient { // Sets the uploader handler to be called when metrics are ready for // upload. - void SetOnUploadHandler(CobaltMetricsUploaderCallback* uploader_callback); + void SetOnUploadHandler( + const CobaltMetricsUploaderCallback* uploader_callback); // Returns the MetricsService instance that this client is associated with. // With the exception of testing contexts, the returned instance must be valid @@ -164,7 +165,7 @@ class CobaltMetricsServiceClient : public ::metrics::MetricsServiceClient { CobaltMetricsLogUploader* log_uploader_ = nullptr; - CobaltMetricsUploaderCallback* upload_handler_ = nullptr; + const CobaltMetricsUploaderCallback* upload_handler_ = nullptr; uint32_t custom_upload_interval_ = UINT32_MAX; diff --git a/cobalt/browser/metrics/cobalt_metrics_services_manager.cc b/cobalt/browser/metrics/cobalt_metrics_services_manager.cc new file mode 100644 index 000000000000..3874d08b6eca --- /dev/null +++ b/cobalt/browser/metrics/cobalt_metrics_services_manager.cc @@ -0,0 +1,98 @@ +// Copyright 2023 The Cobalt Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "cobalt/browser/metrics/cobalt_metrics_services_manager.h" + +#include + +#include "cobalt/browser/metrics/cobalt_metrics_service_client.h" +#include "cobalt/browser/metrics/cobalt_metrics_services_manager_client.h" +#include "components/metrics_services_manager/metrics_services_manager.h" + +namespace cobalt { +namespace browser { +namespace metrics { + +CobaltMetricsServicesManager* CobaltMetricsServicesManager::instance_ = nullptr; + +CobaltMetricsServicesManager::CobaltMetricsServicesManager() + : task_runner_(base::ThreadTaskRunnerHandle::Get()), + metrics_services_manager::MetricsServicesManager( + std::make_unique()) {} + + +// Static Singleton getter for metrics services manager. +CobaltMetricsServicesManager* CobaltMetricsServicesManager::GetInstance() { + if (instance_ == nullptr) { + instance_ = new CobaltMetricsServicesManager(); + } + return instance_; +} + +void CobaltMetricsServicesManager::DeleteInstance() { delete instance_; } + +void CobaltMetricsServicesManager::SetOnUploadHandler( + const CobaltMetricsUploaderCallback* uploader_callback) { + instance_->task_runner_->PostTask( + FROM_HERE, + base::Bind(&CobaltMetricsServicesManager::SetOnUploadHandlerInternal, + base::Unretained(instance_), uploader_callback)); +} + +void CobaltMetricsServicesManager::SetOnUploadHandlerInternal( + const CobaltMetricsUploaderCallback* uploader_callback) { + CobaltMetricsServiceClient* client = + static_cast(GetMetricsServiceClient()); + DCHECK(client); + client->SetOnUploadHandler(uploader_callback); +} + +void CobaltMetricsServicesManager::ToggleMetricsEnabled(bool is_enabled) { + instance_->task_runner_->PostTask( + FROM_HERE, + base::Bind(&CobaltMetricsServicesManager::ToggleMetricsEnabledInternal, + base::Unretained(instance_), is_enabled)); +} +void CobaltMetricsServicesManager::ToggleMetricsEnabledInternal( + bool is_enabled) { + CobaltMetricsServicesManagerClient* client = + static_cast( + GetMetricsServicesManagerClient()); + DCHECK(client); + client->GetEnabledStateProvider()->SetConsentGiven(is_enabled); + client->GetEnabledStateProvider()->SetReportingEnabled(is_enabled); + UpdateUploadPermissions(is_enabled); +} + +void CobaltMetricsServicesManager::SetUploadInterval( + uint32_t interval_seconds) { + instance_->task_runner_->PostTask( + FROM_HERE, + base::Bind(&CobaltMetricsServicesManager::SetUploadIntervalInternal, + base::Unretained(instance_), interval_seconds)); +} + +void CobaltMetricsServicesManager::SetUploadIntervalInternal( + uint32_t interval_seconds) { + browser::metrics::CobaltMetricsServiceClient* client = + static_cast( + GetMetricsServiceClient()); + DCHECK(client); + client->SetUploadInterval(interval_seconds); +} + + +} // namespace metrics +} // namespace browser +} // namespace cobalt diff --git a/cobalt/browser/metrics/cobalt_metrics_services_manager.h b/cobalt/browser/metrics/cobalt_metrics_services_manager.h index 13d1b918559c..056b5397390e 100644 --- a/cobalt/browser/metrics/cobalt_metrics_services_manager.h +++ b/cobalt/browser/metrics/cobalt_metrics_services_manager.h @@ -18,32 +18,62 @@ #include +#include "base//memory/scoped_refptr.h" +#include "base/single_thread_task_runner.h" #include "cobalt/browser/metrics/cobalt_metrics_services_manager_client.h" +#include "cobalt/browser/metrics/cobalt_metrics_uploader_callback.h" #include "components/metrics_services_manager/metrics_services_manager.h" +#include "components/metrics_services_manager/metrics_services_manager_client.h" + namespace cobalt { namespace browser { namespace metrics { -// A static wrapper around Chromium's MetricsServicesManager. We need a way +// A static wrapper around CobaltMetricsServicesManager. We need a way // to provide a static instance of the "Cobaltified" MetricsServicesManager (and // its public APIs) to control metrics behavior outside of //cobalt/browser -// (e.g., via H5vcc). -class CobaltMetricsServicesManager { +// (e.g., via H5vcc). Note, it's important that all public methods execute +// on the same thread in which CobaltMetricsServicesManager was constructed. +// This is a requirement of the metrics client code. +class CobaltMetricsServicesManager + : public metrics_services_manager::MetricsServicesManager { public: + CobaltMetricsServicesManager(); + // Static Singleton getter for metrics services manager. - static metrics_services_manager::MetricsServicesManager* GetInstance() { - static const auto instance = - new metrics_services_manager::MetricsServicesManager( - std::make_unique()); - return instance; - } - - // Destroy passed metrics service manager. - static void DeleteInstance(metrics_services_manager::MetricsServicesManager* - metrics_services_manager) { - delete metrics_services_manager; - } + static CobaltMetricsServicesManager* GetInstance(); + + // Destructs the static instance of CobaltMetricsServicesManager. + static void DeleteInstance(); + + // Sets the upload handler onto the current static instance of + // CobaltMetricsServicesManager. + static void SetOnUploadHandler( + const CobaltMetricsUploaderCallback* uploader_callback); + + // Toggles whether metric reporting is enabled via + // CobaltMetricsServicesManager. + static void ToggleMetricsEnabled(bool is_enabled); + + // Sets the upload interval for metrics reporting. That is, how often are + // metrics snapshotted and attempted to upload. + static void SetUploadInterval(uint32_t interval_seconds); + + private: + void SetOnUploadHandlerInternal( + const CobaltMetricsUploaderCallback* uploader_callback); + + void ToggleMetricsEnabledInternal(bool is_enabled); + + void SetUploadIntervalInternal(uint32_t interval_seconds); + + static CobaltMetricsServicesManager* instance_; + + // The task runner of the thread this class was constructed on. All logic + // interacting with containing metrics classes must be invoked on this + // task_runner thread. + scoped_refptr const task_runner_; }; } // namespace metrics diff --git a/cobalt/browser/metrics/cobalt_metrics_uploader_callback.h b/cobalt/browser/metrics/cobalt_metrics_uploader_callback.h index 7d302e394ee6..db980c5fa28a 100644 --- a/cobalt/browser/metrics/cobalt_metrics_uploader_callback.h +++ b/cobalt/browser/metrics/cobalt_metrics_uploader_callback.h @@ -17,21 +17,17 @@ #include +#include "base/callback.h" #include "cobalt/h5vcc/h5vcc_metric_type.h" namespace cobalt { namespace browser { namespace metrics { -// An abstract class representing the callback to be invoked when the Cobalt -// metrics payload is ready to be uploaded. -class CobaltMetricsUploaderCallback { - public: - virtual ~CobaltMetricsUploaderCallback() = default; - - virtual void Run(const cobalt::h5vcc::H5vccMetricType&, - const std::string&) = 0; -}; +typedef base::RepeatingCallback + CobaltMetricsUploaderCallback; } // namespace metrics } // namespace browser diff --git a/cobalt/demos/content/telemetry/index.html b/cobalt/demos/content/telemetry/index.html new file mode 100644 index 000000000000..4213e03b32be --- /dev/null +++ b/cobalt/demos/content/telemetry/index.html @@ -0,0 +1,63 @@ + + + + + Cobalt Telemetry Demo + + + + + +

LOG OUTPUT:

+
+ + + + + diff --git a/cobalt/h5vcc/h5vcc_metrics.cc b/cobalt/h5vcc/h5vcc_metrics.cc index 14e7e079cfc4..b37888b75b7d 100644 --- a/cobalt/h5vcc/h5vcc_metrics.cc +++ b/cobalt/h5vcc/h5vcc_metrics.cc @@ -16,7 +16,6 @@ #include -#include "cobalt/browser/metrics/cobalt_h5vcc_metrics_uploader_callback.h" #include "cobalt/browser/metrics/cobalt_metrics_service_client.h" #include "cobalt/browser/metrics/cobalt_metrics_services_manager.h" #include "cobalt/h5vcc/h5vcc_metric_type.h" @@ -27,43 +26,49 @@ namespace h5vcc { void H5vccMetrics::OnMetricEvent( const h5vcc::MetricEventHandlerWrapper::ScriptValue& event_handler) { - auto callback = - new cobalt::browser::metrics::CobaltH5vccMetricsUploaderCallback( - new h5vcc::MetricEventHandlerWrapper(this, event_handler)); - uploader_callback_.reset(callback); - browser::metrics::CobaltMetricsServiceClient* client = - static_cast( - browser::metrics::CobaltMetricsServicesManager::GetInstance() - ->GetMetricsServiceClient()); - DCHECK(client); - client->SetOnUploadHandler(uploader_callback_.get()); + if (!uploader_callback_) { + run_event_handler_callback_ = std::make_unique< + cobalt::browser::metrics::CobaltMetricsUploaderCallback>( + base::BindRepeating(&H5vccMetrics::RunEventHandler, + base::Unretained(this))); + browser::metrics::CobaltMetricsServicesManager::GetInstance() + ->SetOnUploadHandler(run_event_handler_callback_.get()); + } + + uploader_callback_ = + new h5vcc::MetricEventHandlerWrapper(this, event_handler); +} + +void H5vccMetrics::RunEventHandler( + const cobalt::h5vcc::H5vccMetricType& metric_type, + const std::string& serialized_proto) { + task_runner_->PostTask( + FROM_HERE, + base::Bind(&H5vccMetrics::RunEventHandlerInternal, base::Unretained(this), + metric_type, serialized_proto)); } + +void H5vccMetrics::RunEventHandlerInternal( + const cobalt::h5vcc::H5vccMetricType& metric_type, + const std::string& serialized_proto) { + uploader_callback_->callback.value().Run(metric_type, serialized_proto); +} + void H5vccMetrics::Enable() { ToggleMetricsEnabled(true); } void H5vccMetrics::Disable() { ToggleMetricsEnabled(false); } -void H5vccMetrics::ToggleMetricsEnabled(bool isEnabled) { - browser::metrics::CobaltMetricsServicesManagerClient* client = - static_cast( - browser::metrics::CobaltMetricsServicesManager::GetInstance() - ->GetMetricsServicesManagerClient()); - DCHECK(client); - is_enabled_ = isEnabled; - client->GetEnabledStateProvider()->SetConsentGiven(isEnabled); - client->GetEnabledStateProvider()->SetReportingEnabled(isEnabled); +void H5vccMetrics::ToggleMetricsEnabled(bool is_enabled) { + is_enabled_ = is_enabled; browser::metrics::CobaltMetricsServicesManager::GetInstance() - ->UpdateUploadPermissions(isEnabled); + ->ToggleMetricsEnabled(is_enabled); } bool H5vccMetrics::IsEnabled() { return is_enabled_; } void H5vccMetrics::SetMetricEventInterval(uint32_t interval_seconds) { - browser::metrics::CobaltMetricsServiceClient* client = - static_cast( - browser::metrics::CobaltMetricsServicesManager::GetInstance() - ->GetMetricsServiceClient()); - DCHECK(client); - client->SetUploadInterval(interval_seconds); + browser::metrics::CobaltMetricsServicesManager::GetInstance() + ->SetUploadInterval(interval_seconds); } } // namespace h5vcc diff --git a/cobalt/h5vcc/h5vcc_metrics.h b/cobalt/h5vcc/h5vcc_metrics.h index 24b92de62ca9..675e7e9cecb2 100644 --- a/cobalt/h5vcc/h5vcc_metrics.h +++ b/cobalt/h5vcc/h5vcc_metrics.h @@ -18,7 +18,9 @@ #include #include -#include "cobalt/browser/metrics/cobalt_h5vcc_metrics_uploader_callback.h" +#include "base/single_thread_task_runner.h" +#include "base/threading/thread_task_runner_handle.h" +#include "cobalt/browser/metrics/cobalt_metrics_uploader_callback.h" #include "cobalt/h5vcc/h5vcc_metric_type.h" #include "cobalt/h5vcc/metric_event_handler_wrapper.h" #include "cobalt/script/callback_function.h" @@ -39,7 +41,7 @@ class H5vccMetrics : public script::Wrappable { // its entirety. typedef MetricEventHandler H5vccMetricEventHandler; - H5vccMetrics() {} + H5vccMetrics() : task_runner_(base::ThreadTaskRunnerHandle::Get()) {} H5vccMetrics(const H5vccMetrics&) = delete; H5vccMetrics& operator=(const H5vccMetrics&) = delete; @@ -66,11 +68,23 @@ class H5vccMetrics : public script::Wrappable { private: // Internal convenience method for toggling enabled/disabled state. - void ToggleMetricsEnabled(bool isEnabled); + void ToggleMetricsEnabled(bool is_enabled); + + void RunEventHandler(const cobalt::h5vcc::H5vccMetricType& metric_type, + const std::string& serialized_proto); + + void RunEventHandlerInternal( + const cobalt::h5vcc::H5vccMetricType& metric_type, + const std::string& serialized_proto); + + h5vcc::MetricEventHandlerWrapper* uploader_callback_ = nullptr; + + std::unique_ptr + run_event_handler_callback_; - std::unique_ptr - uploader_callback_; bool is_enabled_ = false; + + scoped_refptr const task_runner_; }; } // namespace h5vcc