Browse Source

fix: `desktopCapturer` and `screen` source ids should match screen ids (#42860)

* fix: desktopCapturer screen source ids should match screen ids

Co-authored-by: Shelley Vohr <[email protected]>

* test: add a regression test

Co-authored-by: Shelley Vohr <[email protected]>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <[email protected]>
trop[bot] 9 months ago
parent
commit
b1bf730793
2 changed files with 48 additions and 6 deletions
  1. 26 5
      shell/browser/api/electron_api_desktop_capturer.cc
  2. 22 1
      spec/api-screen-spec.ts

+ 26 - 5
shell/browser/api/electron_api_desktop_capturer.cc

@@ -167,6 +167,16 @@ std::unique_ptr<ThumbnailCapturer> MakeScreenCapturer() {
                          : nullptr;
 }
 
+#if BUILDFLAG(IS_WIN)
+BOOL CALLBACK EnumDisplayMonitorsCallback(HMONITOR monitor,
+                                          HDC hdc,
+                                          LPRECT rect,
+                                          LPARAM data) {
+  reinterpret_cast<std::vector<HMONITOR>*>(data)->push_back(monitor);
+  return TRUE;
+}
+#endif
+
 }  // namespace
 
 namespace gin {
@@ -396,11 +406,22 @@ void DesktopCapturer::UpdateSourcesList(DesktopMediaList* list) {
         return;
       }
 
-      int device_name_index = 0;
-      for (auto& source : screen_sources) {
-        const auto& device_name = device_names[device_name_index++];
-        const int64_t device_id = base::PersistentHash(device_name);
-        source.display_id = base::NumberToString(device_id);
+      std::vector<HMONITOR> monitors;
+      EnumDisplayMonitors(nullptr, nullptr, EnumDisplayMonitorsCallback,
+                          reinterpret_cast<LPARAM>(&monitors));
+
+      std::vector<std::pair<std::string, MONITORINFOEX>> pairs;
+      for (const auto& device_name : device_names) {
+        std::wstring wide_device_name;
+        base::UTF8ToWide(device_name.c_str(), device_name.size(),
+                         &wide_device_name);
+        for (const auto monitor : monitors) {
+          MONITORINFOEX monitorInfo{{sizeof(MONITORINFOEX)}};
+          if (GetMonitorInfo(monitor, &monitorInfo)) {
+            if (wide_device_name == monitorInfo.szDevice)
+              pairs.push_back(std::make_pair(device_name, monitorInfo));
+          }
+        }
       }
     }
 #elif BUILDFLAG(IS_MAC)

+ 22 - 1
spec/api-screen-spec.ts

@@ -1,5 +1,6 @@
 import { expect } from 'chai';
-import { Display, screen } from 'electron/main';
+import { Display, screen, desktopCapturer } from 'electron/main';
+import { ifit } from './lib/spec-helpers';
 
 describe('screen module', () => {
   describe('methods reassignment', () => {
@@ -14,6 +15,26 @@ describe('screen module', () => {
     });
   });
 
+  describe('screen.getAllDisplays', () => {
+    it('returns an array of displays', () => {
+      const displays = screen.getAllDisplays();
+      expect(displays).to.be.an('array').with.lengthOf.at.least(1);
+      for (const display of displays) {
+        expect(display).to.be.an('object');
+      }
+    });
+
+    // desktopCapturer.getSources does not work as expected in Windows CI.
+    ifit(process.platform !== 'win32')('returns displays with IDs matching desktopCapturer source display IDs', async () => {
+      const displayIds = screen.getAllDisplays().map(d => `${d.id}`);
+
+      const sources = await desktopCapturer.getSources({ types: ['screen'] });
+      const sourceIds = sources.map(s => s.display_id);
+
+      expect(displayIds).to.have.members(sourceIds);
+    });
+  });
+
   describe('screen.getCursorScreenPoint()', () => {
     it('returns a point object', () => {
       const point = screen.getCursorScreenPoint();