From 573784967c02764c37d8a1f8fb3fd9dd232451c9 Mon Sep 17 00:00:00 2001 From: Eryk Rakowski Date: Thu, 18 Feb 2021 09:09:26 +0100 Subject: [PATCH] feat: add `win.setTopBrowserView()` so that BrowserViews can be raised (#27007) (#27713) * 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 Co-authored-by: Stewart Lord --- docs/api/browser-window.md | 7 +++++ shell/browser/api/electron_api_base_window.cc | 20 ++++++++++++++ shell/browser/api/electron_api_base_window.h | 2 ++ .../api/electron_api_browser_window.cc | 8 ++++++ .../browser/api/electron_api_browser_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 f9939327a5425..fd45e3c431128 100644 --- a/docs/api/browser-window.md +++ b/docs/api/browser-window.md @@ -1867,6 +1867,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_base_window.cc b/shell/browser/api/electron_api_base_window.cc index 9583bbe2fb5c9..8a1a7c42d2b94 100644 --- a/shell/browser/api/electron_api_base_window.cc +++ b/shell/browser/api/electron_api_base_window.cc @@ -787,6 +787,25 @@ void BaseWindow::RemoveBrowserView(v8::Local value) { } } +void BaseWindow::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 BaseWindow::GetMediaSourceId() const { return window_->GetDesktopMediaID().ToString(); } @@ -1215,6 +1234,7 @@ void BaseWindow::BuildPrototype(v8::Isolate* isolate, .SetMethod("setBrowserView", &BaseWindow::SetBrowserView) .SetMethod("addBrowserView", &BaseWindow::AddBrowserView) .SetMethod("removeBrowserView", &BaseWindow::RemoveBrowserView) + .SetMethod("setTopBrowserView", &BaseWindow::SetTopBrowserView) .SetMethod("getMediaSourceId", &BaseWindow::GetMediaSourceId) .SetMethod("getNativeWindowHandle", &BaseWindow::GetNativeWindowHandle) .SetMethod("setProgressBar", &BaseWindow::SetProgressBar) diff --git a/shell/browser/api/electron_api_base_window.h b/shell/browser/api/electron_api_base_window.h index a1551fbdd0b56..2cd0bf7fd41cf 100644 --- a/shell/browser/api/electron_api_base_window.h +++ b/shell/browser/api/electron_api_base_window.h @@ -176,6 +176,8 @@ class BaseWindow : 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/api/electron_api_browser_window.cc b/shell/browser/api/electron_api_browser_window.cc index 6a6566facad0f..8026cdc56b67d 100644 --- a/shell/browser/api/electron_api_browser_window.cc +++ b/shell/browser/api/electron_api_browser_window.cc @@ -378,6 +378,14 @@ void BrowserWindow::RemoveBrowserView(v8::Local value) { #endif } +void BrowserWindow::SetTopBrowserView(v8::Local value, + gin_helper::Arguments* args) { + BaseWindow::SetTopBrowserView(value, args); +#if defined(OS_MACOSX) + UpdateDraggableRegions(draggable_regions_); +#endif +} + void BrowserWindow::ResetBrowserViews() { BaseWindow::ResetBrowserViews(); #if defined(OS_MAC) diff --git a/shell/browser/api/electron_api_browser_window.h b/shell/browser/api/electron_api_browser_window.h index a76badcfc9926..0eece0321383b 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 BaseWindow, 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/native_window.h b/shell/browser/native_window.h index ad5ccc35a4d9f..a24b65a5d2bcb 100644 --- a/shell/browser/native_window.h +++ b/shell/browser/native_window.h @@ -166,6 +166,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 25435b1d9babe..fe46c6a888d1e 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 e64b7d6f3f3ac..5b78ecd27f0fc 100644 --- a/shell/browser/native_window_mac.mm +++ b/shell/browser/native_window_mac.mm @@ -1296,6 +1296,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 b79cc3f370ec3..0d1d7b23baeb9 100644 --- a/shell/browser/native_window_views.cc +++ b/shell/browser/native_window_views.cc @@ -1151,6 +1151,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 e1fff4a0d0754..b477bf7f28d4f 100644 --- a/shell/browser/native_window_views.h +++ b/shell/browser/native_window_views.h @@ -115,6 +115,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 6c12dd543a5c2..9c7b576ea6251 100644 --- a/spec-main/api-browser-view-spec.ts +++ b/spec-main/api-browser-view-spec.ts @@ -210,6 +210,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();