fix: ensure `BrowserView` bounds are always relative to window (#39605)

fix: ensure BrowserView bounds are always relative to window

shell/browser/api/electron_api_base_window.cc

@@ -768,10 +768,20 @@ void BaseWindow::AddBrowserView(gin::Handle<BrowserView> browser_view) {
       browser_view->SetOwnerWindow(nullptr);
     }
 
+    // If the user set the BrowserView's bounds before adding it to the window,
+    // we need to get those initial bounds *before* adding it to the window
+    // so bounds isn't returned relative despite not being correctly positioned
+    // relative to the window.
+    auto bounds = browser_view->GetBounds();
+
     window_->AddBrowserView(browser_view->view());
     window_->AddDraggableRegionProvider(browser_view.get());
     browser_view->SetOwnerWindow(this);
     browser_views_.emplace_back().Reset(isolate(), browser_view.ToV8());
+
+    // Recalibrate bounds relative to the containing window.
+    if (!bounds.IsEmpty())
+      browser_view->SetBounds(bounds);
   }
 }
 

shell/browser/api/electron_api_browser_view.h

@@ -66,6 +66,9 @@ class BrowserView : public gin::Wrappable<BrowserView>,
   BrowserView(const BrowserView&) = delete;
   BrowserView& operator=(const BrowserView&) = delete;
 
+  gfx::Rect GetBounds();
+  void SetBounds(const gfx::Rect& bounds);
+
  protected:
   BrowserView(gin::Arguments* args, const gin_helper::Dictionary& options);
   ~BrowserView() override;
@@ -78,8 +81,6 @@ class BrowserView : public gin::Wrappable<BrowserView>,
 
  private:
   void SetAutoResize(AutoResizeFlags flags);
-  void SetBounds(const gfx::Rect& bounds);
-  gfx::Rect GetBounds();
   void SetBackgroundColor(const std::string& color_name);
   v8::Local<v8::Value> GetWebContents(v8::Isolate*);
 

shell/browser/native_browser_view_mac.mm

@@ -55,25 +55,27 @@ void NativeBrowserViewMac::SetBounds(const gfx::Rect& bounds) {
   auto* iwc_view = GetInspectableWebContentsView();
   if (!iwc_view)
     return;
+
   auto* view = iwc_view->GetNativeView().GetNativeNSView();
-  auto* superview = view.superview;
-  const auto superview_height = superview ? superview.frame.size.height : 0;
-  view.frame =
-      NSMakeRect(bounds.x(), superview_height - bounds.y() - bounds.height(),
-                 bounds.width(), bounds.height());
+  const auto superview_height =
+      view.superview ? view.superview.frame.size.height : 0;
+  int y_coord = superview_height - bounds.y() - bounds.height();
+
+  view.frame = NSMakeRect(bounds.x(), y_coord, bounds.width(), bounds.height());
 }
 
 gfx::Rect NativeBrowserViewMac::GetBounds() {
   auto* iwc_view = GetInspectableWebContentsView();
   if (!iwc_view)
     return gfx::Rect();
+
   NSView* view = iwc_view->GetNativeView().GetNativeNSView();
   const int superview_height =
-      (view.superview) ? view.superview.frame.size.height : 0;
-  return gfx::Rect(
-      view.frame.origin.x,
-      superview_height - view.frame.origin.y - view.frame.size.height,
-      view.frame.size.width, view.frame.size.height);
+      view.superview ? view.superview.frame.size.height : 0;
+  int y_coord = superview_height - view.frame.origin.y - view.frame.size.height;
+
+  return gfx::Rect(view.frame.origin.x, y_coord, view.frame.size.width,
+                   view.frame.size.height);
 }
 
 void NativeBrowserViewMac::SetBackgroundColor(SkColor color) {

spec/api-browser-view-spec.ts

@@ -147,6 +147,41 @@ describe('BrowserView module', () => {
         view.setBounds({} as any);
       }).to.throw(/conversion failure/);
     });
+
+    it('can set bounds after view is added to window', () => {
+      view = new BrowserView();
+
+      const bounds = { x: 0, y: 0, width: 50, height: 50 };
+
+      w.addBrowserView(view);
+      view.setBounds(bounds);
+
+      expect(view.getBounds()).to.deep.equal(bounds);
+    });
+
+    it('can set bounds before view is added to window', () => {
+      view = new BrowserView();
+
+      const bounds = { x: 0, y: 0, width: 50, height: 50 };
+
+      view.setBounds(bounds);
+      w.addBrowserView(view);
+
+      expect(view.getBounds()).to.deep.equal(bounds);
+    });
+
+    it('can update bounds', () => {
+      view = new BrowserView();
+      w.addBrowserView(view);
+
+      const bounds1 = { x: 0, y: 0, width: 50, height: 50 };
+      view.setBounds(bounds1);
+      expect(view.getBounds()).to.deep.equal(bounds1);
+
+      const bounds2 = { x: 0, y: 150, width: 50, height: 50 };
+      view.setBounds(bounds2);
+      expect(view.getBounds()).to.deep.equal(bounds2);
+    });
   });
 
   describe('BrowserView.getBounds()', () => {
@@ -156,6 +191,16 @@ describe('BrowserView module', () => {
       view.setBounds(bounds);
       expect(view.getBounds()).to.deep.equal(bounds);
     });
+
+    it('does not changer after being added to a window', () => {
+      view = new BrowserView();
+      const bounds = { x: 10, y: 20, width: 30, height: 40 };
+      view.setBounds(bounds);
+      expect(view.getBounds()).to.deep.equal(bounds);
+
+      w.addBrowserView(view);
+      expect(view.getBounds()).to.deep.equal(bounds);
+    });
   });
 
   describe('BrowserWindow.setBrowserView()', () => {