Skip to content

Remove stats usage ping disabled by policy pref, rename policy #30514

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: origin-pref-cleanup1
Choose a base branch
from

Conversation

DJAndries
Copy link
Collaborator

@DJAndries DJAndries commented Aug 7, 2025

@DJAndries DJAndries requested review from bbondy and bsclifton August 7, 2025 00:52
@github-actions github-actions bot added the CI/storybook-url Deploy storybook and provide a unique URL for each build label Aug 7, 2025
@DJAndries DJAndries changed the title Remove stats usage ping disabled by policy pref Remove stats usage ping disabled by policy pref, rename policy Aug 7, 2025
@brave-builds
Copy link
Collaborator

Warning

You have got a presubmit warning. Please address it if possible.

components/brave_stats/browser/brave_stats_updater_util.h: PrefService forward declaration is no longer needed

@brave-builds
Copy link
Collaborator

Warning

You have got a presubmit warning. Please address it if possible.

Discovered possible removal of preference registrations.

Please make sure to properly deprecate preferences by clearing their
value for a couple of milestones before finally removing the code.
Otherwise data may stay in the preferences files forever. See
Migrate*Prefs() in chrome/browser/prefs/browser_prefs.cc and
chrome/browser/prefs/README.md for examples.
This may be a false positive warning (e.g. if you move preference
registrations to a different place).

Items:

browser/brave_stats/brave_stats_updater.cc: -  registry->RegisterBooleanPref(kStatsReportingDisabledByPolicy, false);

void BravePrivacyHandler::OnStatsUsagePingEnabledChanged() {
if (IsJavascriptAllowed()) {
PrefService* local_state = g_browser_process->local_state();
bool user_enabled = local_state->GetBoolean(kStatsReportingEnabled);
bool policy_disabled =
local_state->GetBoolean(kStatsReportingDisabledByPolicy);
bool hidden = IsStatsReportingHidden();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, after some discussion with @bbondy I think I understand what we're doing here now. I thought we were using brave origin to set the managed pref value, but the two things are independent so ideally we really would have split them into two PRs. Still working through some details to figure out what makes the most sense here

Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++

@DJAndries DJAndries force-pushed the origin-pref-cleanup2 branch from 9d83e51 to 3565f94 Compare August 7, 2025 21:13
@DJAndries DJAndries force-pushed the origin-pref-cleanup2 branch from 3565f94 to 4044766 Compare August 7, 2025 22:31
@brave-builds
Copy link
Collaborator

Warning

You have got a presubmit warning. Please address it if possible.

Found 3 lines longer than 80 characters (first 5 shown).

Items:

browser/resources/settings/brave_data_collection_page/brave_data_collection_page.ts, line 102, 86 chars
browser/resources/settings/brave_data_collection_page/brave_data_collection_page.ts, line 113, 86 chars
browser/resources/settings/brave_data_collection_page/brave_data_collection_page.ts, line 118, 97 chars

@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/storybook-url Deploy storybook and provide a unique URL for each build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants