Repository: electron/electron (language: cpp)

Issue:
[Bug]: BrowserViews doesn't work correctly on macOS
### Preflight Checklist

- [X] I have read the [Contributing Guidelines](https://github.com/electron/electron/blob/main/CONTRIBUTING.md) for this project.
- [X] I agree to follow the [Code of Conduct](https://github.com/electron/electron/blob/main/CODE_OF_CONDUCT.md) that this project adheres to.
- [X] I have searched the [issue tracker](https://www.github.com/electron/electron/issues) for a bug report that matches the one I want to file, without success.

### Electron Version

30.0.0-nightly.20231214

### What operating system are you using?

macOS

### Operating System Version

13.5

### What arch are you using?

arm64 (including Apple Silicon)

### Last Known Working Electron version

30.0.0-nightly.20231213

### Expected Behavior

1. Launch following app
[views_problem.zip](https://github.com/electron/electron/files/14065597/views_problem.zip)
2. Wait for full cycle to complete (all three views show)

After that first view should be displayed again



### Actual Behavior

Views dissapear and none of them shows again.

### Testcase Gist URL

_No response_

### Additional Information

_No response_


Discussion / hints:

Fix patch:
diff --git a/shell/browser/api/electron_api_browser_window.cc b/shell/browser/api/electron_api_browser_window.cc
index 106af4a6acff7..acbbd88cb4684 100644
--- a/shell/browser/api/electron_api_browser_window.cc
+++ b/shell/browser/api/electron_api_browser_window.cc
@@ -77,6 +77,7 @@ BrowserWindow::BrowserWindow(gin::Arguments* args,
       WebContentsView::Create(isolate, web_preferences);
   DCHECK(web_contents_view.get());
   window_->AddDraggableRegionProvider(web_contents_view.get());
+  web_contents_view_.Reset(isolate, web_contents_view.ToV8());
 
   // Save a reference of the WebContents.
   gin::Handle<WebContents> web_contents =
@@ -92,7 +93,14 @@ BrowserWindow::BrowserWindow(gin::Arguments* args,
   InitWithArgs(args);
 
   // Install the content view after BaseWindow's JS code is initialized.
-  SetContentView(gin::CreateHandle<View>(isolate, web_contents_view.get()));
+  // The WebContentsView is added a sibling of BaseWindow's contentView (before
+  // it in the paint order) so that any views added to BrowserWindow's
+  // contentView will be painted on top of the BrowserWindow's WebContentsView.
+  // See https://github.com/electron/electron/pull/41256.
+  // Note that |GetContentsView|, confusingly, does not refer to the same thing
+  // as |BaseWindow::GetContentView|.
+  window()->GetContentsView()->AddChildViewAt(web_contents_view->view(), 0);
+  window()->GetContentsView()->DeprecatedLayoutImmediately();
 
   // Init window after everything has been setup.
   window()->InitFromOptions(options);
diff --git a/shell/browser/api/electron_api_browser_window.h b/shell/browser/api/electron_api_browser_window.h
index e5d580138f56f..df9f9754ce0fd 100644
--- a/shell/browser/api/electron_api_browser_window.h
+++ b/shell/browser/api/electron_api_browser_window.h
@@ -95,6 +95,7 @@ class BrowserWindow : public BaseWindow,
   base::CancelableRepeatingClosure window_unresponsive_closure_;
 
   v8::Global<v8::Value> web_contents_;
+  v8::Global<v8::Value> web_contents_view_;
   base::WeakPtr<api::WebContents> api_web_contents_;
 
   base::WeakPtrFactory<BrowserWindow> weak_factory_{this};
diff --git a/shell/browser/api/frame_subscriber.cc b/shell/browser/api/frame_subscriber.cc
index 358ce6e116466..cdb65c76677f4 100644
--- a/shell/browser/api/frame_subscriber.cc
+++ b/shell/browser/api/frame_subscriber.cc
@@ -42,6 +42,7 @@ void FrameSubscriber::AttachToHost(content::RenderWidgetHost* host) {
 
   // Create and configure the video capturer.
   gfx::Size size = GetRenderViewSize();
+  DCHECK(!size.IsEmpty());
   video_capturer_ = host_->GetView()->CreateVideoCapturer();
   video_capturer_->SetResolutionConstraints(size, size, true);
   video_capturer_->SetAutoThrottlingEnabled(false);
diff --git a/shell/browser/native_window_views.cc b/shell/browser/native_window_views.cc
index 86dbfc1e9c2e2..0528acdadabf8 100644
--- a/shell/browser/native_window_views.cc
+++ b/shell/browser/native_window_views.cc
@@ -469,12 +469,12 @@ void NativeWindowViews::SetGTKDarkThemeEnabled(bool use_dark_theme) {
 
 void NativeWindowViews::SetContentView(views::View* view) {
   if (content_view()) {
-    root_view_.RemoveChildView(content_view());
+    root_view_.GetMainView()->RemoveChildView(content_view());
   }
   set_content_view(view);
   focused_view_ = view;
-  root_view_.AddChildView(content_view());
-  root_view_.DeprecatedLayoutImmediately();
+  root_view_.GetMainView()->AddChildView(content_view());
+  root_view_.GetMainView()->DeprecatedLayoutImmediately();
 }
 
 void NativeWindowViews::Close() {
@@ -1664,7 +1664,7 @@ std::u16string NativeWindowViews::GetWindowTitle() const {
 }
 
 views::View* NativeWindowViews::GetContentsView() {
-  return &root_view_;
+  return root_view_.GetMainView();
 }
 
 bool NativeWindowViews::ShouldDescendIntoChildForEventHandling(
@@ -1674,7 +1674,7 @@ bool NativeWindowViews::ShouldDescendIntoChildForEventHandling(
 }
 
 views::ClientView* NativeWindowViews::CreateClientView(views::Widget* widget) {
-  return new NativeWindowClientView{widget, GetContentsView(), this};
+  return new NativeWindowClientView{widget, &root_view_, this};
 }
 
 std::unique_ptr<views::NonClientFrameView>
diff --git a/shell/browser/ui/views/root_view.cc b/shell/browser/ui/views/root_view.cc
index 273f96f48ba44..b0f59e994b990 100644
--- a/shell/browser/ui/views/root_view.cc
+++ b/shell/browser/ui/views/root_view.cc
@@ -9,18 +9,12 @@
 #include "content/public/common/input/native_web_keyboard_event.h"
 #include "shell/browser/native_window.h"
 #include "shell/browser/ui/views/menu_bar.h"
+#include "ui/views/layout/box_layout.h"
 
 namespace electron {
 
 namespace {
 
-// The menu bar height in pixels.
-#if BUILDFLAG(IS_WIN)
-const int kMenuBarHeight = 20;
-#else
-const int kMenuBarHeight = 25;
-#endif
-
 bool IsAltKey(const content::NativeWebKeyboardEvent& event) {
   return event.windows_key_code == ui::VKEY_MENU;
 }
@@ -41,6 +35,12 @@ RootView::RootView(NativeWindow* window)
     : window_(window),
       last_focused_view_tracker_(std::make_unique<views::ViewTracker>()) {
   set_owned_by_client();
+  views::BoxLayout* layout =
+      SetLayoutManager(std::make_unique<views::BoxLayout>(
+          views::BoxLayout::Orientation::kVertical));
+  main_view_ = AddChildView(std::make_unique<views::View>());
+  main_view_->SetUseDefaultFillLayout(true);
+  layout->SetFlexForView(main_view_, 1);
 }
 
 RootView::~RootView() = default;
@@ -77,7 +77,7 @@ bool RootView::HasMenu() const {
 }
 
 int RootView::GetMenuBarHeight() const {
-  return kMenuBarHeight;
+  return menu_bar_ ? menu_bar_->GetPreferredSize().height() : 0;
 }
 
 void RootView::SetAutoHideMenuBar(bool auto_hide) {
@@ -90,10 +90,8 @@ void RootView::SetMenuBarVisibility(bool visible) {
 
   menu_bar_visible_ = visible;
   if (visible) {
-    DCHECK_EQ(children().size(), 1ul);
-    AddChildView(menu_bar_.get());
+    AddChildViewAt(menu_bar_.get(), 0);
   } else {
-    DCHECK_EQ(children().size(), 2ul);
     RemoveChildView(menu_bar_.get());
   }
 
@@ -166,21 +164,6 @@ void RootView::ResetAltState() {
   menu_bar_alt_pressed_ = false;
 }
 
-void RootView::Layout(PassKey) {
-  if (!window_->content_view())  // Not ready yet.
-    return;
-
-  const auto menu_bar_bounds =
-      menu_bar_visible_ ? gfx::Rect(0, 0, size().width(), kMenuBarHeight)
-                        : gfx::Rect();
-  if (menu_bar_)
-    menu_bar_->SetBoundsRect(menu_bar_bounds);
-
-  window_->content_view()->SetBoundsRect(
-      gfx::Rect(0, menu_bar_visible_ ? menu_bar_bounds.bottom() : 0,
-                size().width(), size().height() - menu_bar_bounds.height()));
-}
-
 gfx::Size RootView::GetMinimumSize() const {
   return window_->GetMinimumSize();
 }
diff --git a/shell/browser/ui/views/root_view.h b/shell/browser/ui/views/root_view.h
index 7aa52a22c1a81..5f27e23c60723 100644
--- a/shell/browser/ui/views/root_view.h
+++ b/shell/browser/ui/views/root_view.h
@@ -46,8 +46,9 @@ class RootView : public views::View {
   void RegisterAcceleratorsWithFocusManager(ElectronMenuModel* menu_model);
   void UnregisterAcceleratorsWithFocusManager();
 
+  views::View* GetMainView() { return main_view_; }
+
   // views::View:
-  void Layout(PassKey) override;
   gfx::Size GetMinimumSize() const override;
   gfx::Size GetMaximumSize() const override;
   bool AcceleratorPressed(const ui::Accelerator& accelerator) override;
@@ -62,6 +63,9 @@ class RootView : public views::View {
   bool menu_bar_visible_ = false;
   bool menu_bar_alt_pressed_ = false;
 
+  // Main view area.
+  raw_ptr<views::View> main_view_;
+
   // Map from accelerator to menu item's command id.
   accelerator_util::AcceleratorTable accelerator_table_;
 
diff --git a/spec/api-browser-window-spec.ts b/spec/api-browser-window-spec.ts
index 1ed5d2f4807ae..64c794163f94d 100644
--- a/spec/api-browser-window-spec.ts
+++ b/spec/api-browser-window-spec.ts
@@ -31,7 +31,7 @@ const isScaleFactorRounding = () => {
 
 const expectBoundsEqual = (actual: any, expected: any) => {
   if (!isScaleFactorRounding()) {
-    expect(expected).to.deep.equal(actual);
+    expect(actual).to.deep.equal(expected);
   } else if (Array.isArray(actual)) {
     expect(actual[0]).to.be.closeTo(expected[0], 1);
     expect(actual[1]).to.be.closeTo(expected[1], 1);


Test patch: