`web_security` flag was removed from cef 91

obsproject / obs-browser

CEF-based OBS Studio browser plugin

GNU General Public License v2.0

776 stars 220 forks source link

`web_security` flag was removed from cef 91 #306

Closed OceanS2000 closed 2 years ago

OceanS2000 commented 3 years ago

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:

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

gxalpha commented 3 years ago

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

OceanS2000 commented 3 years ago

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 commented 2 years ago

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.

OceanS2000 commented 2 years ago

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 commented 2 years ago

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.

OceanS2000 commented 2 years ago

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 commented 2 years ago

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