Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: prevent destroyed view references from causing crashes #25509

12 changes: 11 additions & 1 deletion shell/browser/native_browser_view.cc
Expand Up @@ -13,16 +13,26 @@ namespace electron {

NativeBrowserView::NativeBrowserView(
InspectableWebContents* inspectable_web_contents)
: inspectable_web_contents_(inspectable_web_contents) {}
: inspectable_web_contents_(inspectable_web_contents) {
Observe(inspectable_web_contents_->GetWebContents());
}

NativeBrowserView::~NativeBrowserView() = default;

InspectableWebContentsView* NativeBrowserView::GetInspectableWebContentsView() {
if (!inspectable_web_contents_)
return nullptr;
return inspectable_web_contents_->GetView();
}

content::WebContents* NativeBrowserView::GetWebContents() {
if (!inspectable_web_contents_)
return nullptr;
return inspectable_web_contents_->GetWebContents();
}

void NativeBrowserView::WebContentsDestroyed() {
inspectable_web_contents_ = nullptr;
}

} // namespace electron
7 changes: 5 additions & 2 deletions shell/browser/native_browser_view.h
Expand Up @@ -9,6 +9,7 @@

#include "base/macros.h"
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_contents_observer.h"
#include "third_party/skia/include/core/SkColor.h"

namespace gfx {
Expand All @@ -27,9 +28,9 @@ enum AutoResizeFlags {
class InspectableWebContents;
class InspectableWebContentsView;

class NativeBrowserView {
class NativeBrowserView : public content::WebContentsObserver {
public:
virtual ~NativeBrowserView();
~NativeBrowserView() override;

static NativeBrowserView* Create(
InspectableWebContents* inspectable_web_contents);
Expand All @@ -52,6 +53,8 @@ class NativeBrowserView {

protected:
explicit NativeBrowserView(InspectableWebContents* inspectable_web_contents);
// content::WebContentsObserver:
void WebContentsDestroyed() override;

InspectableWebContents* inspectable_web_contents_;

Expand Down
37 changes: 25 additions & 12 deletions shell/browser/native_browser_view_mac.mm
Expand Up @@ -162,8 +162,10 @@ - (BOOL)mouseDownCanMoveWindow {
NativeBrowserViewMac::NativeBrowserViewMac(
InspectableWebContents* inspectable_web_contents)
: NativeBrowserView(inspectable_web_contents) {
auto* view =
GetInspectableWebContentsView()->GetNativeView().GetNativeNSView();
auto* iwc_view = GetInspectableWebContentsView();
if (!iwc_view)
return;
auto* view = iwc_view->GetNativeView().GetNativeNSView();
view.autoresizingMask = kDefaultAutoResizingMask;
}

Expand All @@ -186,14 +188,18 @@ - (BOOL)mouseDownCanMoveWindow {
NSViewMaxYMargin | NSViewMinYMargin | NSViewHeightSizable;
}

auto* view =
GetInspectableWebContentsView()->GetNativeView().GetNativeNSView();
auto* iwc_view = GetInspectableWebContentsView();
if (!iwc_view)
return;
auto* view = iwc_view->GetNativeView().GetNativeNSView();
view.autoresizingMask = autoresizing_mask;
}

void NativeBrowserViewMac::SetBounds(const gfx::Rect& bounds) {
auto* view =
GetInspectableWebContentsView()->GetNativeView().GetNativeNSView();
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 =
Expand All @@ -202,8 +208,10 @@ - (BOOL)mouseDownCanMoveWindow {
}

gfx::Rect NativeBrowserViewMac::GetBounds() {
NSView* view =
GetInspectableWebContentsView()->GetNativeView().GetNativeNSView();
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(
Expand All @@ -213,17 +221,22 @@ - (BOOL)mouseDownCanMoveWindow {
}

void NativeBrowserViewMac::SetBackgroundColor(SkColor color) {
auto* view =
GetInspectableWebContentsView()->GetNativeView().GetNativeNSView();
auto* iwc_view = GetInspectableWebContentsView();
if (!iwc_view)
return;
auto* view = iwc_view->GetNativeView().GetNativeNSView();
view.wantsLayer = YES;
view.layer.backgroundColor = skia::CGColorCreateFromSkColor(color);
}

void NativeBrowserViewMac::UpdateDraggableRegions(
const std::vector<gfx::Rect>& drag_exclude_rects) {
auto* iwc_view = GetInspectableWebContentsView();
auto* web_contents = GetWebContents();
if (!(iwc_view && web_contents))
return;
NSView* web_view = GetWebContents()->GetNativeView().GetNativeNSView();
NSView* inspectable_view =
GetInspectableWebContentsView()->GetNativeView().GetNativeNSView();
NSView* inspectable_view = iwc_view->GetNativeView().GetNativeNSView();
NSView* window_content_view = inspectable_view.superview;
const auto window_content_view_height = NSHeight(window_content_view.bounds);

Expand Down
30 changes: 24 additions & 6 deletions shell/browser/native_browser_view_views.cc
Expand Up @@ -26,7 +26,10 @@ void NativeBrowserViewViews::SetAutoResizeProportions(
const gfx::Size& window_size) {
if ((auto_resize_flags_ & AutoResizeFlags::kAutoResizeHorizontal) &&
!auto_horizontal_proportion_set_) {
auto* view = GetInspectableWebContentsView()->GetView();
auto* iwc_view = GetInspectableWebContentsView();
if (iwc_view)
return;
auto* view = iwc_view->GetView();
auto view_bounds = view->bounds();
auto_horizontal_proportion_width_ =
static_cast<float>(window_size.width()) /
Expand All @@ -37,7 +40,10 @@ void NativeBrowserViewViews::SetAutoResizeProportions(
}
if ((auto_resize_flags_ & AutoResizeFlags::kAutoResizeVertical) &&
!auto_vertical_proportion_set_) {
auto* view = GetInspectableWebContentsView()->GetView();
auto* iwc_view = GetInspectableWebContentsView();
if (iwc_view)
return;
auto* view = iwc_view->GetView();
auto view_bounds = view->bounds();
auto_vertical_proportion_height_ =
static_cast<float>(window_size.height()) /
Expand All @@ -51,7 +57,10 @@ void NativeBrowserViewViews::SetAutoResizeProportions(
void NativeBrowserViewViews::AutoResize(const gfx::Rect& new_window,
int width_delta,
int height_delta) {
auto* view = GetInspectableWebContentsView()->GetView();
auto* iwc_view = GetInspectableWebContentsView();
if (iwc_view)
return;
auto* view = iwc_view->GetView();
const auto flags = GetAutoResizeFlags();
if (!(flags & kAutoResizeWidth)) {
width_delta = 0;
Expand Down Expand Up @@ -92,17 +101,26 @@ void NativeBrowserViewViews::ResetAutoResizeProportions() {
}

void NativeBrowserViewViews::SetBounds(const gfx::Rect& bounds) {
auto* view = GetInspectableWebContentsView()->GetView();
auto* iwc_view = GetInspectableWebContentsView();
if (!iwc_view)
return;
auto* view = iwc_view->GetView();
view->SetBoundsRect(bounds);
ResetAutoResizeProportions();
}

gfx::Rect NativeBrowserViewViews::GetBounds() {
return GetInspectableWebContentsView()->GetView()->bounds();
auto* iwc_view = GetInspectableWebContentsView();
if (!iwc_view)
return gfx::Rect();
return iwc_view->GetView()->bounds();
}

void NativeBrowserViewViews::SetBackgroundColor(SkColor color) {
auto* view = GetInspectableWebContentsView()->GetView();
auto* iwc_view = GetInspectableWebContentsView();
if (!iwc_view)
return;
auto* view = iwc_view->GetView();
view->SetBackground(views::CreateSolidBackground(color));
view->SchedulePaint();
}
Expand Down
25 changes: 25 additions & 0 deletions spec-main/fixtures/crash-cases/api-browser-destroy/index.js
@@ -0,0 +1,25 @@
const { app, BrowserWindow, BrowserView } = require('electron');
const { expect } = require('chai');

function createWindow () {
// Create the browser window.
const mainWindow = new BrowserWindow({
width: 800,
height: 600,
webPreferences: {
nodeIntegration: true
}
});
const view = new BrowserView();
mainWindow.addBrowserView(view);
view.webContents.destroy();
view.setBounds({ x: 0, y: 0, width: 0, height: 0 });
const bounds = view.getBounds();
expect(bounds).to.deep.equal({ x: 0, y: 0, width: 0, height: 0 });
view.setBackgroundColor('#56cc5b10');
}

app.on('ready', () => {
createWindow();
setTimeout(() => app.quit());
});