Discharge7443/brave-core
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "[Origin] Apply profile policies before pref store reads (#36023)"

Browse Source 
Reverts: https://github.com/brave/brave-core/pull/36023
This reverts commit https://github.com/brave/brave-core/commit/acaec910e9757dae29cb1d622846373966a4dd6c.
This commit is contained in:
Serg
2026-05-08 00:50:27 +01:00
committed by GitHub
parent 35cde49e8a
commit de7f477b75
9 changed files with 8 additions and 240 deletions
Download Patch File Download Diff File Expand all files Collapse all files
browser/brave_browser_process_impl.cc
-13
View File
@@ -14,7 +14,6 @@
#include "base/memory/scoped_refptr.h"
#include "base/path_service.h"
#include "base/task/thread_pool.h"
#include "brave/browser/brave_origin/brave_origin_service_factory.h"
#include "brave/browser/brave_referrals/referrals_service_delegate.h"
#include "brave/browser/brave_shields/ad_block_subscription_download_manager_getter.h"
#include "brave/browser/brave_stats/brave_stats_updater.h"
@@ -211,18 +210,6 @@ void BraveBrowserProcessImpl::Init() {
// Lazy initialization of AdBlockOnlyModePolicyManager
brave_policy::AdBlockOnlyModePolicyManager::GetInstance()->Init(
local_state());
// Initialize BraveOriginPolicyManager at browser-process startup so the
// manager is ready before profile policy connectors are built. Combined
// with the profile-path stash consumed by
// BRAVE_PROFILE_POLICY_CONNECTOR_INIT, this ensures
// BraveProfilePolicyProvider can populate its bundle and propagate the
// resulting policies to the managed pref store before consumers like
// AdsService::CanStartBatAdsService read kDisabledByPolicy.
brave_origin::BraveOriginPolicyManager::GetInstance()->Init(
brave_origin::BraveOriginServiceFactory::GetBrowserPolicyDefinitions(),
brave_origin::BraveOriginServiceFactory::GetProfilePolicyDefinitions(),
local_state());
}
void BraveBrowserProcessImpl::PreMainMessageLoopRun() {
chromium_src/chrome/browser/DEPS
+1 -2
View File
@@ -7,8 +7,7 @@ include_rules += [
"+brave/common",
"+brave/build/android/jni_headers",
"+brave/grit",
"+brave/components/brave_origin",
"+brave/components/brave_policy",
"+brave/components/brave_origin/buildflags",
"+brave/components/constants",
"+brave/components/history_embeddings",
"+brave/components/local_ai/core",
chromium_src/chrome/browser/policy/profile_policy_connector.cc
+5 -27
View File
@@ -3,8 +3,6 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at https://mozilla.org/MPL/2.0/. */
#include "brave/components/brave_origin/profile_id.h"
#include "brave/components/brave_policy/brave_profile_policy_provider.h"
#include "components/policy/core/common/configuration_policy_provider.h"
namespace brave_policy {
@@ -16,31 +14,11 @@ CreateBraveProfilePolicyProvider();
// for Brave Origin and other Brave specific policies.
// We do not need to define a patch for Shutdown since anything in
// wrapped_policy_providers_ will automatically call Shutdown.
//
// If a caller stashed a profile path via
// BraveProfilePolicyProvider::SetPendingProfilePath() immediately before
// the policy connector is built, consume it and call SetProfileID BEFORE
// provider->Init(). That ordering matters: Init's Observe() call fires
// OnBravePoliciesReady synchronously (when BraveOriginPolicyManager is
// already initialized at browser-process startup), and since profile_id_
// is now set, OnBravePoliciesReady triggers RefreshPolicies → LoadPolicies
// → UpdatePolicy. The provider's bundle is therefore populated before
// PolicyServiceImpl's constructor performs its synchronous merge of all
// providers, so the resulting policy_bundle_ already carries our policies.
// Without this, PolicyServiceImpl would read an empty bundle from us and
// the later async merge (posted from OnUpdatePolicy) would have to race
// the consumers reading prefs from the resulting PrefService.
#define BRAVE_PROFILE_POLICY_CONNECTOR_INIT \
auto provider = brave_policy::CreateBraveProfilePolicyProvider(); \
if (auto pending_path = \
brave_policy::BraveProfilePolicyProvider::TakePendingProfilePath(); \
!pending_path.empty()) { \
static_cast<brave_policy::BraveProfilePolicyProvider*>(provider.get()) \
->SetProfileID(brave_origin::GetProfileId(pending_path)); \
} \
brave_profile_policy_provider_ = provider.get(); \
policy_providers_.push_back(provider.get()); \
provider->Init(schema_registry); \
#define BRAVE_PROFILE_POLICY_CONNECTOR_INIT \
auto provider = brave_policy::CreateBraveProfilePolicyProvider(); \
brave_profile_policy_provider_ = provider.get(); \
policy_providers_.push_back(provider.get()); \
provider->Init(schema_registry); \
wrapped_policy_providers_.push_back(std::move(provider));
#include <chrome/browser/policy/profile_policy_connector.cc> // IWYU pragma: export
chromium_src/chrome/browser/policy/profile_policy_connector_builder.cc
+2 -16
View File
@@ -4,7 +4,6 @@
* You can obtain one at https://mozilla.org/MPL/2.0/. */
#include "base/files/file_path.h"
#include "brave/components/brave_policy/brave_profile_policy_provider.h"
#include "chrome/browser/policy/profile_policy_connector.h"
#define CreateProfilePolicyConnectorForBrowserContext \
@@ -15,6 +14,7 @@
#undef CreateProfilePolicyConnectorForBrowserContext
namespace brave_policy {
class BraveProfilePolicyProvider;
void SetBraveProfilePolicyProviderProfileID(
policy::ConfigurationPolicyProvider* provider,
const base::FilePath& profile_path);
@@ -30,24 +30,10 @@ CreateProfilePolicyConnectorForBrowserContext(
policy::ChromeBrowserPolicyConnector* browser_policy_connector,
bool force_immediate_load,
content::BrowserContext* context) {
// Stash the profile path so that BRAVE_PROFILE_POLICY_CONNECTOR_INIT can
// call SetProfileID on the BraveProfilePolicyProvider before its Init() runs
// — which causes the provider's bundle to be populated synchronously,
// before PolicyServiceImpl's constructor performs its synchronous merge.
// See chromium_src/chrome/browser/policy/profile_policy_connector.cc for
// the full rationale.
brave_policy::BraveProfilePolicyProvider::SetPendingProfilePath(
context->GetPath());
auto connector = CreateProfilePolicyConnectorForBrowserContext_ChromiumImpl(
schema_registry, cloud_policy_manager, policy_provider,
browser_policy_connector, force_immediate_load, context);
// Defensive clear: the macro normally consumes the stash, but if upstream
// changes ever cause the macro not to fire, don't leak the path into the
// next profile's connector creation.
brave_policy::BraveProfilePolicyProvider::TakePendingProfilePath();
// Some upstream browser tests don't do the normal flow so have no provider.
// Also, the macro-driven SetProfileID above is the load-bearing path; this
// call is now an idempotent fallback for any flow we might have missed.
// Some upstream browser tests don't do the normal flow so have no provider
if (connector->GetBraveProfilePolicyProvider()) {
brave_policy::SetBraveProfilePolicyProviderProfileID(
connector->GetBraveProfilePolicyProvider().get(), context->GetPath());
chromium_src/chrome/browser/startup_data.cc
-27
View File
@@ -1,27 +0,0 @@
/* Copyright (c) 2026 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at https://mozilla.org/MPL/2.0/. */
#include "brave/components/brave_policy/brave_profile_policy_provider.h"
// Stash the profile path that StartupData::CreateServicesInternal is about to
// use for the profile policy connector. The BRAVE_PROFILE_POLICY_CONNECTOR_INIT
// macro (in chromium_src/chrome/browser/policy/profile_policy_connector.cc)
// consumes the stash to call SetProfileID on the BraveProfilePolicyProvider
// before its Init() runs, so the provider's bundle is populated before
// PolicyServiceImpl's constructor performs its synchronous merge.
//
// This is the Android-specific entry point. On desktop, the equivalent is set
// in chromium_src/chrome/browser/policy/profile_policy_connector_builder.cc.
#define BRAVE_STARTUP_DATA_PRE_PROFILE_POLICY_CONNECTOR \
brave_policy::BraveProfilePolicyProvider::SetPendingProfilePath(path);
// Defensive clear: the macro normally consumes the stash, but if for any
// reason it didn't fire, don't leak the path into a subsequent profile.
#define BRAVE_STARTUP_DATA_POST_PROFILE_POLICY_CONNECTOR \
brave_policy::BraveProfilePolicyProvider::TakePendingProfilePath();
#include <chrome/browser/startup_data.cc>
#undef BRAVE_STARTUP_DATA_PRE_PROFILE_POLICY_CONNECTOR
#undef BRAVE_STARTUP_DATA_POST_PROFILE_POLICY_CONNECTOR
components/brave_policy/brave_profile_policy_provider.cc
-36
View File
@@ -22,35 +22,6 @@
namespace brave_policy {
namespace {
// Thread-local stash consumed by BRAVE_PROFILE_POLICY_CONNECTOR_INIT.
// See header for rationale. Stored as a raw pointer (a trivial type, so it
// avoids -Wexit-time-destructors which forbids non-trivial destructors on
// static / thread_local storage). The pointer is null between Set and
// Take calls; Set deletes any prior stash, and Take takes ownership.
thread_local base::FilePath* g_pending_profile_path = nullptr;
} // namespace
// static
void BraveProfilePolicyProvider::SetPendingProfilePath(
const base::FilePath& path) {
delete g_pending_profile_path;
g_pending_profile_path = new base::FilePath(path);
}
// static
base::FilePath BraveProfilePolicyProvider::TakePendingProfilePath() {
base::FilePath path;
if (g_pending_profile_path) {
path = std::move(*g_pending_profile_path);
delete g_pending_profile_path;
g_pending_profile_path = nullptr;
}
return path;
}
BraveProfilePolicyProvider::BraveProfilePolicyProvider() = default;
BraveProfilePolicyProvider::~BraveProfilePolicyProvider() = default;
@@ -182,13 +153,6 @@ void BraveProfilePolicyProvider::OnProfilePolicyChanged(
}
void BraveProfilePolicyProvider::SetProfileID(const std::string& profile_id) {
// If the same profile_id is set again (which happens when the
// path stash already set it during BRAVE_PROFILE_POLICY_CONNECTOR_INIT and
// a legacy SetBraveProfilePolicyProviderProfileID caller sets it later with
// the same value), don't re-fire RefreshPolicies.
if (profile_id_ == profile_id) {
return;
}
profile_id_ = profile_id;
// If policies are ready already and we now have a profile_Id, refresh the
components/brave_policy/brave_profile_policy_provider.h
-11
View File
@@ -7,7 +7,6 @@
#define BRAVE_COMPONENTS_BRAVE_POLICY_BRAVE_PROFILE_POLICY_PROVIDER_H_
#include "base/containers/flat_set.h"
#include "base/files/file_path.h"
#include "base/memory/raw_ptr.h"
#include "base/memory/weak_ptr.h"
#include "base/scoped_observation.h"
@@ -43,16 +42,6 @@ class BraveProfilePolicyProvider : public policy::ConfigurationPolicyProvider,
std::string_view profile_id) override;
void SetProfileID(const std::string& profile_id);
// Stash the profile path for the next BraveProfilePolicyProvider that the
// BRAVE_PROFILE_POLICY_CONNECTOR_INIT macro creates. Set immediately before
// the policy connector is built (in callers that have BrowserContext / path
// available); the macro consumes the stash and calls SetProfileID before
// Init, so OnBravePoliciesReady fires synchronously inside Observe and
// RefreshPolicies populates the bundle BEFORE PolicyServiceImpl's
// synchronous-merge constructor reads it.
static void SetPendingProfilePath(const base::FilePath& path);
static base::FilePath TakePendingProfilePath();
private:
policy::PolicyBundle LoadPolicies();
components/brave_policy/brave_profile_policy_provider_unittest.cc
-88
View File
@@ -7,9 +7,7 @@
#include <memory>
#include "base/files/file_path.h"
#include "base/test/task_environment.h"
#include "components/policy/core/common/configuration_policy_provider.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"
@@ -185,90 +183,4 @@ TEST_F(BraveProfilePolicyProviderTest, OnProfilePolicyChanged_EmptyProfileId) {
provider_.IsFirstPolicyLoadComplete(policy::POLICY_DOMAIN_CHROME));
}
// ---- Pending profile-path stash ----
class BraveProfilePolicyProviderStashTest : public ::testing::Test {
protected:
void TearDown() override {
// Defensive: clear the stash so tests don't leak state between cases.
BraveProfilePolicyProvider::TakePendingProfilePath();
}
};
TEST_F(BraveProfilePolicyProviderStashTest, TakeWithoutSetReturnsEmpty) {
EXPECT_TRUE(BraveProfilePolicyProvider::TakePendingProfilePath().empty());
}
TEST_F(BraveProfilePolicyProviderStashTest, SetThenTakeReturnsSamePath) {
const base::FilePath path(FILE_PATH_LITERAL("/tmp/profile/Default"));
BraveProfilePolicyProvider::SetPendingProfilePath(path);
EXPECT_EQ(path, BraveProfilePolicyProvider::TakePendingProfilePath());
}
TEST_F(BraveProfilePolicyProviderStashTest, TakeClearsStash) {
BraveProfilePolicyProvider::SetPendingProfilePath(
base::FilePath(FILE_PATH_LITERAL("/tmp/profile/Default")));
EXPECT_FALSE(BraveProfilePolicyProvider::TakePendingProfilePath().empty());
// Subsequent Take should return empty.
EXPECT_TRUE(BraveProfilePolicyProvider::TakePendingProfilePath().empty());
}
TEST_F(BraveProfilePolicyProviderStashTest, RepeatedSetOnlyKeepsLatest) {
BraveProfilePolicyProvider::SetPendingProfilePath(
base::FilePath(FILE_PATH_LITERAL("/tmp/profile/A")));
BraveProfilePolicyProvider::SetPendingProfilePath(
base::FilePath(FILE_PATH_LITERAL("/tmp/profile/B")));
// Latest set wins; no leak from prior Set.
EXPECT_EQ(base::FilePath(FILE_PATH_LITERAL("/tmp/profile/B")),
BraveProfilePolicyProvider::TakePendingProfilePath());
// Only one path was stashed; subsequent Take is empty.
EXPECT_TRUE(BraveProfilePolicyProvider::TakePendingProfilePath().empty());
}
// ---- SetProfileID idempotency ----
namespace {
// Counts how many times the provider notifies observers via UpdatePolicy.
class UpdateCountingObserver
: public policy::ConfigurationPolicyProvider::Observer {
public:
void OnUpdatePolicy(
policy::ConfigurationPolicyProvider* /*provider*/) override {
++count_;
}
int count() const { return count_; }
private:
int count_ = 0;
};
} // namespace
TEST_F(BraveProfilePolicyProviderTest, SetProfileID_SameIdIsIdempotent) {
provider_.Init(&schema_registry_);
UpdateCountingObserver observer;
provider_.AddObserver(&observer);
// Make policies_ready_ true so SetProfileID would otherwise fire
// RefreshPolicies.
provider_.OnBravePoliciesReady();
// First SetProfileID triggers a RefreshPolicies → one observer notification.
provider_.SetProfileID("test-profile-id");
const int after_first = observer.count();
EXPECT_GT(after_first, 0);
// Calling SetProfileID with the same id should be a no-op — no new
// notifications.
provider_.SetProfileID("test-profile-id");
EXPECT_EQ(after_first, observer.count());
// Calling with a different id does fire RefreshPolicies again.
provider_.SetProfileID("different-id");
EXPECT_GT(observer.count(), after_first);
provider_.RemoveObserver(&observer);
}
} // namespace brave_policy
patches/chrome-browser-startup_data.cc.patch
-20
View File
@@ -1,20 +0,0 @@
diff --git a/chrome/browser/startup_data.cc b/chrome/browser/startup_data.cc
index 57ccf58618fb75ac909b7b2812ee3ede0b1333a5..745d21543de71be6287804a2e8944390611faac5 100644
--- a/chrome/browser/startup_data.cc
+++ b/chrome/browser/startup_data.cc
@@ -223,6 +223,7 @@ void StartupData::CreateServicesInternal() {
true /* force_immediate_policy_load */, io_task_runner,
base::BindRepeating(&content::GetNetworkConnectionTracker));
+ BRAVE_STARTUP_DATA_PRE_PROFILE_POLICY_CONNECTOR
profile_policy_connector_ = policy::CreateAndInitProfilePolicyConnector(
schema_registry_service_->registry(),
static_cast<policy::ChromeBrowserPolicyConnector*>(
@@ -230,6 +231,7 @@ void StartupData::CreateServicesInternal() {
user_cloud_policy_manager_.get(),
user_cloud_policy_manager_->core()->store(),
true /* force_immediate_policy_load*/, nullptr /* user */);
+ BRAVE_STARTUP_DATA_POST_PROFILE_POLICY_CONNECTOR
RegisterProfilePrefs(false /* is_signin_profile */,
chrome_feature_list_creator()->actual_locale(),