Repository: electron/electron
Pull request: fix: segfault when moving WebContentsView between BrowserWindows

#### Description of Change
- Fixes #44571
The root of this issue is that View was not properly accounting for children that had been removed by being moved over to another View.  This PR fixes that issue by using `OnChildViewRemoved` from `views::ViewObserver` to handle this use case.

This PR also adds a test case to verify this fix.

Here is a run with that test case before the fix has been applied:
https://github.com/electron/electron/actions/runs/11748536232/job/32734670103?pr=44599

<!--
Thank you for your Pull Request. Please provide a description above and review
the requirements below.

Contributors guide: https://github.com/electron/electron/blob/main/CONTRIBUTING.md
-->

#### Checklist
<!-- Remove items that do not apply. For completed items, change [ ] to [x]. -->

- [ ] PR description included and stakeholders cc'd
- [ ] `npm test` passes
- [ ] tests are [changed or added](https://github.com/electron/electron/blob/main/docs/development/testing.md)
- [ ] relevant documentation, tutorials, templates and examples are changed or added
- [ ] [PR release notes](https://github.com/electron/clerk/blob/main/README.md) describe the change in a way relevant to app developers, and are [capitalized, punctuated, and past tense](https://github.com/electron/clerk/blob/main/README.md#examples).

#### Release Notes

Notes: <!-- Please add a one-line description for app developers to read in the release notes, or 'none' if no notes relevant to app developers. Examples and help on special cases: https://github.com/electron/clerk/blob/main/README.md#examples -->Fixed segfault when moving WebContentsView between BrowserWindows.


Changed files:
filename: shell/browser/api/electron_api_view.cc
patch: @@ -124,6 +124,15 @@ struct Converter<views::FlexAllocationOrder> {
   }
 };
 
+template <>
+struct Converter<electron::api::View> {
+  static bool FromV8(v8::Isolate* isolate,
+                     v8::Local<v8::Value> val,
+                     electron::api::View* out) {
+    return gin::ConvertFromV8(isolate, val, &out);
+  }
+};
+
 template <>
 struct Converter<views::SizeBound> {
   static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
@@ -257,11 +266,13 @@ void View::RemoveChildView(gin::Handle<View> child) {
 #if BUILDFLAG(IS_MAC)
     ScopedCAActionDisabler disable_animations;
 #endif
+    // Remove from child_views first so that OnChildViewRemoved doesn't try to
+    // remove it again
+    child_views_.erase(it);
     // It's possible for the child's view to be invalid here
     // if the child's webContents was closed or destroyed.
     if (child->view())
       view_->RemoveChildView(child->view());
-    child_views_.erase(it);
   }
 }
 
@@ -388,6 +399,19 @@ void View::OnViewIsDeleting(views::View* observed_view) {
   view_ = nullptr;
 }
 
+void View::OnChildViewRemoved(views::View* observed_view, views::View* child) {
+  v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
+  auto it = std::ranges::find_if(
+      child_views_, [&](const v8::Global<v8::Object>& child_view) {
+        View current_view;
+        gin::ConvertFromV8(isolate, child_view.Get(isolate), &current_view);
+        return current_view.view()->GetID() == child->GetID();
+      });
+  if (it != child_views_.end()) {
+    child_views_.erase(it);
+  }
+}
+
 // static
 gin_helper::WrappableBase* View::New(gin::Arguments* args) {
   View* view = new View();

filename: shell/browser/api/electron_api_view.h
patch: @@ -47,6 +47,8 @@ class View : public gin_helper::EventEmitter<View>,
   // views::ViewObserver
   void OnViewBoundsChanged(views::View* observed_view) override;
   void OnViewIsDeleting(views::View* observed_view) override;
+  void OnChildViewRemoved(views::View* observed_view,
+                          views::View* child) override;
 
   views::View* view() const { return view_; }
   std::optional<int> border_radius() const { return border_radius_; }

filename: spec/fixtures/crash-cases/webview-move-between-windows/index.js
patch: @@ -0,0 +1,31 @@
+const { app, BrowserWindow, WebContentsView } = require('electron');
+
+function createWindow () {
+  // Create the browser window.
+  const mainWindow = new BrowserWindow();
+  const secondaryWindow = new BrowserWindow();
+
+  const contentsView = new WebContentsView();
+  mainWindow.contentView.addChildView(contentsView);
+  mainWindow.webContents.setDevToolsWebContents(contentsView.webContents);
+  mainWindow.openDevTools();
+
+  contentsView.setBounds({
+    x: 400,
+    y: 0,
+    width: 400,
+    height: 600
+  });
+
+  setTimeout(() => {
+    secondaryWindow.contentView.addChildView(contentsView);
+    setTimeout(() => {
+      mainWindow.contentView.addChildView(contentsView);
+      app.quit();
+    }, 1000);
+  }, 1000);
+}
+
+app.whenReady().then(() => {
+  createWindow();
+});