[Origin] Defer policy init signal until BraveProfilePolicyProvider refreshes (#36495)

[Origin] Gate BraveProfilePolicyProvider on BraveOriginPolicyManager init

`BraveProvider`'s first `RefreshPolicies` is triggered by
`AdBlockOnlyModePolicyManager` (initialised at process start), well before
`BraveOriginPolicyManager` (lazily initialised by `BraveOriginServiceFactory`)
is ready. The resulting bundle lacks `BraveRewardsDisabled`, but the
default `IsInitializationComplete` lets `PolicyService` report ready
anyway. `AdsServiceImpl::MaybeStartBatAdsService` then evaluates a stale
managed pref store and bat-ads starts despite the policy.

Override `IsInitializationComplete` to additionally gate on
`BraveOriginPolicyManager::IsInitialized()`. Upstream unit tests
(`ProfilePolicyConnectorTest`) bypass our factory bootstrap, so add a
`SetExpectedToBeInitialized` flag set from `BraveOriginServiceFactory`'s
ctor; the override short-circuits when unset (desktop/Android only --
iOS doesn't run those tests).

Resolves: https://github.com/brave/brave-browser/issues/54962

browser/brave_origin/brave_origin_service_factory.cc

@@ -233,6 +233,29 @@ BraveOriginServiceFactory::BraveOriginServiceFactory()
           "BraveOriginService",
           ProfileSelections::BuildRedirectedInIncognito()) {
   DependsOn(skus::SkusServiceFactory::GetInstance());
+  // This factory owns the lazy initialization of `BraveOriginPolicyManager`
+  // (in `BuildServiceInstanceForBrowserContext`). Mark the manager as
+  // expected-to-be-initialized as soon as the factory is registered so that
+  // `BraveProfilePolicyProvider::IsInitializationComplete` knows to gate on
+  // the manager actually finishing initialization. Without this gate
+  // `BraveProvider`'s first `RefreshPolicies` (triggered by
+  // `AdBlockOnlyModePolicyManager`'s `OnBravePoliciesReady`, which fires at
+  // process start) would flip `first_policies_loaded_` to true with an
+  // empty Brave Origin bundle, and consumers of
+  // `PolicyService::OnPolicyServiceInitialized` (notably
+  // `AdsServiceImpl::MaybeStartBatAdsService`) would evaluate against a
+  // managed pref store that does not yet reflect `BraveRewardsDisabled`.
+  //
+  // Upstream unit tests build `ProfilePolicyConnector` directly without
+  // going through `EnsureBrowserContextKeyedServiceFactoriesBuilt()`, so this
+  // factory's `GetInstance()` is never called in those tests, the flag stays
+  // false, and `BraveProvider::IsInitializationComplete` short-circuits to
+  // true to avoid blocking the upstream tests forever. The affected tests
+  // are in `chrome/browser/policy/profile_policy_connector_unittest.cc`,
+  // primarily the `ProfilePolicyConnectorTest.AffiliationMetrics_*`,
+  // `LocalTestProviderUseAndRevert`, and `ChromeosPrimaryUserPoliciesProxied`
+  // cases that use `PolicyServiceInitializedWaiter`.
+  BraveOriginPolicyManager::GetInstance()->SetExpectedToBeInitialized();
 }
 
 BraveOriginServiceFactory::~BraveOriginServiceFactory() = default;

components/brave_origin/brave_origin_policy_manager.cc

@@ -167,6 +167,14 @@ bool BraveOriginPolicyManager::IsInitialized() const {
   return initialized_;
 }
 
+void BraveOriginPolicyManager::SetExpectedToBeInitialized() {
+  expected_to_be_initialized_ = true;
+}
+
+bool BraveOriginPolicyManager::IsExpectedToBeInitialized() const {
+  return expected_to_be_initialized_;
+}
+
 void BraveOriginPolicyManager::SetPurchased(bool purchased) {
   if (is_purchased_ == purchased) {
     return;
@@ -196,6 +204,7 @@ bool BraveOriginPolicyManager::IsPurchased() const {
 
 void BraveOriginPolicyManager::Shutdown() {
   initialized_ = false;
+  expected_to_be_initialized_ = false;
   is_purchased_ = false;
   browser_policy_definitions_.clear();
   profile_policy_definitions_.clear();

components/brave_origin/brave_origin_policy_manager.h

@@ -69,6 +69,13 @@ class BraveOriginPolicyManager {
   // Check if the singleton has been initialized
   bool IsInitialized() const;
 
+  // Declares that `Init()` is expected to be called later in this process.
+  // Consumers gated on this manager use it to distinguish production startup
+  // (where lazy `Init()` will eventually run, so they should wait) from test
+  // contexts that never bootstrap us (where there is nothing to wait for).
+  void SetExpectedToBeInitialized();
+  bool IsExpectedToBeInitialized() const;
+
   // Set/get the purchase state. When the purchase state changes,
   // observers are notified to refresh policies.
   void SetPurchased(bool purchased);
@@ -97,6 +104,7 @@ class BraveOriginPolicyManager {
                               std::optional<std::string_view> profile_id) const;
 
   bool initialized_ = false;
+  bool expected_to_be_initialized_ = false;
   bool is_purchased_ = false;
   BraveOriginPolicyMap browser_policy_definitions_;
   BraveOriginPolicyMap profile_policy_definitions_;

components/brave_policy/BUILD.gn

@@ -56,6 +56,7 @@ source_set("unit_tests") {
     ":policy_initialization_waiter",
     "//base",
     "//base/test:test_support",
+    "//brave/components/brave_origin",
     "//brave/components/resources:strings",
     "//components/enterprise",
     "//components/policy/core/browser",

components/brave_policy/brave_profile_policy_provider.cc

@@ -7,10 +7,10 @@
 
 #include <utility>
 
-#include "base/logging.h"
 #include "base/values.h"
 #include "brave/components/brave_origin/brave_origin_utils.h"
 #include "brave/components/brave_policy/ad_block_only_mode/ad_block_only_mode_policy_manager.h"
+#include "build/build_config.h"
 #include "components/policy/core/common/policy_bundle.h"
 #include "components/policy/core/common/policy_map.h"
 #include "components/policy/core/common/policy_namespace.h"
@@ -50,6 +50,39 @@ void BraveProfilePolicyProvider::RefreshPolicies(
   UpdatePolicy(std::move(bundle));
 }
 
+bool BraveProfilePolicyProvider::IsInitializationComplete(
+    policy::PolicyDomain domain) const {
+  auto* manager = brave_origin::BraveOriginPolicyManager::GetInstance();
+#if !BUILDFLAG(IS_IOS)
+  // Desktop/Android only: short-circuit when our factory has not been
+  // registered in this process. This is the case for upstream unit tests
+  // (e.g. `ProfilePolicyConnectorTest`) that build `ProfilePolicyConnector`
+  // directly without going through Brave's keyed-service-factory startup --
+  // those tests have no Brave Origin source to wait for, so we report
+  // ready and let the upstream `OnPolicyServiceInitialized` signal fire
+  // normally.
+  //
+  // iOS does not run these upstream unit tests, and its own keyed-service
+  // startup always constructs `BraveOriginServiceFactory` (which lazily
+  // initializes `BraveOriginPolicyManager`). The gate below works there
+  // without the flag check.
+  if (!manager->IsExpectedToBeInitialized()) {
+    return true;
+  }
+#endif  // !BUILDFLAG(IS_IOS)
+  // We observe both `BraveOriginPolicyManager` and
+  // `AdBlockOnlyModePolicyManager` but they initialise on different schedules:
+  // `AdBlockOnlyModePolicyManager` is initialised synchronously at process
+  // start while `BraveOriginPolicyManager` is initialised lazily during
+  // profile keyed-service build. The first `RefreshPolicies` triggered by
+  // AdBlockOnlyMode's `OnBravePoliciesReady` produces a bundle without Brave
+  // Origin policies and would otherwise flip `first_policies_loaded_` to true
+  // prematurely. Gate on Brave Origin's `IsInitialized()` so consumers don't
+  // observe an empty managed pref store before Brave Origin policies (e.g.
+  // `BraveRewardsDisabled`) have been merged.
+  return first_policies_loaded_ && manager->IsInitialized();
+}
+
 bool BraveProfilePolicyProvider::IsFirstPolicyLoadComplete(
     policy::PolicyDomain domain) const {
   return first_policies_loaded_;

components/brave_policy/brave_profile_policy_provider.h

@@ -34,6 +34,7 @@ class BraveProfilePolicyProvider : public policy::ConfigurationPolicyProvider,
   // ConfigurationPolicyProvider implementation.
   void Init(policy::SchemaRegistry* registry) override;
   void RefreshPolicies(policy::PolicyFetchReason reason) override;
+  bool IsInitializationComplete(policy::PolicyDomain domain) const override;
   bool IsFirstPolicyLoadComplete(policy::PolicyDomain domain) const override;
 
   // BravePolicyObserver implementation.

components/brave_policy/brave_profile_policy_provider_unittest.cc

@@ -8,6 +8,8 @@
 #include <memory>
 
 #include "base/test/task_environment.h"
+#include "brave/components/brave_origin/brave_origin_policy_manager.h"
+#include "brave/components/brave_origin/brave_origin_prefs.h"
 #include "components/policy/core/common/policy_bundle.h"
 #include "components/policy/core/common/policy_namespace.h"
 #include "components/policy/core/common/policy_types.h"
@@ -19,18 +21,23 @@ namespace brave_policy {
 
 class BraveProfilePolicyProviderTest : public ::testing::Test {
  public:
-  BraveProfilePolicyProviderTest() = default;
+  BraveProfilePolicyProviderTest() {
+    brave_origin::RegisterLocalStatePrefs(local_state_.registry());
+  }
   ~BraveProfilePolicyProviderTest() override = default;
 
   void TearDown() override {
-    if (provider_.IsInitializationComplete(policy::POLICY_DOMAIN_CHROME)) {
-      provider_.Shutdown();
-    }
+    provider_.Shutdown();
+    // Reset the process-wide singleton so its state does not leak between
+    // tests. `BraveOriginPolicyManager::Shutdown()` is safe to call even if
+    // `Init()` was never called.
+    brave_origin::BraveOriginPolicyManager::GetInstance()->Shutdown();
   }
 
  protected:
   base::test::TaskEnvironment task_environment_;
   policy::SchemaRegistry schema_registry_;
+  TestingPrefServiceSimple local_state_;
   BraveProfilePolicyProvider provider_;
 };
 
@@ -55,6 +62,39 @@ TEST_F(BraveProfilePolicyProviderTest, InitAndPolicyLoadComplete) {
       provider_.IsFirstPolicyLoadComplete(policy::POLICY_DOMAIN_CHROME));
 }
 
+TEST_F(BraveProfilePolicyProviderTest, IsInitializationCompleteGated) {
+  auto* manager = brave_origin::BraveOriginPolicyManager::GetInstance();
+  // On desktop/Android the override short-circuits to true when the manager
+  // has not been declared as expected-to-initialize (see the `#if !IS_IOS`
+  // block in `IsInitializationComplete`). Set the flag so the gating path
+  // below is exercised on all platforms.
+  manager->SetExpectedToBeInitialized();
+
+  // Before the manager is initialized, the override gates initialisation on
+  // `IsInitialized()` so consumers do not observe an empty managed pref
+  // store before Brave Origin policies have been merged.
+  EXPECT_FALSE(provider_.IsInitializationComplete(policy::POLICY_DOMAIN_CHROME))
+      << "Before provider Init: manager not initialised, no refresh";
+
+  provider_.Init(&schema_registry_);
+  EXPECT_FALSE(provider_.IsInitializationComplete(policy::POLICY_DOMAIN_CHROME))
+      << "After provider Init: manager still not initialised";
+
+  // Initializing the manager flips `IsInitialized()` and synchronously
+  // notifies our observer, but `RefreshPolicies` is gated on a non-empty
+  // `profile_id_`, so `first_policies_loaded_` is still false.
+  manager->Init(/*browser_policy_definitions=*/{},
+                /*profile_policy_definitions=*/{}, &local_state_);
+  EXPECT_FALSE(provider_.IsInitializationComplete(policy::POLICY_DOMAIN_CHROME))
+      << "After manager Init: profile_id_ still empty, refresh has not run";
+
+  // Setting the profile ID drives the first refresh.
+  provider_.SetProfileID("test-profile-id");
+  EXPECT_TRUE(provider_.IsInitializationComplete(policy::POLICY_DOMAIN_CHROME))
+      << "After SetProfileID: first refresh has run and manager is "
+         "initialised";
+}
+
 TEST_F(BraveProfilePolicyProviderTest, EmptyPolicyBundle) {
   // Initialize the provider
   provider_.Init(&schema_registry_);

components/brave_policy/policy_initialization_waiter.h

@@ -8,6 +8,7 @@
 
 #include "base/functional/callback.h"
 #include "base/memory/raw_ptr.h"
+#include "components/policy/core/common/policy_namespace.h"
 #include "components/policy/core/common/policy_service.h"
 
 namespace brave_policy {