Try to eliminate unknown policy fetch reason

We have lots of policy fetch with "Unspecified" reason on desktop and
mobile platforms.

The main reason is CBCM policy provider tracks schema change
unnecessarily. Because we always download all component policies. And
component_policy_service also listen to schema update and filter out
unused ones locally. Hence there is no reason to refresh policy
because of that.

Also added a few new reasons that technically shouldn't trigger policy
fetch request for future debugging purpose.


Bug: b/369420289
Change-Id: Id8344ac4bd333969fccbeb7ea9c88fec9f657c8e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5891932
Commit-Queue: Owen Min <[email protected]>
Reviewed-by: Yann Dago <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1363106}


CrOS-Libchrome-Original-Commit: 5a57d06fdce53a37169d136ed2152ea03210ec9a

Author: Owen Min

components/policy/core/common/command_line_policy_provider.cc

@@ -61,7 +61,7 @@ bool CommandLinePolicyProvider::IsFirstPolicyLoadComplete(
 CommandLinePolicyProvider::CommandLinePolicyProvider(
     const base::CommandLine& command_line)
     : loader_(command_line) {
-  RefreshPolicies(PolicyFetchReason::kUnspecified);
+  RefreshPolicies(PolicyFetchReason::kBrowserStart);
 }
 
 }  // namespace policy

components/policy/core/common/policy_types.h

@@ -154,6 +154,10 @@ enum class PolicyFetchReason {
   kUserRequest,
   // Policy fetch as previous request failed.
   kRetry,
+  // Schema Update
+  kSchemaUpdated,
+  // Disconnect from cloud management
+  kDisconnect,
 };
 
 }  // namespace policy

components/policy/core/common/schema_registry_tracking_policy_provider.cc

@@ -9,6 +9,7 @@
 #include "components/policy/core/common/policy_types.h"
 #include "components/policy/core/common/schema_map.h"
 #include "components/policy/core/common/schema_registry.h"
+#include "policy_types.h"
 
 namespace policy {
 
@@ -68,15 +69,15 @@ void SchemaRegistryTrackingPolicyProvider::OnSchemaRegistryReady() {
   }
 
   state_ = WAITING_FOR_REFRESH;
-  RefreshPolicies(PolicyFetchReason::kUnspecified);
+  RefreshPolicies(PolicyFetchReason::kSchemaUpdated);
 }
 
 void SchemaRegistryTrackingPolicyProvider::OnSchemaRegistryUpdated(
     bool has_new_schemas) {
   if (state_ != READY)
     return;
   if (has_new_schemas) {
-    RefreshPolicies(PolicyFetchReason::kUnspecified);
+    RefreshPolicies(PolicyFetchReason::kSchemaUpdated);
   } else {
     // Remove the policies that were being served for the component that have
     // been removed. This is important so that update notifications are also

components/policy/core/common/schema_registry_tracking_policy_provider_unittest.cc

@@ -132,14 +132,16 @@ TEST_F(SchemaRegistryTrackingPolicyProviderTest, SchemaReadyWithComponents) {
   mock_provider_.UpdatePolicy(std::move(bundle));
   Mock::VerifyAndClearExpectations(&observer_);
 
-  EXPECT_CALL(mock_provider_, RefreshPolicies(PolicyFetchReason::kUnspecified))
+  EXPECT_CALL(mock_provider_,
+              RefreshPolicies(PolicyFetchReason::kSchemaUpdated))
       .Times(0);
   schema_registry_.RegisterComponent(
       PolicyNamespace(POLICY_DOMAIN_EXTENSIONS, "xyz"), CreateTestSchema());
   schema_registry_.SetExtensionsDomainsReady();
   Mock::VerifyAndClearExpectations(&mock_provider_);
 
-  EXPECT_CALL(mock_provider_, RefreshPolicies(PolicyFetchReason::kUnspecified));
+  EXPECT_CALL(mock_provider_,
+              RefreshPolicies(PolicyFetchReason::kSchemaUpdated));
   schema_registry_.SetDomainReady(POLICY_DOMAIN_CHROME);
   Mock::VerifyAndClearExpectations(&mock_provider_);
 
@@ -178,7 +180,8 @@ TEST_F(SchemaRegistryTrackingPolicyProviderTest, DelegateUpdates) {
   mock_provider_.UpdateChromePolicy(policy_map);
   Mock::VerifyAndClearExpectations(&observer_);
 
-  EXPECT_CALL(mock_provider_, RefreshPolicies(PolicyFetchReason::kUnspecified));
+  EXPECT_CALL(mock_provider_,
+              RefreshPolicies(PolicyFetchReason::kSchemaUpdated));
   schema_registry_.SetAllDomainsReady();
   EXPECT_TRUE(schema_registry_.IsReady());
   Mock::VerifyAndClearExpectations(&mock_provider_);
@@ -201,7 +204,8 @@ TEST_F(SchemaRegistryTrackingPolicyProviderTest, DelegateUpdates) {
 }
 
 TEST_F(SchemaRegistryTrackingPolicyProviderTest, RemoveAndAddComponent) {
-  EXPECT_CALL(mock_provider_, RefreshPolicies(PolicyFetchReason::kUnspecified));
+  EXPECT_CALL(mock_provider_,
+              RefreshPolicies(PolicyFetchReason::kSchemaUpdated));
   const PolicyNamespace ns(POLICY_DOMAIN_EXTENSIONS, "xyz");
   schema_registry_.RegisterComponent(ns, CreateTestSchema());
   schema_registry_.SetAllDomainsReady();
@@ -227,7 +231,8 @@ TEST_F(SchemaRegistryTrackingPolicyProviderTest, RemoveAndAddComponent) {
 
   // Adding it back should serve the current policies again, even though they
   // haven't changed on the platform provider.
-  EXPECT_CALL(mock_provider_, RefreshPolicies(PolicyFetchReason::kUnspecified));
+  EXPECT_CALL(mock_provider_,
+              RefreshPolicies(PolicyFetchReason::kSchemaUpdated));
   schema_registry_.RegisterComponent(ns, CreateTestSchema());
   Mock::VerifyAndClearExpectations(&mock_provider_);