Enable support for CEF 6613+ (Chrome 128 to 150) and Chrome Runtime #523
Labels
No labels
area/accessibility
area/dependency
Confirmed
Crash Report
Delayed
Design Issue
Duplicate
Feedback required
Good first issue
kind/bug
kind/cleanup
kind/enhancement
platform/linux
platform/macos
platform/windows
priority/high
Question
Request for Comments
Seeking Other Contributors
Seeking Testers
Won't Fix / By Design
No milestone
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
gh-obsproject/obs-browser!523
Loading…
Reference in a new issue
No description provided.
Delete branch "chrome-runtime-changes"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Description
We made it! After a lot of work, burnout, and testing, support for CEF 6613 (Chrome 128) and the Chrome Runtime is ready to be reviewed!
This (finally) gives us the ability to safely migrate to the Chrome Runtime, the replacement to the Alloy Runtime that CEF has been using forever. CEF/Chrome versions from 6613/128 to 7871/150 (yes, current stable) are now fully supported and work out of the box.
Test builds of CEF can be downloaded from https://cef-builds.spotifycdn.com/index.html (use Minimal, Current Stable)
Add a dummy Browser Client to introduce limitations into the Chrome UI windows that can be opened in uncontrolled edge cases. Chrome UI windows are unmanaged by default. By returning something other than null in
GetDefaultClient()we can lock it down in various ways. https://magpcss.org/ceforum/viewtopic.php?f=6&t=19898OnBeforePopup,OnOpenURLFromTabin the Dummy Client can probably be removed as they don't get called. There's no direct way to stop the Chrome UI window from being opened, so instead close it immediately. However, I've kept them just in case.Disable Various Chrome Settings in custom docks & browser sources via
SetPreferences. For more, checkchrome/common/pref_names.h. We should actively keep an eye on this in future when updating CEF.Chrome's default error display is now used by CEF, so on top of the existing override in browser docks, a similar override has been added to browser sources which just redirects to about:blank.
Also blocks
A data migration function is included. It moves some directories into a new 'Default' profile subdirectory, and performs the necessary renaming of certain files to ensure the new Profile loads. This is necessary for user's data to be maintained, as the Alloy runtime used a bit of a custom structure which has been discarded in favour of Chrome Runtime's. CEF does not provide a migration path.
Additionally, the cookie directories for each service integration must be moved into the root config directory, as subdirectories in their current form (
obs_profile_cookies/<cookie_id>) are not supported. This is intentional, and according to Marshall this was never a supported setup. cache_path must be a direct child of the root_cache_path. Invalid cache_path_ is silently treated as Incognito Mode and cookies are not stored.To work around this, I've included some temporary code to translate the path received from the OBS frontend. When this PR is to be merged, I can submit a PR to obs-studio to match the intended behaviour.
Originally, I tracked down a number of crashes when loading various
chrome://urls in the new runtime. This also exposed the fact that users would be able to access pages we didn't want them to see, such aschrome://settings. My original implementation therefore added a filter to block such pages from loading. Since then, CEF added its own whitelist to protect users from crashes, so my PR in its current form no longer has such protection. If it comes up, I can re-implement such protections in the future.Motivation and Context
Keeping CEF up to date is important - for security, reliability, support, and access to modern features.
How Has This Been Tested?
Tested on Windows 10 on CEF 147 with a
plugin_config/obs-browserconfig directory from OBS 32.1 to ensure data is migrated correctly. Launched again after migration to ensure data was still accessible.cef_binary_147.0.14+g76d2442+chromium-147.0.7727.138_windows64_minimalTested again with our current CEF 127 build to ensure we can use 6533 unchanged if CEF tests themselves backfire.
Additionally tested over the past few months with the following versions as they were released. No regressions were found. Mostly my fault for taking so long.
Additionally, @hoshinolina has done tremendous work testing this on Fedora, with users actively using the code in this PR since November 2025. As far as I know, users have had very few complaints.
Types of changes
Checklist:
For reference, the only issues we've run into in Fedora as far as I remember have been:
EXPERIMENTALAPI version (which is the default), this should not affect it (in all cases, rebuilding OBS against the current version/API of CEF would have fixed/worked around the ABI break we saw).So overall, nothing really relevant to this PR. That said, I should point out that Fedora never shipped nor tested the profile migration code. While Fedora did briefly ship the old obs-cef at one point in the past, that was abandoned as unmaintainable (making it build with newer compilers/libs became a nightmare), so there was a period where OBS had no CEF support in Fedora before this was reintroduced, and therefore no good reason to worry about profile migrations (it would only benefit people who had used the old version, to revive their old dormant profiles, which we figured wasn't really worth it).
If anyone wants to build this PR in Fedora against system CEF, the only required changes should be:
cef-develrm cmake/finders/FindCEF.cmake(cef-devel provides this systemwide, interface-compatible with the OBS version)LD_LIBRARY_PATH=/usr/lib64/cefor change the rpath setting:sed -e 's,INSTALL_RPATH ".*",INSTALL_RPATH "/usr/lib64/cef/",' -i plugins/obs-browser/cmake/os-linux.cmake-DENABLE_BROWSER=ONand optionally,-DCEF_API_VERSION=xxxxxif you want to build against a specific versioned API/ABI (Fedora arbitrarily picks 13700 here, but I don't think CEF has actually changed any APIs in a source-incompatible way with OBS since 13300, the very first version, so you can pick whatever right now).Note that unlike the standard OBS build, this setup embeds the
libcef_dll_wrapperbuild into the OBS build, so you will see that being built alongside the rest of obs-browser (the wrapper source code is shipped incef-devel). All that is taken care of by the system FindCEF.cmake and the way the CEF package itself is patched/configured, and in fact it turned out to be quite easy to do and it works very well. We settled on this approach after trying to pre-build the wrapper incef-devel, because we realized that it was impractical to build a separate wrapper for every possible version and additionally, consumers may want to build the wrapper with their own choice of compile-time options making "universal" wrapper builds not really a good idea (upstream CEF confirmed this too). Relevant links: FindCEF.cmake cef.spec (search for libcef_dll for the few relevant lines). I would personally encourage OBS to consider exploring this approach instead of a separate libcef_dll_wrapper build for official binaries/CI, but that's just me (and it's not a blocker for this PR in any way) ^^@ -41,2 +45,4 @@#endifCefRefPtr<CefRenderProcessHandler> BrowserApp::GetRenderProcessHandler(){Use
constexprfor the constants:I also wonder if this should maybe be wrapped in a lambda for each value, because any error while disabling this feature should be considered a critical failure (it would leave the browser with partially enabled Chromium runtime features which in turn could lead to unexpected behaviour and related support burden).
@ -56,0 +75,4 @@CefRefPtr<CefValue> optionValue = CefValue::Create();constexpr std::array<std::string_view, 20> kBrowserFeaturesToDisable{"autofill.credit_card_enabled",What's the purpose of the "dummy" client? Some comment explaining the technical decision making process behind it would be helpful.
@ -626,6 +626,17 @@ void BrowserClient::OnLoadEnd(CefRefPtr<CefBrowser>, CefRefPtr<CefFrame> frame,}Some high-level explanation of the issue in addition to the issue link would be helpful. Also a
FIXMEorTODOlead-in would be great as most IDEs and syntax-highlighters detect those.@ -293,6 +293,9 @@ static void BrowserInit(void)CefString(&settings.log_file) = log_path_abs;What's the reason to guard this against versions 7103 and up?
@ -714,8 +720,122 @@ static void check_hwaccel_support(void)#endifThis is entirely new C++ code, so I'd prefer if we'd use standard C++ library types and functions if available (e.g.
std::string,std::string_view,std::filesystem, etc.) rather than custom OBS types and APIs and treating it as "C with some OOP".(And even though
stdis included by default, this would be a violation of our C++ code style standards, so usestd::stringexplicitly so this code does not need to be touched whenusing namespace stdhopefully is excised at a later point).Also be mindful of any
libobsAPI that potentially returnsnullptrinstead of an actualchar *and guard against that.Use
constexprfor the constants:Which enables:
I don't know if I like the design with this function and
MigrateProfileConfigDirrather than one single function. I'm kinda fine with using string concatenation at first rather than usingstd::filesystem::pathobjects as any string token would have to be converted usingu8pathfirst due to Windows using UTF-16 for paths internally.My hunch is that this might break in unforeseen ways as you pass in a
std::stringwith UTF-8 bytes into a function that expects UTF-16 bytes instead.Create a task issue and add a
TODOcomment with a link to that issue here (outlining the issue and what needs to be changed in that issue).how would I test this on arch? (sorry, really new to arch at the moment)
@ -41,2 +45,4 @@#endifCefRefPtr<CefRenderProcessHandler> BrowserApp::GetRenderProcessHandler(){Forgot to mention - as per our code style guidelines those
constexprwould also have to use thekprefix (kBrowserFeaturesToDisable).It’s very nice to see this 🎉
Also – @hoshinolina: thanks for the CEF package in Fedora, it was very helpful when creating the Arch package ^^
@Phr3d13: I pushed
obs-studio 32.1.2-5andobs-studio-plugin-browser 32.1.2-5with these changes to the [extra-testing] repository. Look at the Arch Wiki on how to use the testing repositories. Though be warned, that will install all testing packages, not just obs.@ -41,2 +45,4 @@#endifCefRefPtr<CefRenderProcessHandler> BrowserApp::GetRenderProcessHandler(){Easy enough. Can do.
@ -56,0 +75,4 @@CefRefPtr<CefValue> optionValue = CefValue::Create();constexpr std::array<std::string_view, 20> kBrowserFeaturesToDisable{"autofill.credit_card_enabled",I documented it in the commit description and the PR description, but I'll add the note in code as well.
@ -626,6 +626,17 @@ void BrowserClient::OnLoadEnd(CefRefPtr<CefBrowser>, CefRefPtr<CefFrame> frame,}Can do.
@ -293,6 +293,9 @@ static void BrowserInit(void)CefString(&settings.log_file) = log_path_abs;During the transition period, CEF shipped with a toggle. After CEF 128 (6613), it was no longer needed because the Alloy Runtime was entirely removed - therefore the toggle would error. This ensures compatibility with intermediary versions, though if we wanted to we could lock it purely to 6613. I left it in because it was, on occasion, useful in 6533 (127, what we currently ship) to test with.
@ -714,8 +720,122 @@ static void check_hwaccel_support(void)#endifHmm, probably. Do we have documentation on how best to transfer from
obs_module_config_pathtostd::filesystemcleanly? I have tried and failed to find out. I'd guess the same way I didold_patha few lines earlier? Just seems a bit noisy going obs_module_config_path -> os_get_abs_path_ptr ->std::filesystem::u8path, unless theos_get_abs_path_ptrcan be skipped?Same question as above for how best to transition from
obs_module_config_pathhere, as I don't think we have any existing examples in the OBS codebase.Unfortunately,
MigrateProfileConfigDiris used both for migrating the Default profile, and later to migrate individual Profile cookie directories, so it has to be reusable.Good point on catching
nullptrs. I'll revisit.Can do.
@ -293,6 +293,9 @@ static void BrowserInit(void)CefString(&settings.log_file) = log_path_abs;If it leads to an error starting with 6613, shouldn't it then be limited to versions before 6613 and not potentially lead to errors for all versions up to 7103?
@ -714,8 +720,122 @@ static void check_hwaccel_support(void)#endifWithout consulting a compiler, I'd probably do:
On macOS
std::filesystemseems to handle absolute paths with relative path tokens just fine (so something like/Path/../SomePathworks), otherwisestd::filesystem::canonicalcan be used to resolve such a path completely, which you'd probably wrap the assignment tooldPathandnewPathin - but I'm not sure that's necessary.I kinda agree with @notr1ch that we don't need to use
nullptrchecks if the occurrence of it would be a truly exceptional situation (as in: this should never happen and if it does, some developer messed up).This would be in opposition to an API that could very well return a
nullptrand that's part of its "normal operation".The last consideration is whether even the exceptional situation is indeed "recoverable" or doing so would allow OBS to limp on as a program in a weird, undefined, state, which could lead to data corruption or data loss as a result.
Just crashing (to avoid that) is incredibly disruptive, but preferable to e.g. potentially overwriting a scene collection with garbage data.
@ -293,6 +293,9 @@ static void BrowserInit(void)CefString(&settings.log_file) = log_path_abs;Yeah, I re-checked the CEF docs and it's gone as early as 6613, so I've updated the check.
@ -714,8 +720,122 @@ static void check_hwaccel_support(void)#endifDone. Let me know if there are additional concerns outside of the lack of u8path handling due to C++20 deprecation warnings.
@ -714,8 +720,122 @@ static void check_hwaccel_support(void)#endifYep, that code worked pretty much out of the box - feel free to look over my changes. I have verified
std::filesystem::canonicalwas not necessary (though it could be used to make error logging a bit clearer).Changes made, outside of properly handing u8paths for now.
@ -626,6 +626,17 @@ void BrowserClient::OnLoadEnd(CefRefPtr<CefBrowser>, CefRefPtr<CefFrame> frame,}Done.
@ -56,0 +75,4 @@CefRefPtr<CefValue> optionValue = CefValue::Create();constexpr std::array<std::string_view, 20> kBrowserFeaturesToDisable{"autofill.credit_card_enabled",Note added.
@ -41,2 +45,4 @@#endifCefRefPtr<CefRenderProcessHandler> BrowserApp::GetRenderProcessHandler(){Changes made outside of swapping to a lambda. I'll need to think on it some more, as majority of the features being disabled are mostly to avoid quirks. At the very least we could log the ones that fail to apply so we know to fix them. Happy to revisit.
C strings leaks…
@ -714,8 +720,122 @@ static void check_hwaccel_support(void)#endifLeaks the C string
Leaks the C string
Leaks the C string
Leaks the C string
Leaks the C string
Issue met while building on Arch and Flatpak.
@ -106,5 +109,9 @@ public:QTimer frameTimer;@ -68,10 +75,18 @@ CefRefPtr<CefJSDialogHandler> QCefBrowserClient::GetJSDialogHandler()return this;Unused params
@ -714,8 +720,122 @@ static void check_hwaccel_support(void)#endifFixed using my original solution. If Pat knows of a cleaner way, I'll make the additional adjustment.
@ -714,8 +720,122 @@ static void check_hwaccel_support(void)#endifFixed.
@ -714,8 +720,122 @@ static void check_hwaccel_support(void)#endifFixed.
@ -68,10 +75,18 @@ CefRefPtr<CefJSDialogHandler> QCefBrowserClient::GetJSDialogHandler()return this;Oops, not sure how I missed that one. Fixed.
@ -106,5 +109,9 @@ public:QTimer frameTimer;I don't fully understand the error message itself, but I think I've fixed it. Please confirm.
@ -106,5 +109,9 @@ public:QTimer frameTimer;It works
@ -106,5 +109,9 @@ public:QTimer frameTimer;The type was
BrowserDummyClient *constbut the function returnBrowserDummyClient *so mismatch, theconstqualifier was assigned to the type not to the function.@ -56,0 +75,4 @@CefRefPtr<CefValue> optionValue = CefValue::Create();constexpr std::array<std::string_view, 20> kBrowserFeaturesToDisable{"autofill.credit_card_enabled",Requires use of C++-style comments and not C-style comments, per code style guidelines.
@ -56,0 +103,4 @@for (std::string_view feature : kBrowserFeaturesToDisable) {requestContext->SetPreference(feature.data(), optionValue.get(), errorMessage);}I guess this is
clang-format's doing? 🙈@ -626,6 +626,17 @@ void BrowserClient::OnLoadEnd(CefRefPtr<CefBrowser>, CefRefPtr<CefFrame> frame,}Wouldn't be sufficient to decorate the
frameargument with[[maybe_unused]]?@ -714,8 +720,122 @@ static void check_hwaccel_support(void)#endifPrefer direct-list-initialization to catch possible narrowing conversions.
See above.
Replace with
constexpr std::string_viewconstant.What does this comment relate to? The call to
MigrateProfileConfigDir?Makes it easier to debug the code and keeps the amount of temporaries low.
The existing implementation is equivalent to:
Which potentially involves a whole lot of copies after reallocations, and at least according to compiler explorer the resulting code uses about twice as many instructions even with
-O3optimisation.@ -717,2 +832,4 @@#endifbool obs_module_load(void){Wouldn't it make more sense to move this check to before extracting the string (which will create temporary path objects on the way there) and bail out early?
Is also missing a link to the task issue on the main repo.
Applies below as well.
@ -626,6 +626,17 @@ void BrowserClient::OnLoadEnd(CefRefPtr<CefBrowser>, CefRefPtr<CefFrame> frame,}It would be the first instance in the entire OBS codebase, but I'm not against trying something new!
@ -56,0 +103,4 @@for (std::string_view feature : kBrowserFeaturesToDisable) {requestContext->SetPreference(feature.data(), optionValue.get(), errorMessage);}It is, yes. :)
@ -714,8 +720,122 @@ static void check_hwaccel_support(void)#endifIt was a leftover comment with the intent of describing why this particular path doesn't manually copy Local Storage, Session Storage or Network, but thinking on it now it's unnecessary.
@ -714,8 +720,122 @@ static void check_hwaccel_support(void)#endif@ -714,8 +720,122 @@ static void check_hwaccel_support(void)#endifDone.
@ -714,8 +720,122 @@ static void check_hwaccel_support(void)#endifDone.
@ -714,8 +720,122 @@ static void check_hwaccel_support(void)#endifDone.
@ -714,8 +720,122 @@ static void check_hwaccel_support(void)#endifDone. Switched to using
.data()for the libobs call.@ -714,8 +720,122 @@ static void check_hwaccel_support(void)#endifDone.
@ -717,2 +832,4 @@#endifbool obs_module_load(void){You're right. Fixed.
@ -714,8 +720,122 @@ static void check_hwaccel_support(void)#endifUnfortunate, I love a one-liner. Fixed.
@ -714,8 +720,122 @@ static void check_hwaccel_support(void)#endifDone.
@ -56,0 +75,4 @@CefRefPtr<CefValue> optionValue = CefValue::Create();constexpr std::array<std::string_view, 20> kBrowserFeaturesToDisable{"autofill.credit_card_enabled",Oops, my JavaScript Developer is showing. Fixed.
@ -626,6 +626,17 @@ void BrowserClient::OnLoadEnd(CefRefPtr<CefBrowser>, CefRefPtr<CefFrame> frame,}Seems to build fine on both Windows and Linux, therefore marking as resolved.
Took me a bit to figure out how to create a Task. Done.
@ -626,6 +626,17 @@ void BrowserClient::OnLoadEnd(CefRefPtr<CefBrowser>, CefRefPtr<CefFrame> frame,}Done, and created Task.
Looks fine otherwise. Haven't given this a test on my machine yet though, will do so soon.
@ -553,1 +569,4 @@}} // namespaceQCefCookieManager *QCefInternal::create_cookie_manager(const std::string &storage_path, bool persist_session_cookies)@ -553,1 +569,4 @@}} // namespaceQCefCookieManager *QCefInternal::create_cookie_manager(const std::string &storage_path, bool persist_session_cookies)Fixed
I've rebased and pushed the latest review fixes, with a couple minor additions
os_event_try(cef_started_event)check toQCefWidgetInternal::Resize()to fix a crash when the user has no browser sources but does have browser docks. This is initiated by therestoreDockStatefunction before CEF has initialised. Docks do correctly resize once they've loaded.bool QueueCEFTaskfor ultimate protection. Open to feedback on this.StorageNotificationServicejust in caseAdditionally, after some testing from Warchamp, we have confirmed the two following regressions
shell_devtools_discovery_page.htmlresource is not being shipped in the.pakfile by CEF, so the website returns an empty string. We are experimenting with options. Users do have the alternative of usingchrome://inspectfrom a desktop browser instead, though it is very slow to refresh its list and currently displays duplicates.github.com/WizardCM/obs-browser@4c65eaaf40Additional area I'm looking into, which may be separate PR
obs.rc.inin the main repo if I don't successfully guess the icon ID number.@ -74,24 +131,40 @@ void BrowserApp::OnBeforeCommandLineProcessing(const CefString &, CefRefPtr<CefC}}?
@tytan652 I saw your 👎, is that only related to generally using obs-browser with cef 148, or is it related to applying the patch? If it’s the latter, could you elaborate?
As far as I know, obs-browser worked before this patch, and it still works with this patch.
Is this not what the
--enable-media-streamcurrently does? Is that removed in newer Chromium versions?I've been using this for a while for browser docks to have access to output device listing, but AFAIK this also allows input device listing.
@ -74,24 +131,40 @@ void BrowserApp::OnBeforeCommandLineProcessing(const CefString &, CefRefPtr<CefC}}Of course I did something this silly. Fixed.
I'm unsure if it was removed, or deprecated in favour of other things. At the very least, CEF explicitly denies the request in the Chrome Runtime's Alloy Style when it didn't in the Alloy Runtime.
We could, as a temporary solution, read the flag and manually allow those calls for now, but ultimately I think having some kind of UI for it would be better, especially as there's a proper API for it.
Thanks a lot for working on this — I’m really looking forward to using a modern Chromium build in browser sources.
I have one request.
If possible, I’d like you to consider including IndexedDB in the scope of this migration.
github.com/WizardCM/obs-browser@6d1031e0e8/obs-browser-plugin.cpp (L749-L755)Thanks again!
Thanks for the feedback! I see my mistake was using a barebones scene collection with basic browser sources & docks as a testing ground - there do seem to be other directories, like IndexedDB (and probably Service Worker or WebStorage) that should be migrated too. I'll have to do some additional research on this and I'll update the PR.
@ -74,24 +131,40 @@ void BrowserApp::OnBeforeCommandLineProcessing(const CefString &, CefRefPtr<CefC}}At this point, wouldn't it be more efficient to do this:
The
constexprvalues are all evaluated at compile time, so the resulting code will only contain a singledisableFeatures.append(char const*, unsigned long)call, with the correct length value computed by the compiler.This brings the code down to a single
appendoperation by default, and at most 3appendoperations in the case of an existingdisable-featurescommand-line argument.Because Windows always requires this feature to be disabled, appending it at runtime would be a permanent runtime cost that this variant avoids.
(As an added bonus, converting the preprocessor statement into
constexpr booland usingif constexprmakes navigating multi-platform code easier because IDEs and language servers will now "see" the entire code again.)@ -554,1 +573,4 @@{std::string path = storage_path;#if CHROME_VERSION_BUILD > 6533// TODO: Update obs-studio to use the new path structure due to subdirectories not being supported by CEFSee above, this should also use the helper function.
@ -565,1 +590,3 @@BPtr<char> rpath = obs_module_config_path(storage_path.c_str());std::string path = storage_path;#if CHROME_VERSION_BUILD > 6533// TODO: Update obs-studio to use the new path structure due to subdirectories not being supported by CEFAs this code is effectively implemented twice, it should be factored out:
Which enables the suggestion at the top. The helper function gets its own copy of the referenced string by default, and I would also expect this code to be optimised via RVO, so there's only a single copy involved (the string is created in the calling function).
It's a bit unfortunate that this copy is done unconditionally, even though over time there should be less and less users for which this "upgrade" (and thus copy) is necessary, but it's a drop in the bucket for all the string copies and mutations we do in the program already.
@ -74,24 +131,40 @@ void BrowserApp::OnBeforeCommandLineProcessing(const CefString &, CefRefPtr<CefC}}Yeah when I was preparing this PR the same consideration had crossed my mind, but I felt it was out of scope and that I'd do it first thing after this PR is merged to make this one more straightforward to review. Let me know which you'd prefer - a separate PR post-merge or a separate commit within this PR and I'll get it done.
@ -74,24 +131,40 @@ void BrowserApp::OnBeforeCommandLineProcessing(const CefString &, CefRefPtr<CefC}}I personally prefer the "if I'm already here making changes, I might as well clean this up and make it nice, tidy, and modern" approach (because then it's done with and not added to a pile of stuff that is usually forgotten), but @RytoEX is more selective in this regard.
I defer to him on this one.
@ -74,24 +131,40 @@ void BrowserApp::OnBeforeCommandLineProcessing(const CefString &, CefRefPtr<CefC}}Feel free to add it as a separate commit and I'll evaluate if it should be squashed or merged as a separate PR.
@ -565,1 +590,3 @@BPtr<char> rpath = obs_module_config_path(storage_path.c_str());std::string path = storage_path;#if CHROME_VERSION_BUILD > 6533// TODO: Update obs-studio to use the new path structure due to subdirectories not being supported by CEFThis code path literally won't be around that long, it'll only exist for as long as it takes to merge this PR, create a PR in obs-studio, and then merge that PR - ideally never actually released. Or the check could be reversed I guess? I have made the change, let me know if it needs any additional adjustment.
@ -74,24 +131,40 @@ void BrowserApp::OnBeforeCommandLineProcessing(const CefString &, CefRefPtr<CefC}}Added as a separate commit.
Tested this on macOS arm64 (Apple silicon) against CEF 147 — browser sources render, Chrome Runtime initialises, and the Alloy→Chrome Runtime profile migration completes cleanly against a real long-lived profile. Thanks for grinding through this one.
Two macOS-specific notes:
#517 isn't quite enough on macOS. It bumps the plugin and the Linux helper, but
cmake/os-macos.cmakebuilds four more helper executables that also compile the CEF headers, and they still fail onstd::same_as/derived_from/convertible_to. One extratarget_compile_features(... cxx_std_20)in the helper loop fixes it — details in https://github.com/obsproject/obs-browser/pull/517.The migration writes to the real profile even in portable mode.
obs_module_config_path()doesn't honour--portable, so the migration runs against~/Library/Application Support/obs-studio/plugin_config/obs-browserregardless. It's copy-only so nothing is destroyed, and I confirmed a stock OBS 32.1.2 (Alloy, CEF 127.0.6533.120) still loads and renders browser sources against the migrated profile afterwards — so the two can coexist on one profile. Worth mentioning to testers so they back up first, since--portablelooks like it would isolate things but doesn't.Environment
147.0.14+g76d2442+chromium-147.0.7727.138(macosarm64), wrapper built withUSE_SANDBOX=OFF0218aa658738d9d0Warchamp performed some quick testing and noticed that CEF 151 (7922) crashes when interacting with browser docks. I have verified that is the case, and that CEF 150 (7871) behaves normally. I have updated the title and description to reflect this, and will separately look into why CEF 151 is misbehaving.
Thanks for the huge effort on the Chrome runtime migration!
I wanted to flag a potential compatibility issue related to the permission handling that was deferred to a follow-up PR.
Chromium 142 introduced permission-gated Local Network Access (LNA) checks for requests from public HTTPS origins to local and loopback destinations (Chrome blog). Chromium 147 extends these checks to WebSocket connections, including loopback destinations such as
ws://127.0.0.1(Intent to Ship).The CEF range supported by this PR includes Chromium 147+. Since OBS browser sources and docks use Alloy style, where permission prompts are not shown by default, I'm concerned that remotely hosted HTTPS overlays connecting to local WebSocket servers may no longer be able to connect.
Connecting from a remotely hosted page to a WebSocket server running on the local machine is a fairly common pattern among streaming tools and overlays, so the impact could be fairly broad in the streaming ecosystem.
Would this be worth taking into account in the follow-up permission work, or tracking as a separate issue?
I mainly wanted to make sure it was on the radar before a CEF 147+ build ships.
Thanks for the feedback @kurehajime - it'll definitely be taken into account with an initial permission implementation alongside the release of this PR. Keep an eye out for a follow-up PR, likely once this one is merged.
Mostly nits.
@ -48,11 +54,62 @@ CefRefPtr<CefBrowserProcessHandler> BrowserApp::GetBrowserProcessHandler()return this;@ -74,24 +131,40 @@ void BrowserApp::OnBeforeCommandLineProcessing(const CefString &, CefRefPtr<CefC}@ -626,6 +626,17 @@ void BrowserClient::OnLoadEnd(CefRefPtr<CefBrowser>, CefRefPtr<CefFrame> frame,}@ -0,0 +14,4 @@You should have received a copy of the GNU General Public Licensealong with this program. If not, see <http://www.gnu.org/licenses/>.******************************************************************************/Add an appropriate license header comment block (GPLv2+).
@ -0,0 +15,4 @@along with this program. If not, see <http://www.gnu.org/licenses/>.******************************************************************************/#pragma onceAdd an appropriate license header comment block (GPLv2+).
@ -14,7 +14,8 @@ add_executable(OBS::browser-helper ALIAS browser-helper)@ -29,7 +29,8 @@ foreach(helper IN LISTS helper_suffixes)target_sources(${target_name} PRIVATE # cmake-format: sortable@ -13,8 +13,8 @@ add_executable(OBS::browser-helper ALIAS obs-browser-helper)target_sources(@ -714,8 +720,122 @@ static void check_hwaccel_support(void)#endif@ -0,0 +14,4 @@You should have received a copy of the GNU General Public Licensealong with this program. If not, see <http://www.gnu.org/licenses/>.******************************************************************************/Fixed
@ -0,0 +15,4 @@along with this program. If not, see <http://www.gnu.org/licenses/>.******************************************************************************/#pragma onceFixed
@ -74,24 +131,40 @@ void BrowserApp::OnBeforeCommandLineProcessing(const CefString &, CefRefPtr<CefC}In my testing the condition returns
true, but the value is empty, so a single,is added todisableFeatures.I don't see
disableFeaturesactually being used here to actually disable the features, or am I missing something?It looks like this line was removed and not re-added:
Oops, must've disappeared in the rebase or just as part of the refactor commit. Fixed.
@ -74,24 +131,40 @@ void BrowserApp::OnBeforeCommandLineProcessing(const CefString &, CefRefPtr<CefC}Hmm, interesting. I could extend the check to first check the length of
GetSwitchValue? Very weird that it would do that at all, though.@ -74,24 +131,40 @@ void BrowserApp::OnBeforeCommandLineProcessing(const CefString &, CefRefPtr<CefC}The if statement is checking the value of:
but the append operation in the
ifstatement block is using:Note the swapped hyphen (
-) and underscore (_). Maybe this is why the value is empty?@ -74,24 +131,40 @@ void BrowserApp::OnBeforeCommandLineProcessing(const CefString &, CefRefPtr<CefC}Good catch, this should be a hyphen. I've now fixed it, and Windows correctly includes the additional
AutofillActorMode.Nits.
@ -74,24 +131,40 @@ void BrowserApp::OnBeforeCommandLineProcessing(const CefString &, CefRefPtr<CefC}Ordering nit. The last item is left "out of order" to accommodate the substring usage below.
Nit, move
disable-featuresbelowdisable-extensions:Adding
GlobalShortcutsPortalin disabled features for the side Linux is needed (CEF should not meddle with this portal).@ -74,24 +131,40 @@ void BrowserApp::OnBeforeCommandLineProcessing(const CefString &, CefRefPtr<CefC}Done
@ -74,24 +131,40 @@ void BrowserApp::OnBeforeCommandLineProcessing(const CefString &, CefRefPtr<CefC}Done
Checked with CEF 150 on macOS, profiles were migrated correctly as far as I could tell and new profiles (and profile data) is not visible to current master either.
Added a commit based on @tytan652's request. I have verified with Chrome verbose logging that CEF doesn't complain about unrecognised entries on other platforms. A future change will be to do platform-specific implementations so we're not disabling unrecognised features on all platforms and don't overcomplicate the conditional checks in this area of the code.
Great to see this merged, thanks to everyone involved!