Enabled Performance features by default #17290

simonhong · 2023-02-20T06:29:52Z

fix brave/brave-browser#28615

Settings with device w/o battery

Settings with device with battery

When energy saver is on, toolbar has this button

Resolves

Submitter Checklist:

I confirm that no security/privacy review is needed, or that I have requested one
There is a ticket for my issue
Used Github auto-closing keywords in the PR description above
Wrote a good PR/commit description
Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
Checked the PR locally:
- npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
- npm run lint, npm run presubmit wiki, npm run gn_check, npm run tslint
Ran git rebase master (if needed)

Reviewer Checklist:

A security review is not needed, or a link to one is included in the PR description
New files have MPL-2.0 license header
Adequate test coverage exists to prevent regressions
Major classes, functions and non-trivial code blocks are well-commented
Changes in component dependencies are properly reflected in gn
Code follows the style guide
Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

BraveMainDelegateBrowserTest.EnabledFeatures
See the linked issue for manual testing

brave-builds · 2023-02-20T07:49:35Z

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

brave-builds · 2023-02-22T06:28:29Z

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

brave-builds · 2023-02-22T07:39:17Z

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

brave-builds · 2023-02-22T09:06:05Z

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

simonhong · 2023-02-22T12:28:27Z

There are some upstream test failures when performance features are enabled by default.
All of them are failed at the below point. Maybe code flow is changed with enabled features.
@goodov In this case, should we disable failed tests?

[2023-02-22T08:35:33.052Z] [104893:104893:FATAL:page_discarding_helper.cc(395)] Check failed: it != profiles_no_discard_patterns_.end(). 
[2023-02-22T08:35:33.052Z] #0 0x558b8df860e2 base::debug::CollectStackTrace()
[2023-02-22T08:35:33.052Z] #1 0x558b8de706c3 base::debug::StackTrace::StackTrace()
[2023-02-22T08:35:33.052Z] #2 0x558b8de8c46f logging::LogMessage::~LogMessage()
[2023-02-22T08:35:33.052Z] #3 0x558b8de8d2ee logging::LogMessage::~LogMessage()
[2023-02-22T08:35:33.052Z] #4 0x558b8e92690f performance_manager::policies::PageDiscardingHelper::IsPageOptedOutOfDiscarding()
[2023-02-22T08:35:33.052Z] #5 0x558b8e925fe8 performance_manager::policies::PageDiscardingHelper::CanUrgentlyDiscard()
[2023-02-22T08:35:33.052Z] #6 0x558b85d9af7a performance_manager::testing::MakePageNodeDiscardable()
[2023-02-22T08:35:33.052Z] #7 0x558b85d9ac9e performance_manager::testing::GraphTestHarnessWithMockDiscarder::SetUp()
[2023-02-22T08:35:33.052Z] #8 0x558b86434299 performance_manager::policies::HighEfficiencyModeTest::SetUp()
[2023-02-22T08:35:33.052Z] #9 0x558b88086da2 testing::Test::Run()

[2023-02-22T08:35:28.206Z] [104578:104578:FATAL:user_performance_tuning_manager.cc(98)] Check failed: g_user_performance_tuning_manager. 
[2023-02-22T08:35:28.206Z] #0 0x5638d251f0e2 base::debug::CollectStackTrace()
[2023-02-22T08:35:28.206Z] #1 0x5638d24096c3 base::debug::StackTrace::StackTrace()
[2023-02-22T08:35:28.206Z] #2 0x5638d242546f logging::LogMessage::~LogMessage(
[2023-02-22T08:35:28.206Z] #3 0x5638d24262ee logging::LogMessage::~LogMessage()
[2023-02-22T08:35:28.206Z] #4 0x5638d2ec4b80 performance_manager::user_tuning::UserPerformanceTuningManager::GetInstance()
[2023-02-22T08:35:28.206Z] #5 0x5638d2ba7489 performance_manager::user_tuning::IsRefreshRateThrottled()
[2023-02-22T08:35:28.206Z] #6 0x5638d2b70a27 UkmPageLoadMetricsObserver::OnStart()
[2023-02-22T08:35:28.206Z] #7 0x5638cbcbf8ff page_load_metrics::PageLoadTracker::InvokeAndPruneObservers()
[2023-02-22T08:35:28.206Z] #8 0x5638cbcbf69f page_load_metrics::PageLoadTracker::PageLoadTracker()
[2023-02-22T08:35:28.206Z] #9 0x5638cbca0eb5 page_load_metrics::MetricsWebContentsObserver::WillStartNavigationRequestImpl()
[2023-02-22T08:35:28.206Z] #10 0x5638cbca0c08 page_load_metrics::MetricsWebContentsObserver::WillStartNavigationRequest()
[2023-02-22T08:35:28.206Z] #11 0x5638cbc9f4be page_load_metrics::MetricsNavigationThrottle::WillStartRequest()

