From 4293389679cb0933533253ff3351bf72d0d60255 Mon Sep 17 00:00:00 2001 From: Eryk Rakowski Date: Thu, 18 Feb 2021 11:17:04 +0100 Subject: [PATCH] feat: add `win.setTopBrowserView()` so that BrowserViews can be raised (#27711) * feat: add `win.setTopBrowserView()` so that BrowserViews can be raised (#27007) * feat: Raise a browser view via `BrowserWindow.setTopBrowserView()`. This is similar to removing and re-adding a browser view, but avoids a visible flicker as the browser view is not removed from the window when using `setTopBrowserView`. Note: if the given browser view is not attached to the window, it will be added. This commit contains the macOS implementation. * feat: setTopBrowserView support for Windows and Linux * docs: add info about setTopBrowserView * docs: Clarify behavior when browserView is not yet attached. * fix: throw en error when browserView is not attached to the window * fix: build error * fix: test * fix: add test case * fix: tests * fix: reparenting * fix: close second window in tests Co-authored-by: sentialx * fix: build error Co-authored-by: Stewart Lord --- docs/api/browser-window.md | 7 +++++ .../api/electron_api_browser_window.cc | 8 ++++++ .../browser/api/electron_api_browser_window.h | 2 ++ .../api/electron_api_top_level_window.cc | 20 ++++++++++++++ .../api/electron_api_top_level_window.h | 2 ++ shell/browser/native_window.h | 1 + shell/browser/native_window_mac.h | 1 + shell/browser/native_window_mac.mm | 24 +++++++++++++++++ shell/browser/native_window_views.cc | 16 ++++++++++++ shell/browser/native_window_views.h | 1 + spec-main/api-browser-view-spec.ts | 26 +++++++++++++++++++ 11 files changed, 108 insertions(+) diff --git a/docs/api/browser-window.md b/docs/api/browser-window.md index 6f48540be58ea..8ef399fd0d41d 100644 --- a/docs/api/browser-window.md +++ b/docs/api/browser-window.md @@ -1829,6 +1829,13 @@ Replacement API for setBrowserView supporting work with multi browser views. * `browserView` [BrowserView](browser-view.md) +#### `win.setTopBrowserView(browserView)` _Experimental_ + +* `browserView` [BrowserView](browser-view.md) + +Raises `browserView` above other `BrowserView`s attached to `win`. +Throws an error if `browserView` is not attached to `win`. + #### `win.getBrowserViews()` _Experimental_ Returns `BrowserView[]` - an array of all BrowserViews that have been attached diff --git a/shell/browser/api/electron_api_browser_window.cc b/shell/browser/api/electron_api_browser_window.cc index 0cd5fe85cc4e3..ffa1db23aa7c1 100644 --- a/shell/browser/api/electron_api_browser_window.cc +++ b/shell/browser/api/electron_api_browser_window.cc @@ -373,6 +373,14 @@ void BrowserWindow::RemoveBrowserView(v8::Local value) { #endif } +void BrowserWindow::SetTopBrowserView(v8::Local value, + gin_helper::Arguments* args) { + TopLevelWindow::SetTopBrowserView(value, args); +#if defined(OS_MACOSX) + UpdateDraggableRegions(draggable_regions_); +#endif +} + void BrowserWindow::ResetBrowserViews() { TopLevelWindow::ResetBrowserViews(); #if defined(OS_MACOSX) diff --git a/shell/browser/api/electron_api_browser_window.h b/shell/browser/api/electron_api_browser_window.h index 42db26b85e90d..dcd51f8d28ede 100644 --- a/shell/browser/api/electron_api_browser_window.h +++ b/shell/browser/api/electron_api_browser_window.h @@ -84,6 +84,8 @@ class BrowserWindow : public TopLevelWindow, void SetBrowserView(v8::Local value) override; void AddBrowserView(v8::Local value) override; void RemoveBrowserView(v8::Local value) override; + void SetTopBrowserView(v8::Local value, + gin_helper::Arguments* args) override; void ResetBrowserViews() override; void SetVibrancy(v8::Isolate* isolate, v8::Local value) override; void OnWindowShow() override; diff --git a/shell/browser/api/electron_api_top_level_window.cc b/shell/browser/api/electron_api_top_level_window.cc index daf23d899302e..14218960d80ba 100644 --- a/shell/browser/api/electron_api_top_level_window.cc +++ b/shell/browser/api/electron_api_top_level_window.cc @@ -784,6 +784,25 @@ void TopLevelWindow::RemoveBrowserView(v8::Local value) { } } +void TopLevelWindow::SetTopBrowserView(v8::Local value, + gin_helper::Arguments* args) { + gin::Handle browser_view; + if (value->IsObject() && + gin::ConvertFromV8(isolate(), value, &browser_view)) { + if (!browser_view->web_contents()) + return; + auto* owner_window = browser_view->web_contents()->owner_window(); + auto get_that_view = browser_views_.find(browser_view->ID()); + if (get_that_view == browser_views_.end() || + (owner_window && owner_window != window_.get())) { + args->ThrowError("Given BrowserView is not attached to the window"); + return; + } + + window_->SetTopBrowserView(browser_view->view()); + } +} + std::string TopLevelWindow::GetMediaSourceId() const { return window_->GetDesktopMediaID().ToString(); } @@ -1203,6 +1222,7 @@ void TopLevelWindow::BuildPrototype(v8::Isolate* isolate, .SetMethod("removeMenu", &TopLevelWindow::RemoveMenu) .SetMethod("setParentWindow", &TopLevelWindow::SetParentWindow) .SetMethod("setBrowserView", &TopLevelWindow::SetBrowserView) + .SetMethod("setTopBrowserView", &TopLevelWindow::SetTopBrowserView) .SetMethod("addBrowserView", &TopLevelWindow::AddBrowserView) .SetMethod("removeBrowserView", &TopLevelWindow::RemoveBrowserView) .SetMethod("getMediaSourceId", &TopLevelWindow::GetMediaSourceId) diff --git a/shell/browser/api/electron_api_top_level_window.h b/shell/browser/api/electron_api_top_level_window.h index d640c28f5163e..89b33c3b599ff 100644 --- a/shell/browser/api/electron_api_top_level_window.h +++ b/shell/browser/api/electron_api_top_level_window.h @@ -174,6 +174,8 @@ class TopLevelWindow : public gin_helper::TrackableObject, virtual void SetBrowserView(v8::Local value); virtual void AddBrowserView(v8::Local value); virtual void RemoveBrowserView(v8::Local value); + virtual void SetTopBrowserView(v8::Local value, + gin_helper::Arguments* args); virtual std::vector> GetBrowserViews() const; virtual void ResetBrowserViews(); std::string GetMediaSourceId() const; diff --git a/shell/browser/native_window.h b/shell/browser/native_window.h index 63ad19de6ac20..037467b66a628 100644 --- a/shell/browser/native_window.h +++ b/shell/browser/native_window.h @@ -165,6 +165,7 @@ class NativeWindow : public base::SupportsUserData, virtual void SetParentWindow(NativeWindow* parent); virtual void AddBrowserView(NativeBrowserView* browser_view) = 0; virtual void RemoveBrowserView(NativeBrowserView* browser_view) = 0; + virtual void SetTopBrowserView(NativeBrowserView* browser_view) = 0; virtual content::DesktopMediaID GetDesktopMediaID() const = 0; virtual gfx::NativeView GetNativeView() const = 0; virtual gfx::NativeWindow GetNativeWindow() const = 0; diff --git a/shell/browser/native_window_mac.h b/shell/browser/native_window_mac.h index b31b9a2ecd9a2..35171d557505a 100644 --- a/shell/browser/native_window_mac.h +++ b/shell/browser/native_window_mac.h @@ -113,6 +113,7 @@ class NativeWindowMac : public NativeWindow, public ui::NativeThemeObserver { void SetFocusable(bool focusable) override; void AddBrowserView(NativeBrowserView* browser_view) override; void RemoveBrowserView(NativeBrowserView* browser_view) override; + void SetTopBrowserView(NativeBrowserView* browser_view) override; void SetParentWindow(NativeWindow* parent) override; content::DesktopMediaID GetDesktopMediaID() const override; gfx::NativeView GetNativeView() const override; diff --git a/shell/browser/native_window_mac.mm b/shell/browser/native_window_mac.mm index 28dc3b4a2ac72..0d3959aa0061c 100644 --- a/shell/browser/native_window_mac.mm +++ b/shell/browser/native_window_mac.mm @@ -1303,6 +1303,30 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) { [CATransaction commit]; } +void NativeWindowMac::SetTopBrowserView(NativeBrowserView* view) { + [CATransaction begin]; + [CATransaction setDisableActions:YES]; + + if (!view) { + [CATransaction commit]; + return; + } + + remove_browser_view(view); + add_browser_view(view); + if (view->GetInspectableWebContentsView()) { + auto* native_view = view->GetInspectableWebContentsView() + ->GetNativeView() + .GetNativeNSView(); + [[window_ contentView] addSubview:native_view + positioned:NSWindowAbove + relativeTo:nil]; + native_view.hidden = NO; + } + + [CATransaction commit]; +} + void NativeWindowMac::SetParentWindow(NativeWindow* parent) { InternalSetParentWindow(parent, IsVisible()); } diff --git a/shell/browser/native_window_views.cc b/shell/browser/native_window_views.cc index 17f38798bc70c..2770db5d08ee3 100644 --- a/shell/browser/native_window_views.cc +++ b/shell/browser/native_window_views.cc @@ -1107,6 +1107,22 @@ void NativeWindowViews::RemoveBrowserView(NativeBrowserView* view) { remove_browser_view(view); } +void NativeWindowViews::SetTopBrowserView(NativeBrowserView* view) { + if (!content_view()) + return; + + if (!view) { + return; + } + + remove_browser_view(view); + add_browser_view(view); + + if (view->GetInspectableWebContentsView()) + content_view()->ReorderChildView( + view->GetInspectableWebContentsView()->GetView(), -1); +} + void NativeWindowViews::SetParentWindow(NativeWindow* parent) { NativeWindow::SetParentWindow(parent); diff --git a/shell/browser/native_window_views.h b/shell/browser/native_window_views.h index 532f609ff8b60..5352f93899bba 100644 --- a/shell/browser/native_window_views.h +++ b/shell/browser/native_window_views.h @@ -114,6 +114,7 @@ class NativeWindowViews : public NativeWindow, void SetMenu(ElectronMenuModel* menu_model) override; void AddBrowserView(NativeBrowserView* browser_view) override; void RemoveBrowserView(NativeBrowserView* browser_view) override; + void SetTopBrowserView(NativeBrowserView* browser_view) override; void SetParentWindow(NativeWindow* parent) override; gfx::NativeView GetNativeView() const override; gfx::NativeWindow GetNativeWindow() const override; diff --git a/spec-main/api-browser-view-spec.ts b/spec-main/api-browser-view-spec.ts index cde511d637bae..873e4c47bf11a 100644 --- a/spec-main/api-browser-view-spec.ts +++ b/spec-main/api-browser-view-spec.ts @@ -206,6 +206,32 @@ describe('BrowserView module', () => { }); }); + describe('BrowserWindow.setTopBrowserView()', () => { + it('should throw an error when a BrowserView is not attached to the window', () => { + view = new BrowserView(); + expect(() => { + w.setTopBrowserView(view); + }).to.throw(/is not attached/); + }); + + it('should throw an error when a BrowserView is attached to some other window', () => { + view = new BrowserView(); + + const win2 = new BrowserWindow(); + + w.addBrowserView(view); + view.setBounds({ x: 0, y: 0, width: 100, height: 100 }); + win2.addBrowserView(view); + + expect(() => { + w.setTopBrowserView(view); + }).to.throw(/is not attached/); + + win2.close(); + win2.destroy(); + }); + }); + describe('BrowserView.webContents.getOwnerBrowserWindow()', () => { it('points to owning window', () => { view = new BrowserView();