Browse Source

Fix crash when using file chooser in BrowserView

The crash was a segfault caused by a null `WebDialogHelper::window_`.
Birunthan Mohanathas 7 years ago
parent
commit
eb19562316

+ 1 - 0
atom/browser/api/atom_api_browser_view.h

@@ -36,6 +36,7 @@ class BrowserView : public mate::TrackableObject<BrowserView> {
   static void BuildPrototype(v8::Isolate* isolate,
                              v8::Local<v8::FunctionTemplate> prototype);
 
+  WebContents* web_contents() const { return api_web_contents_; }
   NativeBrowserView* view() const { return view_.get(); }
 
   int32_t ID() const;

+ 18 - 1
atom/browser/api/atom_api_window.cc

@@ -186,6 +186,8 @@ void Window::OnWindowClosed() {
 
   RemoveFromParentChildWindows();
 
+  ResetBrowserView();
+
   // Destroy the native class when window is closed.
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, GetDestroyClosure());
@@ -838,16 +840,31 @@ v8::Local<v8::Value> Window::GetBrowserView() const {
 }
 
 void Window::SetBrowserView(v8::Local<v8::Value> value) {
+  ResetBrowserView();
+
   mate::Handle<BrowserView> browser_view;
   if (value->IsNull()) {
     window_->SetBrowserView(nullptr);
-    browser_view_.Reset();
   } else if (mate::ConvertFromV8(isolate(), value, &browser_view)) {
     window_->SetBrowserView(browser_view->view());
+    browser_view->web_contents()->SetOwnerWindow(window_.get());
     browser_view_.Reset(isolate(), value);
   }
 }
 
+void Window::ResetBrowserView() {
+  if (browser_view_.IsEmpty()) {
+    return;
+  }
+
+  mate::Handle<BrowserView> browser_view;
+  if (mate::ConvertFromV8(isolate(), GetBrowserView(), &browser_view)) {
+    browser_view->web_contents()->SetOwnerWindow(nullptr);
+  }
+
+  browser_view_.Reset();
+}
+
 bool Window::IsModal() const {
   return window_->is_modal();
 }

+ 1 - 0
atom/browser/api/atom_api_window.h

@@ -185,6 +185,7 @@ class Window : public mate::TrackableObject<Window>,
   std::vector<v8::Local<v8::Object>> GetChildWindows() const;
   v8::Local<v8::Value> GetBrowserView() const;
   void SetBrowserView(v8::Local<v8::Value> value);
+  void ResetBrowserView();
   bool IsModal() const;
   v8::Local<v8::Value> GetNativeWindowHandle();
 

+ 0 - 4
atom/browser/native_browser_view.h

@@ -18,10 +18,6 @@ class Rect;
 
 namespace atom {
 
-namespace api {
-class WebContents;
-}
-
 enum AutoResizeFlags {
   kAutoResizeWidth = 0x1,
   kAutoResizeHeight = 0x2,

+ 4 - 0
atom/browser/web_dialog_helper.cc

@@ -195,6 +195,10 @@ void WebDialogHelper::RunFileChooser(
 
     AtomBrowserContext* browser_context = static_cast<AtomBrowserContext*>(
         window_->web_contents()->GetBrowserContext());
+    if (!browser_context) {
+      browser_context = static_cast<atom::AtomBrowserContext*>(
+          render_frame_host->GetProcess()->GetBrowserContext());
+    }
     settings.default_path = browser_context->prefs()->GetFilePath(
         prefs::kSelectFileLastDirectory).Append(params.default_file_name);
     settings.properties = flags;

+ 12 - 1
spec/api-browser-view-spec.js

@@ -6,7 +6,7 @@ const {closeWindow} = require('./window-helpers')
 const {remote} = require('electron')
 const {BrowserView, BrowserWindow} = remote
 
-describe('View module', function () {
+describe('BrowserView module', function () {
   var w = null
   var view = null
 
@@ -89,4 +89,15 @@ describe('View module', function () {
       w.setBrowserView(view)
     })
   })
+
+  describe('BrowserView.webContents.getOwnerBrowserWindow()', function () {
+    it('points to owning window', function () {
+      view = new BrowserView()
+      assert.ok(!view.webContents.getOwnerBrowserWindow())
+      w.setBrowserView(view)
+      assert.equal(view.webContents.getOwnerBrowserWindow(), w)
+      w.setBrowserView(null)
+      assert.ok(!view.webContents.getOwnerBrowserWindow())
+    })
+  })
 })