brave-builds · 2023-02-22T12:47:30Z

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

bsclifton · 2023-02-22T20:17:52Z

Clicking Learn more goes to https://support.brave.com/hc/en-us which is a bit weird

Chrome will send you here:
https://support.google.com/chrome/answer/12929150?visit_id=638126938240893485-1502329289&p=chrome_memory_saver&rd=1#memory_saver&zippy=%2Cturn-memory-saver-on-or-off

How should we handle this @rebron?

browser/resources/settings/brave_overrides/basic_page.ts

browser/resources/settings/brave_routes.ts

brave-builds · 2023-02-23T00:51:15Z

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

brave-builds · 2023-02-23T04:11:58Z

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

rebron · 2023-02-23T04:25:24Z

Clicking Learn more goes to https://support.brave.com/hc/en-us which is a bit weird

Chrome will send you here: https://support.google.com/chrome/answer/12929150?visit_id=638126938240893485-1502329289&p=chrome_memory_saver&rd=1#memory_saver&zippy=%2Cturn-memory-saver-on-or-off

How should we handle this @rebron?

We need a different url and stub article for this. @Brave-Matt Can you provide a placeholder url for @simonhong

simonhong · 2023-02-23T04:51:49Z

Clicking Learn more goes to https://support.brave.com/hc/en-us which is a bit weird
Chrome will send you here: https://support.google.com/chrome/answer/12929150?visit_id=638126938240893485-1502329289&p=chrome_memory_saver&rd=1#memory_saver&zippy=%2Cturn-memory-saver-on-or-off

How should we handle this @rebron?

We need a different url and stub article for this. @Brave-Matt Can you provide a placeholder url for @simonhong

We need two learn more urls for memory & battery saver
Chrome's battery saver learn more url - https://support.google.com/chrome/?p=chrome_battery_saver
Chrome's memory saver learn more url - https://support.google.com/chrome/?p=chrome_memory_saver

brave-builds · 2023-02-23T04:54:54Z

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

brave-builds · 2023-02-23T05:56:19Z

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

bsclifton

Changes LGTM; I'd defer to @brave/chromium-src-reviewers on how we'd want to tackle the tests. I think we can exclude them? Thanks for digging in and finding they are failing because of the default value

simonhong · 2023-02-23T07:10:19Z

Changes LGTM; I'd defer to @brave/chromium-src-reviewers on how we'd want to tackle the tests. I think we can exclude them? Thanks for digging in and finding they are failing because of the default value

d8248bd
I found that we have a filter list for it :)

Brave-Matt · 2023-02-24T00:30:05Z

I've got a stub article you can link to here:
Battery saver: https://support.brave.com/hc/en-us/articles/13380606172557
Memory saver: https://support.brave.com/hc/en-us/articles/13383683902733

simonhong · 2023-02-24T01:31:53Z

I've got a stub article you can link to here: Battery saver: https://support.brave.com/hc/en-us/articles/13380606172557 Memory saver: https://support.brave.com/hc/en-us/articles/13383683902733

@Brave-Matt It looks like these urls are internal only? I also can't load it

Brave-Matt · 2023-02-24T03:30:50Z

I've got a stub article you can link to here: Battery saver: https://support.brave.com/hc/en-us/articles/13380606172557 Memory saver: https://support.brave.com/hc/en-us/articles/13383683902733

@Brave-Matt It looks like these urls are internal only? I also can't load it

Yes these are stub articles (not content yet — will start this tomorrow) but those links will work once I push live

brave-builds · 2023-02-24T13:17:10Z

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

fix brave/brave-browser#28615 * kBatterySaverModeAvailable * kHighEfficiencyModeAvailable Deleted brave://settings/performance route. brave://settings/system page has performance section. Battery setting is only visible when device has battery.

brave-builds · 2023-02-24T15:49:01Z

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

simonhong · 2023-03-08T05:44:57Z

I've got a stub article you can link to here: Battery saver: https://support.brave.com/hc/en-us/articles/13380606172557 Memory saver: https://support.brave.com/hc/en-us/articles/13383683902733

@Brave-Matt It looks like these urls are internal only? I also can't load it

Yes these are stub articles (not content yet — will start this tomorrow) but those links will work once I push live

@Brave-Matt Both urls are public and they are in mobile/android category.
but they're for desktop. Let me know if we need to change their urls.

simonhong added the CI/run-draft label Feb 20, 2023

simonhong self-assigned this Feb 20, 2023

github-actions bot added CI/run-network-audit Run network-audit CI/storybook-url Deploy storybook and provide a unique URL for each build labels Feb 20, 2023

simonhong removed the CI/run-draft label Feb 21, 2023

simonhong force-pushed the enable_kHighEfficiencyModeAvailable_desktop branch from 78e87d1 to c47edf7 Compare February 21, 2023 02:17

