Skip to content

Commit

Permalink
feat: Implement BrowserWindow.moveAbove(mediaSourceId)
Browse files Browse the repository at this point in the history
BrowserWindow.{focus,blur,moveTop}() are not enough in some
situations. For example when implementing an overlay that
follows another window that can lose focus. In that case
it is useful to move the overlay above the tracked window.

sourceId is a string in the format of DesktopCapturerSource.id,
for example "window:1869:0".

Notes: Added BrowserWindow.moveAbove(mediaSourceId)

#18922
  • Loading branch information
Julien Isorce authored and Julien Isorce committed Jul 10, 2019
1 parent 443130a commit 1f23b2f
Show file tree
Hide file tree
Showing 12 changed files with 221 additions and 4 deletions.
8 changes: 8 additions & 0 deletions docs/api/browser-window.md
Original file line number Diff line number Diff line change
Expand Up @@ -1116,6 +1116,14 @@ can not be focused on.

Returns `Boolean` - Whether the window is always on top of other windows.

#### `win.moveAbove(mediaSourceId)`

* `mediaSourceId` String - Window id in the format of DesktopCapturerSource's id. For example "window:1869:0".

Moves window above the source window in the sense of z-order. If the
`mediaSourceId` is not of type window or if the window does not exist then
this method throws an error.

#### `win.moveTop()`

Moves window to top(z-order) regardless of focus
Expand Down
6 changes: 6 additions & 0 deletions shell/browser/api/atom_api_top_level_window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,11 @@ std::vector<int> TopLevelWindow::GetPosition() {
result[1] = pos.y();
return result;
}
void TopLevelWindow::MoveAbove(const std::string& sourceId,
mate::Arguments* args) {
if (!window_->MoveAbove(sourceId))
args->ThrowError("Invalid media source id");
}

void TopLevelWindow::MoveTop() {
window_->MoveTop();
Expand Down Expand Up @@ -1075,6 +1080,7 @@ void TopLevelWindow::BuildPrototype(v8::Isolate* isolate,
.SetMethod("setMaximumSize", &TopLevelWindow::SetMaximumSize)
.SetMethod("getMaximumSize", &TopLevelWindow::GetMaximumSize)
.SetMethod("setSheetOffset", &TopLevelWindow::SetSheetOffset)
.SetMethod("moveAbove", &TopLevelWindow::MoveAbove)
.SetMethod("moveTop", &TopLevelWindow::MoveTop)
.SetMethod("_setResizable", &TopLevelWindow::SetResizable)
.SetMethod("_isResizable", &TopLevelWindow::IsResizable)
Expand Down
1 change: 1 addition & 0 deletions shell/browser/api/atom_api_top_level_window.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ class TopLevelWindow : public mate::TrackableObject<TopLevelWindow>,
void SetResizable(bool resizable);
bool IsResizable();
void SetMovable(bool movable);
void MoveAbove(const std::string& sourceId, mate::Arguments* args);
void MoveTop();
bool IsMovable();
void SetMinimizable(bool minimizable);
Expand Down
1 change: 1 addition & 0 deletions shell/browser/native_window.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ class NativeWindow : public base::SupportsUserData,
virtual double GetSheetOffsetX();
virtual double GetSheetOffsetY();
virtual void SetResizable(bool resizable) = 0;
virtual bool MoveAbove(const std::string& sourceId) = 0;
virtual void MoveTop() = 0;
virtual bool IsResizable() = 0;
virtual void SetMovable(bool movable) = 0;
Expand Down
1 change: 1 addition & 0 deletions shell/browser/native_window_mac.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ class NativeWindowMac : public NativeWindow {
void SetContentSizeConstraints(
const extensions::SizeConstraints& size_constraints) override;
void SetResizable(bool resizable) override;
bool MoveAbove(const std::string& sourceId) override;
void MoveTop() override;
bool IsResizable() override;
void SetMovable(bool movable) override;
Expand Down
17 changes: 17 additions & 0 deletions shell/browser/native_window_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "base/strings/sys_string_conversions.h"
#include "components/remote_cocoa/app_shim/native_widget_ns_window_bridge.h"
#include "content/public/browser/browser_accessibility_state.h"
#include "content/public/browser/desktop_media_id.h"
#include "native_mate/dictionary.h"
#include "shell/browser/native_browser_view_mac.h"
#include "shell/browser/ui/cocoa/atom_native_widget_mac.h"
Expand All @@ -28,6 +29,7 @@
#include "shell/browser/window_list.h"
#include "shell/common/options_switches.h"
#include "skia/ext/skia_utils_mac.h"
#include "third_party/webrtc/modules/desktop_capture/mac/window_list_utils.h"
#include "ui/gfx/skia_util.h"
#include "ui/gl/gpu_switching_manager.h"
#include "ui/views/background.h"
Expand Down Expand Up @@ -739,6 +741,21 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) {
NativeWindow::SetContentSizeConstraints(size_constraints);
}

bool NativeWindowMac::MoveAbove(const std::string& sourceId) {
const content::DesktopMediaID id = content::DesktopMediaID::Parse(sourceId);
if (id.type != content::DesktopMediaID::TYPE_WINDOW)
return false;

// Check if the window source is valid.
const CGWindowID window_id = id.id;
if (!webrtc::GetWindowOwnerPid(window_id))
return false;

[window_ orderWindow:NSWindowAbove relativeTo:id.id];

return true;
}

void NativeWindowMac::MoveTop() {
[window_ orderWindow:NSWindowAbove relativeTo:0];
}
Expand Down
26 changes: 26 additions & 0 deletions shell/browser/native_window_views.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

#include "base/strings/utf_string_conversions.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/desktop_media_id.h"
#include "native_mate/dictionary.h"
#include "shell/browser/api/atom_api_web_contents.h"
#include "shell/browser/native_browser_view_views.h"
Expand Down Expand Up @@ -643,6 +644,31 @@ void NativeWindowViews::SetResizable(bool resizable) {
resizable_ = resizable;
}

bool NativeWindowViews::MoveAbove(const std::string& sourceId) {
const content::DesktopMediaID id = content::DesktopMediaID::Parse(sourceId);
if (id.type != content::DesktopMediaID::TYPE_WINDOW)
return false;

#if defined(OS_WIN)
const HWND otherWindow = reinterpret_cast<HWND>(id.id);
if (!::IsWindow(otherWindow))
return false;

gfx::Point pos = GetPosition();
gfx::Size size = GetSize();
::SetWindowPos(GetAcceleratedWidget(), otherWindow, pos.x(), pos.y(),
size.width(), size.height(),
SWP_NOACTIVATE | SWP_NOMOVE | SWP_NOSIZE | SWP_SHOWWINDOW);
#elif defined(USE_X11)
if (!IsWindowValid(id.id))
return false;

electron::MoveWindowAbove(GetAcceleratedWidget(), id.id);
#endif

return true;
}

void NativeWindowViews::MoveTop() {
// TODO(julien.isorce): fix chromium in order to use existing
// widget()->StackAtTop().
Expand Down
1 change: 1 addition & 0 deletions shell/browser/native_window_views.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ class NativeWindowViews : public NativeWindow,
void SetContentSizeConstraints(
const extensions::SizeConstraints& size_constraints) override;
void SetResizable(bool resizable) override;
bool MoveAbove(const std::string& sourceId) override;
void MoveTop() override;
bool IsResizable() override;
void SetMovable(bool movable) override;
Expand Down
11 changes: 10 additions & 1 deletion shell/browser/ui/x/x_window_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ bool ShouldUseGlobalMenuBar() {
}

void MoveWindowToForeground(::Window xwindow) {
MoveWindowAbove(xwindow, 0);
}

void MoveWindowAbove(::Window xwindow, ::Window other_xwindow) {
XDisplay* xdisplay = gfx::GetXDisplay();
XEvent xclient;
memset(&xclient, 0, sizeof(xclient));
Expand All @@ -99,7 +103,7 @@ void MoveWindowToForeground(::Window xwindow) {
xclient.xclient.message_type = GetAtom("_NET_RESTACK_WINDOW");
xclient.xclient.format = 32;
xclient.xclient.data.l[0] = 2;
xclient.xclient.data.l[1] = 0;
xclient.xclient.data.l[1] = other_xwindow;
xclient.xclient.data.l[2] = Above;
xclient.xclient.data.l[3] = 0;
xclient.xclient.data.l[4] = 0;
Expand All @@ -109,4 +113,9 @@ void MoveWindowToForeground(::Window xwindow) {
XFlush(xdisplay);
}

bool IsWindowValid(::Window xwindow) {
XWindowAttributes attrs;
return XGetWindowAttributes(gfx::GetXDisplay(), xwindow, &attrs);
}

} // namespace electron
8 changes: 7 additions & 1 deletion shell/browser/ui/x/x_window_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,15 @@ void SetWindowType(::Window xwindow, const std::string& type);
// Returns true if the bus name "com.canonical.AppMenu.Registrar" is available.
bool ShouldUseGlobalMenuBar();

// Bring the given window to the front and give it the focus.
// Bring the given window to the front regardless of focus.
void MoveWindowToForeground(::Window xwindow);

// Move a given window above the other one.
void MoveWindowAbove(::Window xwindow, ::Window other_xwindow);

// Return true is the given window exists, false otherwise.
bool IsWindowValid(::Window xwindow);

} // namespace electron

#endif // SHELL_BROWSER_UI_X_X_WINDOW_UTILS_H_
42 changes: 42 additions & 0 deletions spec-main/api-browser-window-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,48 @@ describe('BrowserWindow module', () => {
})
})

describe('BrowserWindow.moveAbove(mediaSourceId)', () => {
it('should throw an exception if wrong formatting', async () => {
const fakeSourceIds = [
'none', 'screen:0', 'window:fake', 'window:1234', 'foobar:1:2'
]
fakeSourceIds.forEach((sourceId) => {
expect(() => {
w.moveAbove(sourceId)
}).to.throw(/Invalid media source id/)
})
})
it('should throw an exception if wrong type', async () => {
const fakeSourceIds = [null as any, 123 as any]
fakeSourceIds.forEach((sourceId) => {
expect(() => {
w.moveAbove(sourceId)
}).to.throw(/Error processing argument at index 0 */)
})
})
it('should throw an exception if invalid window', async () => {
// It is very unlikely that these window id exist.
const fakeSourceIds = ['window:99999999:0', 'window:123456:1',
'window:123456:9']
fakeSourceIds.forEach((sourceId) => {
expect(() => {
w.moveAbove(sourceId)
}).to.throw(/Invalid media source id/)
})
})
it('should not throw an exception', async () => {
const w2 = new BrowserWindow({ show: false, title: 'window2' })
const w2Shown = emittedOnce(w2, 'show')
w2.show()
await w2Shown

expect(() => {
w.moveAbove(w2.getMediaSourceId())
}).to.not.throw()

await closeWindow(w2, { assertNotWindows: false })
})
})
})

describe('sizing', () => {
Expand Down
103 changes: 101 additions & 2 deletions spec/api-desktop-capturer-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,10 @@ describe('desktopCapturer', () => {
w.show()
await wShown

const sources = await desktopCapturer.getSources({ types: ['window', 'screen'], thumbnailSize: { width: 0, height: 0 } })
const sources = await desktopCapturer.getSources({
types: ['window', 'screen'],
thumbnailSize: { width: 0, height: 0 }
})
w.destroy()
expect(sources).to.be.an('array').that.is.not.empty()
for (const { thumbnail: thumbnailImage } of sources) {
Expand All @@ -111,7 +114,10 @@ describe('desktopCapturer', () => {
await wFocused

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

// TODO(julien.isorce): investigate why |sources| is empty on the linux
Expand All @@ -128,4 +134,97 @@ describe('desktopCapturer', () => {
})
expect(mediaSourceId).to.equal(foundSource.id)
})

it('moveAbove should move the window at the requested place', async () => {
// DesktopCapturer.getSources() is guaranteed to return in the correct
// z-order from foreground to background.
const MAX_WIN = 4
const { BrowserWindow } = remote
const mainWindow = remote.getCurrentWindow()
const wList = [mainWindow]
try {
// Add MAX_WIN-1 more window so we have MAX_WIN in total.
for (let i = 0; i < MAX_WIN - 1; i++) {
const w = new BrowserWindow({ show: true, width: 100, height: 100 })
wList.push(w)
}
expect(wList.length).to.equal(MAX_WIN)

// Show and focus all the windows.
wList.forEach(async (w) => {
const wFocused = emittedOnce(w, 'focus')
w.focus()
await wFocused
})
// At this point our windows should be showing from bottom to top.

// DesktopCapturer.getSources() returns sources sorted from foreground to
// background, i.e. top to bottom.
let sources = await desktopCapturer.getSources({
types: ['window'],
thumbnailSize: { width: 0, height: 0 }
})

// 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) {
wList.forEach((w) => {
if (w !== mainWindow) {
w.destroy()
}
})
it.skip('desktopCapturer.getSources returned an empty source list')
return
}

expect(sources).to.be.an('array').that.is.not.empty()
expect(sources.length).to.gte(MAX_WIN)

// Only keep our windows, they must be in the MAX_WIN first windows.
sources.splice(MAX_WIN, sources.length - MAX_WIN)
expect(sources.length).to.equal(MAX_WIN)
expect(sources.length).to.equal(wList.length)

// Check that the sources and wList are sorted in the reverse order.
const wListReversed = wList.slice(0).reverse()
const canGoFurther = sources.every(
(source, index) => source.id === wListReversed[index].getMediaSourceId())
if (!canGoFurther) {
// Skip remaining checks because either focus or window placement are
// not reliable in the running test environment. So there is no point
// to go further to test moveAbove as requirements are not met.
return
}

// Do the real work, i.e. move each window above the next one so that
// wList is sorted from foreground to background.
wList.forEach(async (w, index) => {
if (index < (wList.length - 1)) {
const wNext = wList[index + 1]
w.moveAbove(wNext.getMediaSourceId())
}
})

sources = await desktopCapturer.getSources({
types: ['window'],
thumbnailSize: { width: 0, height: 0 }
})
// Only keep our windows again.
sources.splice(MAX_WIN, sources.length - MAX_WIN)
expect(sources.length).to.equal(MAX_WIN)
expect(sources.length).to.equal(wList.length)

// Check that the sources and wList are sorted in the same order.
sources.forEach((source, index) => {
expect(source.id).to.equal(wList[index].getMediaSourceId())
})
} finally {
wList.forEach((w) => {
if (w !== mainWindow) {
w.destroy()
}
})
}
})
})

0 comments on commit 1f23b2f

Please sign in to comment.