feat: add win.setTopBrowserView() so that BrowserViews can be raised (

#27007) (#27712) * 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 <sentialx@gmail.com> Co-authored-by: Stewart Lord <stew@offbynone.com>
electron · Feb 18, 2021 · 795200a · 795200a
1 parent 64d4e59
commit 795200a
Show file tree

Hide file tree

Showing 11 changed files with 108 additions and 0 deletions.
diff --git a/docs/api/browser-window.md b/docs/api/browser-window.md
@@ -1849,6 +1849,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
@@ -783,6 +783,25 @@ void BaseWindow::RemoveBrowserView(v8::Local<v8::Value> value) {
   }
 }
 
+void BaseWindow::SetTopBrowserView(v8::Local<v8::Value> value,
+                                   gin_helper::Arguments* args) {
+  gin::Handle<BrowserView> 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();
 }
@@ -1210,6 +1229,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
@@ -175,6 +175,8 @@ class BaseWindow : public gin_helper::TrackableObject<BaseWindow>,
   virtual void SetBrowserView(v8::Local<v8::Value> value);
   virtual void AddBrowserView(v8::Local<v8::Value> value);
   virtual void RemoveBrowserView(v8::Local<v8::Value> value);
+  virtual void SetTopBrowserView(v8::Local<v8::Value> value,
+                                 gin_helper::Arguments* args);
   virtual std::vector<v8::Local<v8::Value>> 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
@@ -377,6 +377,14 @@ void BrowserWindow::RemoveBrowserView(v8::Local<v8::Value> value) {
 #endif
 }
 
+void BrowserWindow::SetTopBrowserView(v8::Local<v8::Value> 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
@@ -84,6 +84,8 @@ class BrowserWindow : public BaseWindow,
   void SetBrowserView(v8::Local<v8::Value> value) override;
   void AddBrowserView(v8::Local<v8::Value> value) override;
   void RemoveBrowserView(v8::Local<v8::Value> value) override;
+  void SetTopBrowserView(v8::Local<v8::Value> value,
+                         gin_helper::Arguments* args) override;
   void ResetBrowserViews() override;
   void SetVibrancy(v8::Isolate* isolate, v8::Local<v8::Value> value) override;
   void OnWindowShow() override;

diff --git 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
@@ -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
@@ -1301,6 +1301,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
@@ -1104,6 +1104,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
@@ -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
@@ -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();