From 201e86dbe879cc1d38b6304ddfe4c8149f2248e3 Mon Sep 17 00:00:00 2001 From: Alexander Frick Date: Fri, 2 Aug 2024 12:53:16 -0500 Subject: [PATCH] fix crash and update pgo files --- infra/THORIUM_DEV_BOOKMARKS.html | 2 + src/build/config/compiler/pgo/BUILD.gn | 9 +++- src/chrome/browser/about_flags.cc | 2 +- .../chrome_download_manager_delegate.cc | 34 ++----------- .../download/download_target_determiner.cc | 15 ++++-- .../download/insecure_download_blocking.cc | 18 ++++--- src/chrome/browser/ui/views/tabs/tab_strip.cc | 11 +++-- src/tools/pgo/generate_profile.py | 48 +++++++++++++++---- 8 files changed, 81 insertions(+), 58 deletions(-) diff --git a/infra/THORIUM_DEV_BOOKMARKS.html b/infra/THORIUM_DEV_BOOKMARKS.html index dca2e1404..34c42c132 100644 --- a/infra/THORIUM_DEV_BOOKMARKS.html +++ b/infra/THORIUM_DEV_BOOKMARKS.html @@ -14,6 +14,7 @@

Bookmarks

THOR1

BUILD.gn - Chromium Code Search +
PGO BUILD.gn - Chromium Code Search
BUILD.gn - Chromium Code Search
BUILD.gn - Chromium Code Search
arm.gni - Chromium Code Search @@ -235,6 +236,7 @@

Bookmarks

home_button.cc - Chromium Code Search
side_panel_toolbar_button.cc - Chromium Code Search
ui_base_features.cc - Chromium Code Search +
allowlist.cc - Chromium Code Search

Chromium Code Search diff --git a/src/build/config/compiler/pgo/BUILD.gn b/src/build/config/compiler/pgo/BUILD.gn index 43f5357ca..d7ee34043 100644 --- a/src/build/config/compiler/pgo/BUILD.gn +++ b/src/build/config/compiler/pgo/BUILD.gn @@ -17,6 +17,12 @@ config("pgo_instrumentation_flags") { # are not required to be defined when we're not actually using PGO. if (chrome_pgo_phase == 1 && is_clang && !is_nacl && is_a_target_toolchain) { cflags = [ "-fprofile-generate" ] + if (temporal_pgo_profile) { + cflags += [ + "-mllvm", + "-pgo-temporal-instrumentation", + ] + } if (!is_win) { # Windows directly calls link.exe instead of the compiler driver when # linking, and embeds the path to the profile runtime library as @@ -146,11 +152,12 @@ config("pgo_optimization_flags") { ldflags = [ "-mllvm:-enable-ext-tsp-block-placement=1" ] } else if (is_linux) { ldflags = [ - "-Wl,-mllvm,-enable-ext-tsp-block-placement", + "-Wl,-mllvm,-enable-ext-tsp-block-placement=1", "-Wl,-mllvm,-enable-split-machine-functions", ] } else { ldflags = [ "-Wl,-mllvm,-enable-ext-tsp-block-placement=1" ] + ldflags = [ "-Wl,-mllvm,-enable-split-machine-functions" ] } } else { cflags += [ diff --git a/src/chrome/browser/about_flags.cc b/src/chrome/browser/about_flags.cc index c40b662d8..e8f8e18c9 100644 --- a/src/chrome/browser/about_flags.cc +++ b/src/chrome/browser/about_flags.cc @@ -3856,11 +3856,11 @@ const FeatureEntry::FeatureVariation kComposeProactiveNudgeVariations[] = { // Include Thorium Flags #include "chrome/browser/thorium_flag_choices.h" const FeatureEntry kFeatureEntries[] = { -#include "chrome/browser/thorium_flag_entries.h" // Include generated flags for flag unexpiry; see //docs/flag_expiry.md and // //tools/flags/generate_unexpire_flags.py. #include "build/chromeos_buildflags.h" #include "chrome/browser/unexpire_flags_gen.inc" +#include "chrome/browser/thorium_flag_entries.h" {variations::switches::kEnableBenchmarking, flag_descriptions::kEnableBenchmarkingName, flag_descriptions::kEnableBenchmarkingDescription, kOsAll, diff --git a/src/chrome/browser/download/chrome_download_manager_delegate.cc b/src/chrome/browser/download/chrome_download_manager_delegate.cc index 3b5535b10..9eff09837 100644 --- a/src/chrome/browser/download/chrome_download_manager_delegate.cc +++ b/src/chrome/browser/download/chrome_download_manager_delegate.cc @@ -1743,14 +1743,13 @@ bool ChromeDownloadManagerDelegate::IsOpenInBrowserPreferredForFile( return false; } -static const bool allow_insecure_downloads_3 = - base::CommandLine::ForCurrentProcess()->HasSwitch("allow-insecure-downloads"); - bool ChromeDownloadManagerDelegate::ShouldBlockFile( download::DownloadItem* item, download::DownloadDangerType danger_type) const { // Don't block downloads if flag is set - if (allow_insecure_downloads_3) { + static const bool allow_insecure_downloads_ = + base::CommandLine::ForCurrentProcess()->HasSwitch("allow-insecure-downloads"); + if (allow_insecure_downloads_) { return false; } // Chrome-initiated background downloads should not be blocked. @@ -1828,32 +1827,7 @@ void ChromeDownloadManagerDelegate::MaybeSendDangerousDownloadCanceledReport( DownloadItem* download, bool is_shutdown) { #if BUILDFLAG(FULL_SAFE_BROWSING) - if (!DownloadProtectionService::ShouldSendDangerousDownloadReport(download) || - !base::FeatureList::IsEnabled( - safe_browsing::kDownloadReportWithoutUserDecision)) { - return; - } - safe_browsing::SafeBrowsingService* sb_service = - g_browser_process->safe_browsing_service(); - if (!sb_service) { - return; - } - // Note: We cannot go through download_protection_service here, because this - // function may be called at shutdown. The download_protection_service - // object may already be deleted at this point. - if (is_shutdown) { - sb_service->PersistDownloadReportAndSendOnNextStartup( - download, - safe_browsing::ClientSafeBrowsingReportRequest:: - DANGEROUS_DOWNLOAD_PROFILE_CLOSED, - /*did_proceed=*/false, std::nullopt); - } else { - sb_service->SendDownloadReport( - download, - safe_browsing::ClientSafeBrowsingReportRequest:: - DANGEROUS_DOWNLOAD_AUTO_DELETED, - /*did_proceed=*/false, std::nullopt); - } + return; #endif } diff --git a/src/chrome/browser/download/download_target_determiner.cc b/src/chrome/browser/download/download_target_determiner.cc index b29a79d6e..67e2bfa12 100644 --- a/src/chrome/browser/download/download_target_determiner.cc +++ b/src/chrome/browser/download/download_target_determiner.cc @@ -975,6 +975,14 @@ DownloadTargetDeterminer::Result // result is available. danger_level_ = GetDangerLevel(NO_VISITS_TO_REFERRER); + static const bool allow_insecure_downloads_ = + base::CommandLine::ForCurrentProcess()->HasSwitch("allow-insecure-downloads"); + + // Continue with this Thorium flag + if (allow_insecure_downloads_) { + return CONTINUE; + } + if (danger_level_ == DownloadFileType::NOT_DANGEROUS) return CONTINUE; @@ -1265,10 +1273,11 @@ DownloadFileType::DangerLevel DownloadTargetDeterminer::GetDangerLevel( PriorVisitsToReferrer visits) const { DCHECK_CURRENTLY_ON(BrowserThread::UI); - // Allow all downloads with this Thorium flag - static const bool allow_insecure_downloads_2 = + static const bool allow_insecure_downloads_ = base::CommandLine::ForCurrentProcess()->HasSwitch("allow-insecure-downloads"); - if (allow_insecure_downloads_2) { + + // Allow all downloads with this Thorium flag + if (allow_insecure_downloads_) { return DownloadFileType::NOT_DANGEROUS; } diff --git a/src/chrome/browser/download/insecure_download_blocking.cc b/src/chrome/browser/download/insecure_download_blocking.cc index 568666379..98ccd7072 100644 --- a/src/chrome/browser/download/insecure_download_blocking.cc +++ b/src/chrome/browser/download/insecure_download_blocking.cc @@ -36,9 +36,6 @@ using InsecureDownloadStatus = download::DownloadItem::InsecureDownloadStatus; namespace { -static const bool allow_insecure_downloads_1 = - base::CommandLine::ForCurrentProcess()->HasSwitch("allow-insecure-downloads"); - // Configuration for which extensions to warn/block. These parameters are set // differently for testing, so the listed defaults are only used when the flag // is manually enabled (and in unit tests). @@ -325,6 +322,9 @@ struct InsecureDownloadData { !download_delivered_securely); } + static const bool allow_insecure_downloads_ = + base::CommandLine::ForCurrentProcess()->HasSwitch("allow-insecure-downloads"); + // Configure insecure download status. // Exclude download sources needed by Chrome from blocking. While this is // similar to MIX-DL above, it intentionally blocks more user-initiated @@ -337,7 +337,7 @@ struct InsecureDownloadData { download_source == DownloadSource::INTERNAL_API || download_source == DownloadSource::EXTENSION_API || download_source == DownloadSource::EXTENSION_INSTALLER || - allow_insecure_downloads_1) { + allow_insecure_downloads_) { is_insecure_download_ = false; } else { // Not ignorable download. // TODO(crbug.com/40857867): Add blocking metrics. @@ -456,13 +456,11 @@ InsecureDownloadStatus GetInsecureDownloadStatusForDownload( const download::DownloadItem* item) { InsecureDownloadData data(path, item); - // If the download is fully secure, early abort. - if (!data.is_insecure_download_) { - return InsecureDownloadStatus::SAFE; - } + static const bool allow_insecure_downloads_ = + base::CommandLine::ForCurrentProcess()->HasSwitch("allow-insecure-downloads"); - // Don't nag - if (allow_insecure_downloads_1) { + // If the download is fully secure, early abort. Don't nag + if (!data.is_insecure_download_ || allow_insecure_downloads_) { return InsecureDownloadStatus::SAFE; } diff --git a/src/chrome/browser/ui/views/tabs/tab_strip.cc b/src/chrome/browser/ui/views/tabs/tab_strip.cc index 5b9ecb815..6997aa301 100644 --- a/src/chrome/browser/ui/views/tabs/tab_strip.cc +++ b/src/chrome/browser/ui/views/tabs/tab_strip.cc @@ -1258,10 +1258,15 @@ bool TabStrip::ShouldDrawStrokes() const { theme_service->UsingSystemTheme(); } + // Don't want to have to run a full feature query every time this function is + // called. + static const bool force_enable_tab_outlines = + base::CommandLine::ForCurrentProcess()->HasSwitch("force-enable-tab-outlines"); + // The Tabstrip in the refreshed style does not meet the contrast ratio // requirements listed below but does not have strokes for Tabs or the bottom // border. - if (features::IsChromeRefresh2023() && !using_system_theme) { + if (!using_system_theme && !force_enable_tab_outlines) { return false; } @@ -1288,10 +1293,6 @@ bool TabStrip::ShouldDrawStrokes() const { return true; } - // Don't want to have to run a full feature query every time this function is - // called. - static const bool force_enable_tab_outlines = - base::CommandLine::ForCurrentProcess()->HasSwitch("force-enable-tab-outlines"); if (force_enable_tab_outlines) { return true; } diff --git a/src/tools/pgo/generate_profile.py b/src/tools/pgo/generate_profile.py index bc2e7ec9b..8172d9f59 100755 --- a/src/tools/pgo/generate_profile.py +++ b/src/tools/pgo/generate_profile.py @@ -80,6 +80,14 @@ def main(): help='Only run benchmarks that do not require any special access. See ' 'https://www.chromium.org/developers/telemetry/upload_to_cloud_storage/#request-access-for-google-partners ' 'for more information.') + parser.add_argument('--skip-jetstream', + action='store_true', + help='Jetstream2 is very flaky (60% pass rate), so it ' + 'is helpful to skip it when iterating fast locally.') + parser.add_argument( + '--temporal-trace-length', + type=int, + help='Add flags necessary for temporal PGO (experimental).') parser.add_argument('-v', '--verbose', action='count', @@ -159,9 +167,11 @@ def run_benchmark(benchmark_args): if os.path.exists(profiledir): shutil.rmtree(profiledir) - # Run the shortest benchmark first to fail early if anything is wrong. - run_benchmark(['speedometer2']) - run_benchmark(['jetstream2']) + # Run the shortest benchmarks first to fail early if anything is wrong. + run_benchmark(['speedometer3']) + + if not args.skip_jetstream: + run_benchmark(['jetstream2']) # These benchmarks require special access permissions: # https://www.chromium.org/developers/telemetry/upload_to_cloud_storage/#request-access-for-google-partners @@ -173,13 +183,35 @@ def run_benchmark(benchmark_args): run_benchmark([ 'rendering.mobile' if args.android_browser else 'rendering.desktop', '--also-run-disabled-tests', - '--story-tag-filter=motionmark_fixed_2_seconds' + '--story-tag-filter=motionmark_fixed_2_seconds', + '--story-filter-exclude=motionmark_fixed_2_seconds_images' ]) + if sys.platform == 'darwin': + run_benchmark([ + 'rendering.desktop', '--also-run-disabled-tests', + '--story-tag-filter=motionmark_fixed_2_seconds', + '--extra-browser-args=--enable-features=SkiaGraphite' + ]) + if not args.skip_profdata: - subprocess.run( - [PROFDATA, 'merge', '-o', f'{builddir}/profile.profdata'] + - glob.glob(f'{profiledir}/*.profdata'), - check=True) + merge_cmd = [PROFDATA, 'merge'] + if args.temporal_trace_length: + merge_cmd += [ + '--temporal-profile-max-trace-length', + str(args.temporal_trace_length) + ] + profile_output_path = f'{builddir}/profile.profdata' + merge_cmd += ['-o', profile_output_path] + subprocess.run(merge_cmd + glob.glob(f'{profiledir}/*.profdata'), + check=True) + + if args.temporal_trace_length: + orderfile_cmd = [ + PROFDATA, 'order', profile_output_path, '-o', + f'{builddir}/orderfile.txt' + ] + subprocess.run(orderfile_cmd, check=True) + if not args.keep_temps: shutil.rmtree(profiledir)