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

fix: disable kWindowCaptureMacV2 for desktop capturer #30524

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions shell/browser/feature_list.cc
Expand Up @@ -48,6 +48,12 @@ void InitializeFeatureList() {
// an empty suggestions list to be returned
disable_features +=
std::string(",") + spellcheck::kWinRetrieveSuggestionsOnlyOnDemand.name;
#endif
#if defined(OS_MAC)
// Disable kWindowCaptureMacV2, which causes the wrong window id to
// be returned (this has been disabled in upstream Chromium here):
// https://chromium-review.googlesource.com/c/chromium/src/+/3069272
disable_features += std::string(",") + features::kWindowCaptureMacV2.name;
#endif
base::FeatureList::InitializeInstance(enable_features, disable_features);
}
Expand Down
36 changes: 36 additions & 0 deletions spec-main/api-desktop-capturer-spec.ts
Expand Up @@ -146,6 +146,42 @@ ifdescribe(!process.arch.includes('arm') && process.platform !== 'win32')('deskt
expect(mediaSourceId).to.equal(foundSource!.id);
});

it('getSources should not incorrectly duplicate window_id', async () => {
const w = new BrowserWindow({ show: false, width: 100, height: 100, webPreferences: { contextIsolation: false } });
const wShown = emittedOnce(w, 'show');
const wFocused = emittedOnce(w, 'focus');
w.show();
w.focus();
await wShown;
await wFocused;

// ensure window_id isn't duplicated in getMediaSourceId,
// which uses a different method than getSources
const mediaSourceId = w.getMediaSourceId();
const ids = mediaSourceId.split(':');
expect(ids[1]).to.not.equal(ids[2]);

const sources = await getSources({
types: ['window'],
thumbnailSize: { width: 0, height: 0 }
});
w.destroy();

// TODO(julien.isorce): investigate why |sources| is empty on the linux
// bots while it is not on my workstation, as expected, with and without
// the --ci parameter.
if (process.platform === 'linux' && sources.length === 0) {
it.skip('desktopCapturer.getSources returned an empty source list');
return;
}

expect(sources).to.be.an('array').that.is.not.empty();
for (const source of sources) {
const sourceIds = source.id.split(':');
expect(sourceIds[1]).to.not.equal(sourceIds[2]);
}
});

// TODO(deepak1556): currently fails on all ci, enable it after upgrade.
it.skip('moveAbove should move the window at the requested place', async () => {
// DesktopCapturer.getSources() is guaranteed to return in the correct
Expand Down