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

Add support for specifying custom enable/disable feature switches #3209

Closed
mrzealot opened this issue Feb 4, 2019 · 4 comments · Fixed by brave/brave-core#1578
Closed

Add support for specifying custom enable/disable feature switches #3209

mrzealot opened this issue Feb 4, 2019 · 4 comments · Fixed by brave/brave-core#1578

Comments

@mrzealot
Copy link

mrzealot commented Feb 4, 2019

I've found a diff between how Chromium and Brave handle console messages over the devtools protocol where console events from a non-top-level frame are not transmitted to the remote debugging entity in Brave. This leads back to how strict site-per-process isolation is handled; at least, specifying --site-per-process on Chromium makes it behave like Brave in that they are both "wrong" from the console messages' POV. Making them both "right" would probably require setting --disable-features=IsolateOrigins,site-per-process as per these docs, but Brave overrides this flag here.

I think this could be solved by being able to set custom disable switches, but there could be other interactions in the background I'm not aware of yet. Can share a MWE to test console event on the remote side if needed.

@iefremov
Copy link
Contributor

iefremov commented Feb 4, 2019

It seems that base::CommandLine can't merge two --disable-features switches

@mkarolin
Copy link
Contributor

mkarolin commented Feb 4, 2019

@iefremov, exactly: https://cs.chromium.org/chromium/src/base/command_line.cc?q=CommandLine::AppendSwitchNative&sq=package:chromium&g=0&l=344 - if the switch already exists the value is just overwritten with the new one.

@btlechowski
Copy link

btlechowski commented Feb 26, 2019

Verification passed on

Brave 0.61.38 Chromium: 73.0.3683.39 (Official Build) beta (64-bit)
Revision cc53b0e12fcaf42e4bab8d6c23bd4fb7aae99f6c-refs/branch-heads/3683@{#413}
OS Windows 7 Service Pack 1 Build 7601.24312

Default:
image

With --disable-features=IsolateOrigins,site-per-process flag
image

Verification passed on

Brave 0.61.41 Chromium: 73.0.3683.39 (Official Build) beta (64-bit)
Revision cc53b0e12fcaf42e4bab8d6c23bd4fb7aae99f6c-refs/branch-heads/3683@{#413}
OS Linux Mint

Default:
image

With --disable-features=IsolateOrigins,site-per-process flag
image

Verifications PASSED on macOS 10.14.3 x64 using the following build:

Brave 0.61.45 Chromium: 73.0.3683.39 (Official Build) beta(64-bit)
Revision cc53b0e12fcaf42e4bab8d6c23bd4fb7aae99f6c-refs/branch-heads/3683@{#413}
OS Mac OS X

Default:

screen shot 2019-03-05 at 2 40 07 am

Using the --disable-features=IsolateOrigins,site-per-process flag

screen shot 2019-03-05 at 2 39 25 am

@kjozwiak
Copy link
Member

kjozwiak commented Mar 5, 2019

Navigation to chrome://process-internals/#general causing webview crash

@GeetaSarvadnya did you create an issue for the above? Looks like it's only happening when you launch Brave using --disable-features=IsolateOrigins,site-per-process and visit chrome://process-internals/#general instead of brave://process-internals/#general. If you haven't, please create a new issue and CC @mkarolin. Reproduced on macOS 10.14.3 x64 as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment