Skip to content

Conversation

@ashen0xa
Copy link

Description

CefBrowserSettings.web_security no longer exists from CEF >= 91. This patch adds a version check and removes reference to this flag.

Motivation and Context

As suggested in the upstream issue, the web_security flag is considered no longer useful and removed. This change breaks build of current plugin (#305).

How Has This Been Tested?

I build and test on my own working machine with Gentoo Linux and master branch obs-studio code. The plugin builds and works fine.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@gxalpha
Copy link
Member

gxalpha commented Aug 18, 2021

Everywhere else in the code, the 4-digit CHROME_VERSION_BUILD is used, as opposed to CEF_VERSION_MAJOR. Is there any particular reason for using the major version?

In case you want to change that: It looks like the CHROME_VERSION_BUILD in which it got removed was 4430 (90.0.4430.51 to be exact).

Also, try to make sure to follow the guidelines on commit names (a prefix isn't required though). I would suggest something along the lines of Remove web_security flag in CEF 91 and newer

@ashen0xa
Copy link
Author

Everywhere else in the code, the 4-digit CHROME_VERSION_BUILD is used, as opposed to CEF_VERSION_MAJOR. Is there any particular reason for using the major version?

In case you want to change that: It looks like the CHROME_VERSION_BUILD in which it got removed was [4430]

That's my oversight, thanks for your suggestion!

The new commit is now build tested against libcef 92.0.27+g274abcf+chromium-92.0.4515.159 and 89.0.7+gb5952bd+chromium-89.0.4389.72_linux64_minimal.

@PatTheMav
Copy link
Member

Is this enough to keep existing functionality? IIrc this must now be disabled by passing --disable-web-security to the process.

If we still require that functionality for local file support, additional code must be added to check for that source setting and then add --disable-web-security to the CEF command line.

@ashen0xa
Copy link
Author

I'm wondering whether we need the functionality now. The issue referenced above saying disable_web_security not working is dated back to Dec 2020. So disable_web_security flag should be useless since then and there seems (at least for me) to be little impact on obs.

Might someone can direct me to some examples about how this feature is used as I have not yet found one on the forum...

@WizardCM
Copy link
Member

WizardCM commented Oct 24, 2021

Except that we're currently using Chrome 75 as a base on Windows, which was released June 2019, 85 on macOS, released August 2020, and 87 on Linux, released December 2nd 2020.

We use this feature to allow better file system access (and large file access) for "Local file" mode. Best way to test it is to embed a large locally-hosted video file into a local webpage. As the in-code comment states, this is also used to allow local files to access external APIs without trouble.

@ashen0xa
Copy link
Author

Thanks for the information! It now makes sense to me now why only me troubled by this as I now use libcef (and obs) built locally.

I will test with a page as you described and maybe come up something back later.

@WizardCM
Copy link
Member

Closing in favour of the changes of #323 - Chrome 95 is now fully supported and will be the version we recommend going forward.

@WizardCM WizardCM closed this Nov 27, 2021
@ashen0xa ashen0xa deleted the fix-cef-91 branch December 20, 2021 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants