fix: move BrowserWindow's WebContentsView to be a child of rootview (#41802)

fix: move BrowserWindow's WebContentsView to be a child of rootview (#41256)

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Jeremy Rose <jeremya@chromium.org>

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);

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};

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);

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_.GetMainView()->AddChildView(content_view());
-  root_view_.DeprecatedLayoutImmediately();
+  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>

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);
+    AddChildViewAt(menu_bar_.get(), 0);
-    AddChildView(menu_bar_.get());
   } 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();
 }

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_;
 

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);