[DTC] Add CS Signals to inline flow for Mac

Added a call to SignalsAggregator for the kAgent signal in the
existing BrowserSignalsDecorator. That will effectively add the CS
agent signal on Windows and Mac, and allows the removal of the
WinSignalsDecorator (which only collected that value).

Bug: b:266847010
Change-Id: I825f455e13e244f69ff954bacda524fea9113796
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4213403
Reviewed-by: Hamda Mare <hmare@google.com>
Commit-Queue: Sebastien Lalancette <seblalancette@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1100625}

chrome/browser/BUILD.gn

@@ -6126,11 +6126,7 @@ static_library("browser") {
         [ "//chrome/browser/enterprise/connectors/analysis:sdk_manager" ]
 
     if (is_win) {
-      deps += [
-        "//chrome/browser/enterprise/connectors/device_trust/signals/decorators/browser/win",
-        "//components/device_signals/core/browser/win",
-        "//components/device_signals/core/common/win",
-      ]
+      deps += [ "//components/device_signals/core/browser/win" ]
     }
 
     if (is_mac) {

chrome/browser/enterprise/connectors/device_trust/device_trust_service_factory.cc

@@ -24,6 +24,7 @@
 #if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC)
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/enterprise/connectors/device_trust/attestation/desktop/desktop_attestation_service.h"
+#include "chrome/browser/enterprise/signals/signals_aggregator_factory.h"
 #include "chrome/browser/policy/chrome_browser_policy_connector.h"
 #include "components/enterprise/browser/controller/browser_dm_token_storage.h"
 #include "components/enterprise/browser/controller/chrome_browser_cloud_management_controller.h"
@@ -73,6 +74,11 @@ DeviceTrustServiceFactory::DeviceTrustServiceFactory()
           ProfileSelections::BuildForRegularAndIncognitoNonExperimental()) {
   DependsOn(DeviceTrustConnectorServiceFactory::GetInstance());
   DependsOn(policy::ManagementServiceFactory::GetInstance());
+
+#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
+  // Depends on this service via the SignalsService having a dependency on it.
+  DependsOn(enterprise_signals::SignalsAggregatorFactory::GetInstance());
+#endif  // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
 }
 
 bool DeviceTrustServiceFactory::ServiceIsNULLWhileTesting() const {

chrome/browser/enterprise/connectors/device_trust/signals/decorators/browser/BUILD.gn

@@ -9,6 +9,8 @@ source_set("unit_tests") {
   deps = [
     "//chrome/browser",
     "//chrome/browser/enterprise/connectors/device_trust/attestation/common/proto:attestation_ca_proto",
+    "//components/device_signals/core/browser",
+    "//components/device_signals/core/browser:test_support",
     "//components/device_signals/core/common",
     "//components/enterprise:test_support",
     "//components/policy/core/common:test_support",

chrome/browser/enterprise/connectors/device_trust/signals/decorators/browser/browser_signals_decorator.cc

@@ -7,56 +7,73 @@
 #include <functional>
 #include <utility>
 
+#include "base/barrier_closure.h"
 #include "base/check.h"
 #include "base/task/thread_pool.h"
 #include "base/values.h"
 #include "chrome/browser/enterprise/connectors/device_trust/signals/decorators/common/metrics_utils.h"
 #include "chrome/browser/enterprise/connectors/device_trust/signals/decorators/common/signals_utils.h"
 #include "chrome/browser/enterprise/signals/device_info_fetcher.h"
 #include "chrome/browser/enterprise/signals/signals_common.h"
+#include "components/device_signals/core/browser/signals_aggregator.h"
+#include "components/device_signals/core/browser/signals_types.h"
+#include "components/device_signals/core/common/common_types.h"
 #include "components/device_signals/core/common/signals_constants.h"
 #include "components/policy/core/common/cloud/cloud_policy_store.h"
 #include "components/policy/proto/device_management_backend.pb.h"
 
 namespace enterprise_connectors {
 
 namespace {
-using policy::CloudPolicyStore;
-
 constexpr char kLatencyHistogramVariant[] = "Browser";
 }  // namespace
 
 BrowserSignalsDecorator::BrowserSignalsDecorator(
-    CloudPolicyStore* cloud_policy_store)
-    : cloud_policy_store_(cloud_policy_store) {
-  DCHECK(cloud_policy_store_);
-}
+    policy::CloudPolicyStore* cloud_policy_store,
+    device_signals::SignalsAggregator* signals_aggregator)
+    : cloud_policy_store_(cloud_policy_store),
+      signals_aggregator_(signals_aggregator) {}
 
 BrowserSignalsDecorator::~BrowserSignalsDecorator() = default;
 
 void BrowserSignalsDecorator::Decorate(base::Value::Dict& signals,
                                        base::OnceClosure done_closure) {
   auto start_time = base::TimeTicks::Now();
 
-  if (cloud_policy_store_->has_policy()) {
+  if (cloud_policy_store_ && cloud_policy_store_->has_policy()) {
     const auto* policy = cloud_policy_store_->policy();
     signals.Set(device_signals::names::kDeviceEnrollmentDomain,
                 policy->has_managed_by() ? policy->managed_by()
                                          : policy->display_domain());
   }
 
+  auto barrier_closure = base::BarrierClosure(
+      /*num_closures=*/signals_aggregator_ ? 2 : 1,
+      base::BindOnce(&BrowserSignalsDecorator::OnAllSignalsReceived,
+                     weak_ptr_factory_.GetWeakPtr(), start_time,
+                     std::move(done_closure)));
+
   base::ThreadPool::PostTaskAndReplyWithResult(
       FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_BLOCKING},
       base::BindOnce(&enterprise_signals::DeviceInfoFetcher::Fetch,
                      enterprise_signals::DeviceInfoFetcher::CreateInstance()),
       base::BindOnce(&BrowserSignalsDecorator::OnDeviceInfoFetched,
                      weak_ptr_factory_.GetWeakPtr(), std::ref(signals),
-                     start_time, std::move(done_closure)));
+                     barrier_closure));
+
+  if (signals_aggregator_) {
+    device_signals::SignalsAggregationRequest request;
+    request.signal_names.emplace(device_signals::SignalName::kAgent);
+    signals_aggregator_->GetSignals(
+        request,
+        base::BindOnce(&BrowserSignalsDecorator::OnAggregatedSignalsReceived,
+                       weak_ptr_factory_.GetWeakPtr(), std::ref(signals),
+                       barrier_closure));
+  }
 }
 
 void BrowserSignalsDecorator::OnDeviceInfoFetched(
     base::Value::Dict& signals,
-    base::TimeTicks start_time,
     base::OnceClosure done_closure,
     const enterprise_signals::DeviceInfo& device_info) {
   signals.Set(device_signals::names::kSerialNumber, device_info.serial_number);
@@ -84,8 +101,30 @@ void BrowserSignalsDecorator::OnDeviceInfoFetched(
                 static_cast<int32_t>(device_info.secure_boot_enabled.value()));
   }
 
-  LogSignalsCollectionLatency(kLatencyHistogramVariant, start_time);
+  std::move(done_closure).Run();
+}
 
+void BrowserSignalsDecorator::OnAggregatedSignalsReceived(
+    base::Value::Dict& signals,
+    base::OnceClosure done_closure,
+    device_signals::SignalsAggregationResponse response) {
+  if (response.agent_signals_response &&
+      response.agent_signals_response->crowdstrike_signals) {
+    auto serialized_crowdstrike_signals =
+        response.agent_signals_response->crowdstrike_signals->ToValue();
+    if (serialized_crowdstrike_signals) {
+      signals.Set(device_signals::names::kCrowdStrike,
+                  std::move(serialized_crowdstrike_signals.value()));
+    }
+  }
+
+  std::move(done_closure).Run();
+}
+
+void BrowserSignalsDecorator::OnAllSignalsReceived(
+    base::TimeTicks start_time,
+    base::OnceClosure done_closure) {
+  LogSignalsCollectionLatency(kLatencyHistogramVariant, start_time);
   std::move(done_closure).Run();
 }
 

chrome/browser/enterprise/connectors/device_trust/signals/decorators/browser/browser_signals_decorator.h

@@ -20,27 +20,56 @@ namespace enterprise_signals {
 struct DeviceInfo;
 }  // namespace enterprise_signals
 
+namespace device_signals {
+class SignalsAggregator;
+struct SignalsAggregationResponse;
+}  // namespace device_signals
+
 namespace enterprise_connectors {
 
 // Definition of the SignalsDecorator common to all Chrome browser platforms.
 class BrowserSignalsDecorator : public SignalsDecorator {
  public:
-  explicit BrowserSignalsDecorator(
-      policy::CloudPolicyStore* cloud_policy_store);
+  BrowserSignalsDecorator(
+      policy::CloudPolicyStore* cloud_policy_store,
+      device_signals::SignalsAggregator* signals_aggregator);
   ~BrowserSignalsDecorator() override;
 
   // SignalsDecorator:
   void Decorate(base::Value::Dict& signals,
                 base::OnceClosure done_closure) override;
 
  private:
+  // Called when done retrieving signals from DeviceInfoFetcher. The retrieved
+  // signals are added to `signals` for the caller to use. `done_closure` can be
+  // invoked to indicate that this part of the signals collection has concluded.
+  // `device_info` represents the signals collected by the DeviceInfoFetcher.
   void OnDeviceInfoFetched(base::Value::Dict& signals,
-                           base::TimeTicks start_time,
                            base::OnceClosure done_closure,
                            const enterprise_signals::DeviceInfo& device_info);
 
+  // Called when done retrieving signals from SignalsAggregator. The retrieved
+  // signals are added to `signals` for the caller to use. `done_closure` can be
+  // invoked to indicate that this part of the signals collection has concluded.
+  // `response` represents the signals collected by the aggregator.
+  void OnAggregatedSignalsReceived(
+      base::Value::Dict& signals,
+      base::OnceClosure done_closure,
+      device_signals::SignalsAggregationResponse response);
+
+  // Ultimately called when all async signals are retrieved. `start_time`
+  // indicates the timestamp at which signal collection started for this
+  // decorator. `done_closure` will be run to let the caller know that the
+  // decorator is done collecting signals.
+  void OnAllSignalsReceived(base::TimeTicks start_time,
+                            base::OnceClosure done_closure);
+
   const raw_ptr<policy::CloudPolicyStore> cloud_policy_store_;
 
+  // Signals aggregator, which is a profile-keyed service. Can be nullptr in
+  // the case where the Profile is an incognito profile.
+  const raw_ptr<device_signals::SignalsAggregator> signals_aggregator_;
+
   base::WeakPtrFactory<BrowserSignalsDecorator> weak_ptr_factory_{this};
 };