Add ArcPolicyBridge::Observer

Add an observer interface to ArcPolicyBridge. Observers are notified
whenever policy is sent to CloudDPC or a compliance report is
received from it.

The CL also forces a compliance report to be sent after the initial
policy fetch by adding a GUID to the policy JSON which changes
whenever a new instances of ArcPolicyBridge is created. A cleaner
way to do this would be a |force_compliance_report| flag sent to
CloudDPC. This clean-up is tracked in a follow-up bug.

Bug: b/73277923
Bug: b/73762796
Test: unit_tests
Change-Id: Ib4cdfb28e4b3ea2ea0ca03d1cd7293d7e07f0bcd
Reviewed-on: https://chromium-review.googlesource.com/931705
Commit-Queue: Bartosz Fabianowski <bartfab@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538838}

chrome/browser/chromeos/arc/policy/arc_policy_bridge.cc

@@ -9,6 +9,7 @@
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
+#include "base/guid.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_string_value_serializer.h"
 #include "base/logging.h"
@@ -199,6 +200,7 @@ void AddOncCaCertsToPolicies(const policy::PolicyMap& policy_map,
 }
 
 std::string GetFilteredJSONPolicies(const policy::PolicyMap& policy_map,
+                                    const std::string& guid,
                                     bool is_affiliated) {
   base::DictionaryValue filtered_policies;
   // Parse ArcPolicy as JSON string before adding other policies to the
@@ -245,6 +247,8 @@ std::string GetFilteredJSONPolicies(const policy::PolicyMap& policy_map,
   if (!is_affiliated)
     filtered_policies.RemoveKey("apkCacheEnabled");
 
+  filtered_policies.SetString("guid", guid);
+
   std::string policy_json;
   JSONStringValueSerializer serializer(&policy_json);
   serializer.Serialize(filtered_policies);
@@ -330,6 +334,7 @@ ArcPolicyBridge::ArcPolicyBridge(content::BrowserContext* context,
     : context_(context),
       arc_bridge_service_(bridge_service),
       policy_service_(policy_service),
+      instance_guid_(base::GenerateGUID()),
       weak_ptr_factory_(this) {
   VLOG(2) << "ArcPolicyBridge::ArcPolicyBridge";
   arc_bridge_service_->policy()->SetHost(this);
@@ -342,6 +347,18 @@ ArcPolicyBridge::~ArcPolicyBridge() {
   arc_bridge_service_->policy()->SetHost(nullptr);
 }
 
+const std::string& ArcPolicyBridge::GetInstanceGuidForTesting() {
+  return instance_guid_;
+}
+
+void ArcPolicyBridge::AddObserver(Observer* observer) {
+  observers_.AddObserver(observer);
+}
+
+void ArcPolicyBridge::RemoveObserver(Observer* observer) {
+  observers_.RemoveObserver(observer);
+}
+
 void ArcPolicyBridge::OverrideIsManagedForTesting(bool is_managed) {
   is_managed_ = is_managed;
 }
@@ -364,7 +381,11 @@ void ArcPolicyBridge::OnConnectionClosed() {
 
 void ArcPolicyBridge::GetPolicies(GetPoliciesCallback callback) {
   VLOG(1) << "ArcPolicyBridge::GetPolicies";
-  std::move(callback).Run(GetCurrentJSONPolicies());
+  const std::string policy = GetCurrentJSONPolicies();
+  for (Observer& observer : observers_) {
+    observer.OnPolicySent(policy);
+  }
+  std::move(callback).Run(policy);
 }
 
 void ArcPolicyBridge::ReportCompliance(const std::string& request,
@@ -420,7 +441,8 @@ std::string ArcPolicyBridge::GetCurrentJSONPolicies() const {
   const user_manager::User* const user =
       chromeos::ProfileHelper::Get()->GetUserByProfile(profile);
 
-  return GetFilteredJSONPolicies(policy_map, user->IsAffiliated());
+  return GetFilteredJSONPolicies(policy_map, instance_guid_,
+                                 user->IsAffiliated());
 }
 
 void ArcPolicyBridge::OnReportComplianceParseSuccess(
@@ -432,8 +454,12 @@ void ArcPolicyBridge::OnReportComplianceParseSuccess(
       prefs::kArcPolicyComplianceReported, true);
 
   const base::DictionaryValue* dict = nullptr;
-  if (parsed_json->GetAsDictionary(&dict))
+  if (parsed_json->GetAsDictionary(&dict)) {
     UpdateComplianceReportMetrics(dict);
+    for (Observer& observer : observers_) {
+      observer.OnComplianceReportReceived(parsed_json.get());
+    }
+  }
 }
 
 void ArcPolicyBridge::UpdateComplianceReportMetrics(

chrome/browser/chromeos/arc/policy/arc_policy_bridge.h

@@ -10,6 +10,7 @@
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "base/observer_list.h"
 #include "base/time/time.h"
 #include "components/arc/common/policy.mojom.h"
 #include "components/arc/connection_observer.h"
@@ -17,6 +18,10 @@
 #include "components/policy/core/common/policy_namespace.h"
 #include "components/policy/core/common/policy_service.h"
 
+namespace base {
+class Value;
+}
+
 namespace content {
 class BrowserContext;
 }  // namespace content
@@ -43,6 +48,22 @@ class ArcPolicyBridge : public KeyedService,
                         public mojom::PolicyHost,
                         public policy::PolicyService::Observer {
  public:
+  class Observer {
+   public:
+    // Called when policy is sent to CloudDPC.
+    virtual void OnPolicySent(const std::string& policy) = 0;
+
+    // Called when a compliance report is received from CloudDPC.
+    virtual void OnComplianceReportReceived(
+        const base::Value* compliance_report) = 0;
+
+   protected:
+    Observer() = default;
+    virtual ~Observer() = default;
+
+    DISALLOW_COPY_AND_ASSIGN(Observer);
+  };
+
   // Returns singleton instance for the given BrowserContext,
   // or nullptr if the browser |context| is not allowed to use ARC.
   static ArcPolicyBridge* GetForBrowserContext(
@@ -55,6 +76,11 @@ class ArcPolicyBridge : public KeyedService,
                   policy::PolicyService* policy_service);
   ~ArcPolicyBridge() override;
 
+  const std::string& GetInstanceGuidForTesting();
+
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
   void OverrideIsManagedForTesting(bool is_managed);
 
   // ConnectionObserver<mojom::PolicyInstance> overrides.
@@ -90,6 +116,10 @@ class ArcPolicyBridge : public KeyedService,
   policy::PolicyService* policy_service_ = nullptr;
   bool is_managed_ = false;
 
+  // HACK(b/73762796): A GUID that is regenerated whenever |this| is created,
+  // ensuring that the first policy sent to CloudDPC is considered different
+  // from previous policy and a compliance report is sent.
+  const std::string instance_guid_;
   // Hash of the policies that were up to date when ARC started.
   std::string initial_policies_hash_;
   // Whether the UMA metric for the first successfully obtained compliance
@@ -105,6 +135,8 @@ class ArcPolicyBridge : public KeyedService,
   // since the most recent policy update notificaton was already reported.
   bool compliance_since_update_timing_reported_ = false;
 
+  base::ObserverList<Observer, true /* check_empty */> observers_;
+
   // Must be the last member.
   base::WeakPtrFactory<ArcPolicyBridge> weak_ptr_factory_;