Browse Source

fix: disable kWindowCaptureMacV2 for desktopCapturer (#30507)

Keeley Hammond 3 years ago
parent
commit
a11a234eac
2 changed files with 42 additions and 0 deletions
  1. 6 0
      shell/browser/feature_list.cc
  2. 36 0
      spec-main/api-desktop-capturer-spec.ts

+ 6 - 0
shell/browser/feature_list.cc

@@ -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);
 }

+ 36 - 0
spec-main/api-desktop-capturer-spec.ts

@@ -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