simonhong changed the title ~~Enabled kHighEfficiencyModeAvailable feature by default~~ Enabled Performance features by default Feb 22, 2023

simonhong added the CI/run-draft label Feb 22, 2023

simonhong force-pushed the enable_kHighEfficiencyModeAvailable_desktop branch 2 times, most recently from 4d692eb to 33467cc Compare February 22, 2023 05:42

simonhong marked this pull request as ready for review February 22, 2023 06:29

simonhong requested a review from a team as a code owner February 22, 2023 06:29

simonhong force-pushed the enable_kHighEfficiencyModeAvailable_desktop branch from 33467cc to aae4ead Compare February 22, 2023 06:31

simonhong requested review from spylogsster, petemill and bsclifton February 22, 2023 06:32

simonhong force-pushed the enable_kHighEfficiencyModeAvailable_desktop branch 2 times, most recently from 446a299 to 3a64c47 Compare February 22, 2023 08:23

bsclifton reviewed Feb 22, 2023

View reviewed changes

browser/resources/settings/brave_overrides/basic_page.ts Outdated Show resolved Hide resolved

bsclifton reviewed Feb 22, 2023

View reviewed changes

browser/resources/settings/brave_routes.ts Show resolved Hide resolved

simonhong force-pushed the enable_kHighEfficiencyModeAvailable_desktop branch from 2f3638a to 47e7f95 Compare February 23, 2023 03:10

simonhong force-pushed the enable_kHighEfficiencyModeAvailable_desktop branch from 47e7f95 to d8248bd Compare February 23, 2023 04:14

bsclifton approved these changes Feb 23, 2023

View reviewed changes

iefremov approved these changes Feb 24, 2023

View reviewed changes

simonhong force-pushed the enable_kHighEfficiencyModeAvailable_desktop branch from 544766a to 80c240a Compare February 24, 2023 14:51

simonhong force-pushed the enable_kHighEfficiencyModeAvailable_desktop branch from 80c240a to aab42bd Compare February 24, 2023 15:03

simonhong merged commit 30b403d into master Feb 25, 2023

simonhong deleted the enable_kHighEfficiencyModeAvailable_desktop branch February 25, 2023 00:33

github-actions bot added this to the 1.50.x - Nightly milestone Feb 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enabled Performance features by default #17290

Enabled Performance features by default #17290

simonhong commented Feb 20, 2023 •

edited

Loading

brave-builds commented Feb 20, 2023

brave-builds commented Feb 22, 2023

brave-builds commented Feb 22, 2023

brave-builds commented Feb 22, 2023

simonhong commented Feb 22, 2023

brave-builds commented Feb 22, 2023

bsclifton commented Feb 22, 2023

brave-builds commented Feb 23, 2023

brave-builds commented Feb 23, 2023

rebron commented Feb 23, 2023

simonhong commented Feb 23, 2023 •

edited

Loading

brave-builds commented Feb 23, 2023

brave-builds commented Feb 23, 2023

bsclifton left a comment

simonhong commented Feb 23, 2023

Brave-Matt commented Feb 24, 2023 •

edited

Loading

simonhong commented Feb 24, 2023

Brave-Matt commented Feb 24, 2023

brave-builds commented Feb 24, 2023

brave-builds commented Feb 24, 2023

simonhong commented Mar 8, 2023 •

edited

Loading

Enabled Performance features by default #17290

Enabled Performance features by default #17290

Conversation

simonhong commented Feb 20, 2023 • edited Loading

Submitter Checklist:

Reviewer Checklist:

After-merge Checklist:

Test Plan:

brave-builds commented Feb 20, 2023

brave-builds commented Feb 22, 2023

brave-builds commented Feb 22, 2023

brave-builds commented Feb 22, 2023

simonhong commented Feb 22, 2023

brave-builds commented Feb 22, 2023

bsclifton commented Feb 22, 2023

brave-builds commented Feb 23, 2023

brave-builds commented Feb 23, 2023

rebron commented Feb 23, 2023

simonhong commented Feb 23, 2023 • edited Loading

brave-builds commented Feb 23, 2023

brave-builds commented Feb 23, 2023

bsclifton left a comment

Choose a reason for hiding this comment

simonhong commented Feb 23, 2023

Brave-Matt commented Feb 24, 2023 • edited Loading

simonhong commented Feb 24, 2023

Brave-Matt commented Feb 24, 2023

brave-builds commented Feb 24, 2023

brave-builds commented Feb 24, 2023

simonhong commented Mar 8, 2023 • edited Loading

simonhong commented Feb 20, 2023 •

edited

Loading

simonhong commented Feb 23, 2023 •

edited

Loading

Brave-Matt commented Feb 24, 2023 •

edited

Loading

simonhong commented Mar 8, 2023 •

edited

Loading