From f93cc252ba7214ab731bc8307b9021bc8104fe15 Mon Sep 17 00:00:00 2001
From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com>
Date: Thu, 12 Sep 2024 19:20:47 +0200
Subject: [PATCH] fix: native `View` wrapper crash missing when adding child
 view (#43697)

fix: native View wrapper crash missing when adding child view

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
---
 shell/browser/ui/cocoa/delayed_native_view_host.mm | 10 +++-------
 spec/api-view-spec.ts                              | 12 ++++++++++++
 spec/api-web-contents-view-spec.ts                 | 14 ++++++++++++++
 3 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/shell/browser/ui/cocoa/delayed_native_view_host.mm b/shell/browser/ui/cocoa/delayed_native_view_host.mm
index e8f4f0305dd3..6d594e964717 100644
--- a/shell/browser/ui/cocoa/delayed_native_view_host.mm
+++ b/shell/browser/ui/cocoa/delayed_native_view_host.mm
@@ -15,13 +15,9 @@ DelayedNativeViewHost::~DelayedNativeViewHost() = default;
 
 void DelayedNativeViewHost::ViewHierarchyChanged(
     const views::ViewHierarchyChangedDetails& details) {
-  // NativeViewHost doesn't expect to have children, so filter the
-  // ViewHierarchyChanged events before passing them on.
-  if (details.child == this) {
-    NativeViewHost::ViewHierarchyChanged(details);
-    if (details.is_add && GetWidget() && !native_view())
-      Attach(native_view_);
-  }
+  NativeViewHost::ViewHierarchyChanged(details);
+  if (details.is_add && GetWidget() && !native_view())
+    Attach(native_view_);
 }
 
 bool DelayedNativeViewHost::OnMousePressed(const ui::MouseEvent& ui_event) {
diff --git a/spec/api-view-spec.ts b/spec/api-view-spec.ts
index 5af674785c82..27946bd4de92 100644
--- a/spec/api-view-spec.ts
+++ b/spec/api-view-spec.ts
@@ -36,6 +36,18 @@ describe('View', () => {
     expect(w.contentView.children).to.have.lengthOf(1);
   });
 
+  it('can be added as a child of another View', async () => {
+    const w = new BaseWindow();
+    const v1 = new View();
+    const v2 = new View();
+
+    v1.addChildView(v2);
+    w.contentView.addChildView(v1);
+
+    expect(w.contentView.children).to.deep.equal([v1]);
+    expect(v1.children).to.deep.equal([v2]);
+  });
+
   it('correctly reorders children', () => {
     w = new BaseWindow({ show: false });
     const cv = new View();
diff --git a/spec/api-web-contents-view-spec.ts b/spec/api-web-contents-view-spec.ts
index dbfa1d17dbe9..29821fd0f210 100644
--- a/spec/api-web-contents-view-spec.ts
+++ b/spec/api-web-contents-view-spec.ts
@@ -135,6 +135,20 @@ describe('WebContentsView', () => {
     expect(w.isFullScreen()).to.be.true('isFullScreen');
   });
 
+  it('can be added as a child of another View', async () => {
+    const w = new BaseWindow();
+    const v = new View();
+    const wcv = new WebContentsView();
+
+    await wcv.webContents.loadURL('data:text/html,<div id="div">This is a simple div.</div>');
+
+    v.addChildView(wcv);
+    w.contentView.addChildView(v);
+
+    expect(w.contentView.children).to.deep.equal([v]);
+    expect(v.children).to.deep.equal([wcv]);
+  });
+
   describe('visibilityState', () => {
     it('is initially hidden', async () => {
       const v = new WebContentsView();