devtools: Fix ignoreDefaultArgs capability option
#5710
Conversation
|
|
| "--password-store=basic", | ||
| "--use-mock-keychain", | ||
| "--disable-component-extensions-with-background-pages", | ||
| "--disable-breakpad", | ||
| "--disable-dev-shm-usage", | ||
| "--disable-ipc-flooding-protection", | ||
| "--disable-renderer-backgrounding", | ||
| "--force-fieldtrials=*BackgroundTracing/default/", |
There was a problem hiding this comment.
Do we need to disable background tracing by default? Will this have any impact on trace gatherer? @christian-bromann
There was a problem hiding this comment.
From looking at the implementation in chrome-launcher it looks to me like it will always use DEFAULT_FLAGS if ignoreDefaultFlags is falsy. ignoreDefaultFlags should always have been falsy, since we weren't passing it (we passed ignoreDefaultArgs). DEFAULT_FLAGS contains --force-fieldtrials=*BackgroundTracing/default/. So it seems to me like we've always gotten the argument, so this PR shouldn't have any impact on this, as from what I can tell, nothing changes in this regard. Of course, if you'd like to actually stop using the argument, let me know!
The value of `ignoreDefaultArgs` was previously passed directly to `launch` from the package `chrome-launcher`. However, that function doesn't take the same parameter, it takes an `ignoreDefaultFlags` boolean parameter. It also doesn't do any filtering of arguments, it's either just ignoring all default flags or using all of them. This changes code in `devtools` to always send `ignoreDefaultFlags: true`. This should be fine, since we have the default arguments in a list in this project, and passing all default arguments, together with custom arguments, onwards to `chrome-launcher`. To not break previous behavior, I'm also adding these arguments as default arguments, as they were used from within the default arguments in `chrome-launcher`, but not in `devtools`: * `--no-default-browser-check` * `--force-fieldtrials=*BackgroundTracing/default/`
koggdal
force-pushed
the
devtools-fix-ignore-chrome-flags
branch
from
105e859
to
b681f2b
Compare
|
Thanks for the review @SrinivasanTarget! 🙏 Seems like we need someone with write access. @christian-bromann? Thanks! |
christian-bromann
added
the
PR: Polish 💅
Proposed changes
The value of
ignoreDefaultArgswas previously passed directly tolaunchfrom the packagechrome-launcher. However, that function doesn't take the same parameter, it takes anignoreDefaultFlagsboolean parameter. It also doesn't do any filtering of arguments, it's either just ignoring all default flags or using all of them.This changes code in
devtoolsto always sendignoreDefaultFlags: true. This should be fine, since we have the default arguments in a list in this project, and passing all default arguments, together with custom arguments, onwards tochrome-launcher.To not break previous behavior, I'm also adding these arguments as default arguments, as they were used from within the default arguments in
chrome-launcher, but not indevtools:--no-default-browser-check--force-fieldtrials=*BackgroundTracing/default/Types of changes
Checklist
Reviewers: @webdriverio/project-committers