Skip to content
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

build: work around ScreenCaptureKit bad feature flag parsing in Chromium #41622

Merged
merged 1 commit into from Mar 19, 2024

Conversation

codebytere
Copy link
Member

@codebytere codebytere commented Mar 18, 2024

Description of Change

Ref #41397.

Details

[64768:0318/145439.936909:FATAL:feature_list.cc(873)] Check failed: trial. trial='StudyThumbnailCapturerMac' does not exist
0   Electron Framework                  0x000000011cb39cc0 base::debug::CollectStackTrace(void const**, unsigned long) + 28
1   Electron Framework                  0x000000011cb29a20 base::debug::StackTrace::StackTrace() + 24
2   Electron Framework                  0x000000011ca57b40 logging::LogMessage::Flush() + 132
3   Electron Framework                  0x000000011ca57a3c logging::LogMessage::~LogMessage() + 36
4   Electron Framework                  0x000000011ca4200c logging::(anonymous namespace)::DCheckLogMessage::~DCheckLogMessage() + 124
5   Electron Framework                  0x000000011ca4205c logging::(anonymous namespace)::DCheckLogMessage::~DCheckLogMessage() + 12
6   Electron Framework                  0x000000011ca41ae0 logging::CheckError::~CheckError() + 44
7   Electron Framework                  0x000000011ca41b48 logging::CheckError::~CheckError() + 12
8   Electron Framework                  0x000000011ca46734 base::FeatureList::RegisterOverridesFromCommandLine(std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&, base::FeatureList::OverrideState) + 404
9   Electron Framework                  0x000000011ca46380 base::FeatureList::InitFromCommandLine(std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&) + 1708
10  Electron Framework                  0x000000011ca48474 base::FeatureList::InitInstance(std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&, std::__Cr::vector<std::__Cr::pair<std::__Cr::reference_wrapper<base::Feature[abi:logically_const] const> const, base::FeatureList::OverrideState>, std::__Cr::allocator<std::__Cr::pair<std::__Cr::reference_wrapper<base::Feature[abi:logically_const] const> const, base::FeatureList::OverrideState>>> const&) + 180
11  Electron Framework                  0x000000011ca483b4 base::FeatureList::InitInstance(std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&) + 28
12  Electron Framework                  0x0000000117aff9f0 electron::InitializeFeatureList() + 596
13  Electron Framework                  0x00000001179be050 electron::ElectronMainDelegate::PreBrowserMain() + 12
14  Electron Framework                  0x0000000117da13ac content::ContentMainRunnerImpl::RunBrowser(content::MainFunctionParams, bool) + 912
15  Electron Framework                  0x0000000117da0fac content::ContentMainRunnerImpl::Run() + 1424
16  Electron Framework                  0x0000000117d9e9ac content::RunContentProcess(content::ContentMainParams, content::ContentMainRunner*) + 1192
17  Electron Framework                  0x0000000117d9ebf0 content::ContentMain(content::ContentMainParams) + 112
18  Electron Framework                  0x00000001179bacb4 ElectronMain + 320
19  dyld                                0x000000019c13e0e0 start + 2360
Crash keys:
  "platform" = "darwin"
  "process_type" = "browser"

Electron exited with signal SIGTRAP.

This works fine in release builds but bombs in testing because of the way feature flags are currently parsed:

// If a feature appears on both lists, then it will be disabled. If
// a list entry has the format "FeatureName<TrialName" then this
// initialization will also associate the feature state override with the
// named field trial, if it exists. If a list entry has the format
// "FeatureName:k1/v1/k2/v2", "FeatureName<TrialName:k1/v1/k2/v2" or
// "FeatureName<TrialName.GroupName:k1/v1/k2/v2" then this initialization will
// also associate the feature state override with the named field trial and
// its params. If the feature params part is provided but trial and/or group
// isn't, this initialization will also create a synthetic trial, named
// "Study" followed by the feature name, i.e. "StudyFeature", and group, named
// "Group" followed by the feature name, i.e. "GroupFeature", for the params.
// If a feature name is prefixed with the '*' character, it will be created
// with OVERRIDE_USE_DEFAULT - which is useful for associating with a trial
// while using the default state.

To fix this for now and unblock local development on macOS 14.4, only set it in release.

Checklist

Release Notes

Notes: none

@codebytere codebytere changed the title build: unblock bad feature flag parsing build: unblock ScreenCaptureKit bad feature flag parsing in Chromium Mar 18, 2024
@codebytere codebytere changed the title build: unblock ScreenCaptureKit bad feature flag parsing in Chromium build: work around ScreenCaptureKit bad feature flag parsing in Chromium Mar 18, 2024
@codebytere codebytere added the fast-track 🚅 Indicates that this PR is intended to bypass the 24 hour rule. Needs approval from Releases label Mar 19, 2024
@codebytere codebytere merged commit 1cd7419 into main Mar 19, 2024
17 of 19 checks passed
@codebytere codebytere deleted the fix-main-bombing branch March 19, 2024 09:49
Copy link

release-clerk bot commented Mar 19, 2024

No Release Notes

@jkleinsc
Copy link
Contributor

/trop run backport-to 30-x-y

@trop
Copy link
Contributor

trop bot commented Apr 24, 2024

The backport process for this PR has been manually initiated - sending your PR to 30-x-y!

@jkleinsc
Copy link
Contributor

/trop run backport to 29-x-y

@trop
Copy link
Contributor

trop bot commented Apr 24, 2024

I have automatically backported this PR to "30-x-y", please check out #41961

@jkleinsc jkleinsc added target/29-x-y PR should also be added to the "29-x-y" branch. and removed no-backport labels Apr 24, 2024
@jkleinsc
Copy link
Contributor

/trop run backport-to 29-x-y

@trop
Copy link
Contributor

trop bot commented Apr 24, 2024

The backport process for this PR has been manually initiated - sending your PR to 29-x-y!

@trop
Copy link
Contributor

trop bot commented Apr 24, 2024

I have automatically backported this PR to "29-x-y", please check out #41962

@trop trop bot added in-flight/29-x-y merged/29-x-y PR was merged to the "29-x-y" branch. merged/30-x-y PR was merged to the "30-x-y" branch. and removed in-flight/29-x-y in-flight/30-x-y labels Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fast-track 🚅 Indicates that this PR is intended to bypass the 24 hour rule. Needs approval from Releases merged/29-x-y PR was merged to the "29-x-y" branch. merged/30-x-y PR was merged to the "30-x-y" branch. semver/none target/29-x-y PR should also be added to the "29-x-y" branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants