From e058d11657542e7a5c4f4b6d528e77a9568e1ecf Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 7 May 2018 14:52:25 +0900 Subject: [PATCH 1/7] feat: add View API --- atom/browser/api/atom_api_url_request.cc | 2 + atom/browser/api/atom_api_url_request.h | 1 + atom/browser/api/atom_api_view.cc | 54 ++++++++++++++++++++++++ atom/browser/api/atom_api_view.h | 38 +++++++++++++++++ atom/common/node_bindings.cc | 1 + filenames.gypi | 3 ++ lib/browser/api/module-list.js | 1 + lib/browser/api/view.js | 8 ++++ 8 files changed, 108 insertions(+) create mode 100644 atom/browser/api/atom_api_view.cc create mode 100644 atom/browser/api/atom_api_view.h create mode 100644 lib/browser/api/view.js diff --git a/atom/browser/api/atom_api_url_request.cc b/atom/browser/api/atom_api_url_request.cc index 88582708c0..51c7969e31 100644 --- a/atom/browser/api/atom_api_url_request.cc +++ b/atom/browser/api/atom_api_url_request.cc @@ -3,7 +3,9 @@ // found in the LICENSE file. #include "atom/browser/api/atom_api_url_request.h" + #include + #include "atom/browser/api/atom_api_session.h" #include "atom/browser/net/atom_url_request.h" #include "atom/common/api/event_emitter_caller.h" diff --git a/atom/browser/api/atom_api_url_request.h b/atom/browser/api/atom_api_url_request.h index 372ac98ac6..3b5e667d75 100644 --- a/atom/browser/api/atom_api_url_request.h +++ b/atom/browser/api/atom_api_url_request.h @@ -7,6 +7,7 @@ #include #include + #include "atom/browser/api/event_emitter.h" #include "atom/browser/api/trackable_object.h" #include "base/memory/weak_ptr.h" diff --git a/atom/browser/api/atom_api_view.cc b/atom/browser/api/atom_api_view.cc new file mode 100644 index 0000000000..fdb63dc89f --- /dev/null +++ b/atom/browser/api/atom_api_view.cc @@ -0,0 +1,54 @@ +// Copyright (c) 2018 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#include "atom/browser/api/atom_api_view.h" + +#include "native_mate/dictionary.h" + +#include "atom/common/node_includes.h" + +namespace atom { + +namespace api { + +View::View() : view_(new views::View()) { + view_->set_owned_by_client(); +} + +View::~View() {} + +// static +mate::WrappableBase* View::New(mate::Arguments* args) { + return new View(); +} + +// static +void View::BuildPrototype(v8::Isolate* isolate, + v8::Local prototype) {} + +} // namespace api + +} // namespace atom + +namespace { + +using atom::api::View; + +void Initialize(v8::Local exports, + v8::Local unused, + v8::Local context, + void* priv) { + v8::Isolate* isolate = context->GetIsolate(); + View::SetConstructor(isolate, base::Bind(&View::New)); + + mate::Dictionary constructor(isolate, + View::GetConstructor(isolate)->GetFunction()); + + mate::Dictionary dict(isolate, exports); + dict.Set("View", constructor); +} + +} // namespace + +NODE_BUILTIN_MODULE_CONTEXT_AWARE(atom_browser_view, Initialize) diff --git a/atom/browser/api/atom_api_view.h b/atom/browser/api/atom_api_view.h new file mode 100644 index 0000000000..e60aa0329a --- /dev/null +++ b/atom/browser/api/atom_api_view.h @@ -0,0 +1,38 @@ +// Copyright (c) 2018 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef ATOM_BROWSER_API_ATOM_API_VIEW_H_ +#define ATOM_BROWSER_API_ATOM_API_VIEW_H_ + +#include + +#include "atom/browser/api/event_emitter.h" +#include "ui/views/view.h" + +namespace atom { + +namespace api { + +class View : public mate::EventEmitter { + public: + static mate::WrappableBase* New(mate::Arguments* args); + + static void BuildPrototype(v8::Isolate* isolate, + v8::Local prototype); + + protected: + View(); + ~View() override; + + private: + std::unique_ptr view_; + + DISALLOW_COPY_AND_ASSIGN(View); +}; + +} // namespace api + +} // namespace atom + +#endif // ATOM_BROWSER_API_ATOM_API_VIEW_H_ diff --git a/atom/common/node_bindings.cc b/atom/common/node_bindings.cc index 1f68688514..48359d9138 100644 --- a/atom/common/node_bindings.cc +++ b/atom/common/node_bindings.cc @@ -49,6 +49,7 @@ V(atom_browser_top_level_window) \ V(atom_browser_tray) \ V(atom_browser_web_contents) \ + V(atom_browser_view) \ V(atom_browser_web_view_manager) \ V(atom_browser_window) \ V(atom_common_asar) \ diff --git a/filenames.gypi b/filenames.gypi index 43f019e945..205da5ebf7 100644 --- a/filenames.gypi +++ b/filenames.gypi @@ -38,6 +38,7 @@ 'lib/browser/api/top-level-window.js', 'lib/browser/api/touch-bar.js', 'lib/browser/api/tray.js', + 'lib/browser/api/view.js', 'lib/browser/api/web-contents.js', 'lib/browser/chrome-extension.js', 'lib/browser/desktop-capturer.js', @@ -162,6 +163,8 @@ 'atom/browser/api/atom_api_tray.h', 'atom/browser/api/atom_api_url_request.cc', 'atom/browser/api/atom_api_url_request.h', + 'atom/browser/api/atom_api_view.cc', + 'atom/browser/api/atom_api_view.h', 'atom/browser/api/atom_api_web_contents.cc', 'atom/browser/api/atom_api_web_contents.h', 'atom/browser/api/atom_api_web_contents_mac.mm', diff --git a/lib/browser/api/module-list.js b/lib/browser/api/module-list.js index 68f41b491b..4d534d3018 100644 --- a/lib/browser/api/module-list.js +++ b/lib/browser/api/module-list.js @@ -22,6 +22,7 @@ module.exports = [ {name: 'TopLevelWindow', file: 'top-level-window'}, {name: 'TouchBar', file: 'touch-bar'}, {name: 'Tray', file: 'tray'}, + {name: 'View', file: 'view'}, {name: 'webContents', file: 'web-contents'}, // The internal modules, invisible unless you know their names. {name: 'NavigationController', file: 'navigation-controller', private: true} diff --git a/lib/browser/api/view.js b/lib/browser/api/view.js new file mode 100644 index 0000000000..ba215aeada --- /dev/null +++ b/lib/browser/api/view.js @@ -0,0 +1,8 @@ +'use strict' + +const {EventEmitter} = require('events') +const {View} = process.atomBinding('view') + +Object.setPrototypeOf(View.prototype, EventEmitter.prototype) + +module.exports = View From 5a320222e2118a3f305c9915006d71e1a6459156 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 7 May 2018 16:04:33 +0900 Subject: [PATCH 2/7] feat: add WebContentsView API --- atom/browser/api/atom_api_view.cc | 7 +- atom/browser/api/atom_api_view.h | 9 +- .../browser/api/atom_api_web_contents_view.cc | 85 +++++++++++++++++++ atom/browser/api/atom_api_web_contents_view.h | 46 ++++++++++ atom/common/node_bindings.cc | 1 + filenames.gypi | 3 + lib/browser/api/module-list.js | 1 + lib/browser/api/web-contents-view.js | 15 ++++ 8 files changed, 165 insertions(+), 2 deletions(-) create mode 100644 atom/browser/api/atom_api_web_contents_view.cc create mode 100644 atom/browser/api/atom_api_web_contents_view.h create mode 100644 lib/browser/api/web-contents-view.js diff --git a/atom/browser/api/atom_api_view.cc b/atom/browser/api/atom_api_view.cc index fdb63dc89f..f986835ac3 100644 --- a/atom/browser/api/atom_api_view.cc +++ b/atom/browser/api/atom_api_view.cc @@ -12,11 +12,16 @@ namespace atom { namespace api { +View::View(views::View* view) : view_(view) {} + View::View() : view_(new views::View()) { view_->set_owned_by_client(); } -View::~View() {} +View::~View() { + if (delete_view_) + delete view_; +} // static mate::WrappableBase* View::New(mate::Arguments* args) { diff --git a/atom/browser/api/atom_api_view.h b/atom/browser/api/atom_api_view.h index e60aa0329a..6be2423afc 100644 --- a/atom/browser/api/atom_api_view.h +++ b/atom/browser/api/atom_api_view.h @@ -21,12 +21,19 @@ class View : public mate::EventEmitter { static void BuildPrototype(v8::Isolate* isolate, v8::Local prototype); + views::View* view() const { return view_; } + protected: + explicit View(views::View* view); View(); ~View() override; + // Should delete the |view_| in destructor. + void set_delete_view(bool should) { delete_view_ = should; } + private: - std::unique_ptr view_; + bool delete_view_ = true; + views::View* view_ = nullptr; DISALLOW_COPY_AND_ASSIGN(View); }; diff --git a/atom/browser/api/atom_api_web_contents_view.cc b/atom/browser/api/atom_api_web_contents_view.cc new file mode 100644 index 0000000000..ef6d951e9e --- /dev/null +++ b/atom/browser/api/atom_api_web_contents_view.cc @@ -0,0 +1,85 @@ +// Copyright (c) 2018 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#include "atom/browser/api/atom_api_web_contents_view.h" + +#include "atom/browser/api/atom_api_web_contents.h" +#include "brightray/browser/inspectable_web_contents_view.h" +#include "native_mate/dictionary.h" +#include "ui/views/controls/native/native_view_host.h" + +#include "atom/common/node_includes.h" + +namespace atom { + +namespace api { + +WebContentsView::WebContentsView( + v8::Isolate* isolate, + v8::Local web_contents_wrapper, + brightray::InspectableWebContents* web_contents) +#if defined(OS_MACOSX) + : View(new views::NativeViewHost()), +#else + : View(web_contents->GetView()->GetView()), +#endif + web_contents_wrapper_(isolate, web_contents_wrapper) { +#if defined(OS_MACOSX) + // On macOS a View is created to wrap the NSView, and its lifetime is managed + // by us. + auto* host = static_cast(view()); + host->set_owned_by_client(); + host->Attach(web_contents->GetView()->GetNativeView()); +#else + // On other platforms the View is managed by InspectableWebContents. + set_delete_view(false); +#endif +} + +WebContentsView::~WebContentsView() {} + +// static +mate::WrappableBase* WebContentsView::New( + v8::Isolate* isolate, + mate::Handle web_contents) { + if (!web_contents->managed_web_contents()) { + const char* error = "The WebContents must be created by user"; + isolate->ThrowException( + v8::Exception::Error(mate::StringToV8(isolate, error))); + return nullptr; + } + return new WebContentsView(isolate, web_contents->GetWrapper(), + web_contents->managed_web_contents()); +} + +// static +void WebContentsView::BuildPrototype( + v8::Isolate* isolate, + v8::Local prototype) {} + +} // namespace api + +} // namespace atom + +namespace { + +using atom::api::WebContentsView; + +void Initialize(v8::Local exports, + v8::Local unused, + v8::Local context, + void* priv) { + v8::Isolate* isolate = context->GetIsolate(); + WebContentsView::SetConstructor(isolate, base::Bind(&WebContentsView::New)); + + mate::Dictionary constructor( + isolate, WebContentsView::GetConstructor(isolate)->GetFunction()); + + mate::Dictionary dict(isolate, exports); + dict.Set("WebContentsView", constructor); +} + +} // namespace + +NODE_BUILTIN_MODULE_CONTEXT_AWARE(atom_browser_web_contents_view, Initialize) diff --git a/atom/browser/api/atom_api_web_contents_view.h b/atom/browser/api/atom_api_web_contents_view.h new file mode 100644 index 0000000000..0dae03db0b --- /dev/null +++ b/atom/browser/api/atom_api_web_contents_view.h @@ -0,0 +1,46 @@ +// Copyright (c) 2018 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef ATOM_BROWSER_API_ATOM_API_WEB_CONTENTS_VIEW_H_ +#define ATOM_BROWSER_API_ATOM_API_WEB_CONTENTS_VIEW_H_ + +#include "atom/browser/api/atom_api_view.h" +#include "native_mate/handle.h" + +namespace brightray { +class InspectableWebContents; +} + +namespace atom { + +namespace api { + +class WebContents; + +class WebContentsView : public View { + public: + static mate::WrappableBase* New(v8::Isolate* isolate, + mate::Handle web_contents); + + static void BuildPrototype(v8::Isolate* isolate, + v8::Local prototype); + + protected: + WebContentsView(v8::Isolate* isolate, + v8::Local web_contents_wrapper, + brightray::InspectableWebContents* web_contents); + ~WebContentsView() override; + + private: + // Keep a reference to v8 wrapper. + v8::Global web_contents_wrapper_; + + DISALLOW_COPY_AND_ASSIGN(WebContentsView); +}; + +} // namespace api + +} // namespace atom + +#endif // ATOM_BROWSER_API_ATOM_API_WEB_CONTENTS_VIEW_H_ diff --git a/atom/common/node_bindings.cc b/atom/common/node_bindings.cc index 48359d9138..bdcf5f34e3 100644 --- a/atom/common/node_bindings.cc +++ b/atom/common/node_bindings.cc @@ -49,6 +49,7 @@ V(atom_browser_top_level_window) \ V(atom_browser_tray) \ V(atom_browser_web_contents) \ + V(atom_browser_web_contents_view) \ V(atom_browser_view) \ V(atom_browser_web_view_manager) \ V(atom_browser_window) \ diff --git a/filenames.gypi b/filenames.gypi index 205da5ebf7..bfb2967c3b 100644 --- a/filenames.gypi +++ b/filenames.gypi @@ -40,6 +40,7 @@ 'lib/browser/api/tray.js', 'lib/browser/api/view.js', 'lib/browser/api/web-contents.js', + 'lib/browser/api/web-contents-view.js', 'lib/browser/chrome-extension.js', 'lib/browser/desktop-capturer.js', 'lib/browser/guest-view-manager.js', @@ -168,6 +169,8 @@ 'atom/browser/api/atom_api_web_contents.cc', 'atom/browser/api/atom_api_web_contents.h', 'atom/browser/api/atom_api_web_contents_mac.mm', + 'atom/browser/api/atom_api_web_contents_view.cc', + 'atom/browser/api/atom_api_web_contents_view.h', 'atom/browser/api/atom_api_web_request.cc', 'atom/browser/api/atom_api_web_request.h', 'atom/browser/api/atom_api_web_view_manager.cc', diff --git a/lib/browser/api/module-list.js b/lib/browser/api/module-list.js index 4d534d3018..5b218838ac 100644 --- a/lib/browser/api/module-list.js +++ b/lib/browser/api/module-list.js @@ -24,6 +24,7 @@ module.exports = [ {name: 'Tray', file: 'tray'}, {name: 'View', file: 'view'}, {name: 'webContents', file: 'web-contents'}, + {name: 'WebContentsView', file: 'web-contents-view'}, // The internal modules, invisible unless you know their names. {name: 'NavigationController', file: 'navigation-controller', private: true} ] diff --git a/lib/browser/api/web-contents-view.js b/lib/browser/api/web-contents-view.js new file mode 100644 index 0000000000..d7428db7a0 --- /dev/null +++ b/lib/browser/api/web-contents-view.js @@ -0,0 +1,15 @@ +'use strict' + +const electron = require('electron') + +const {View} = electron +const {WebContentsView} = process.atomBinding('web_contents_view') + +Object.setPrototypeOf(WebContentsView.prototype, View.prototype) + +WebContentsView.prototype._init = function () { + // Call parent class's _init. + View.prototype._init.call(this) +} + +module.exports = WebContentsView From 640877ebf8853f54dc28855ee78ba69efb1f123b Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 8 May 2018 12:31:47 +0900 Subject: [PATCH 3/7] attach native view after widget is created --- .../browser/api/atom_api_web_contents_view.cc | 11 ++++--- .../ui/cocoa/delayed_native_view_host.cc | 21 +++++++++++++ .../ui/cocoa/delayed_native_view_host.h | 31 +++++++++++++++++++ filenames.gypi | 2 ++ 4 files changed, 60 insertions(+), 5 deletions(-) create mode 100644 atom/browser/ui/cocoa/delayed_native_view_host.cc create mode 100644 atom/browser/ui/cocoa/delayed_native_view_host.h diff --git a/atom/browser/api/atom_api_web_contents_view.cc b/atom/browser/api/atom_api_web_contents_view.cc index ef6d951e9e..e8ed06b8ae 100644 --- a/atom/browser/api/atom_api_web_contents_view.cc +++ b/atom/browser/api/atom_api_web_contents_view.cc @@ -7,7 +7,10 @@ #include "atom/browser/api/atom_api_web_contents.h" #include "brightray/browser/inspectable_web_contents_view.h" #include "native_mate/dictionary.h" -#include "ui/views/controls/native/native_view_host.h" + +#if defined(OS_MACOSX) +#include "atom/browser/ui/cocoa/delayed_native_view_host.h" +#endif #include "atom/common/node_includes.h" @@ -20,7 +23,7 @@ WebContentsView::WebContentsView( v8::Local web_contents_wrapper, brightray::InspectableWebContents* web_contents) #if defined(OS_MACOSX) - : View(new views::NativeViewHost()), + : View(new DelayedNativeViewHost(web_contents->GetView()->GetNativeView())), #else : View(web_contents->GetView()->GetView()), #endif @@ -28,9 +31,7 @@ WebContentsView::WebContentsView( #if defined(OS_MACOSX) // On macOS a View is created to wrap the NSView, and its lifetime is managed // by us. - auto* host = static_cast(view()); - host->set_owned_by_client(); - host->Attach(web_contents->GetView()->GetNativeView()); + view()->set_owned_by_client(); #else // On other platforms the View is managed by InspectableWebContents. set_delete_view(false); diff --git a/atom/browser/ui/cocoa/delayed_native_view_host.cc b/atom/browser/ui/cocoa/delayed_native_view_host.cc new file mode 100644 index 0000000000..0d48668895 --- /dev/null +++ b/atom/browser/ui/cocoa/delayed_native_view_host.cc @@ -0,0 +1,21 @@ +// Copyright (c) 2018 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#include "atom/browser/ui/cocoa/delayed_native_view_host.h" + +namespace atom { + +DelayedNativeViewHost::DelayedNativeViewHost(gfx::NativeView native_view) + : native_view_(native_view) {} + +DelayedNativeViewHost::~DelayedNativeViewHost() {} + +void DelayedNativeViewHost::ViewHierarchyChanged( + const ViewHierarchyChangedDetails& details) { + NativeViewHost::ViewHierarchyChanged(details); + if (details.is_add) + Attach(native_view_); +} + +} // namespace atom diff --git a/atom/browser/ui/cocoa/delayed_native_view_host.h b/atom/browser/ui/cocoa/delayed_native_view_host.h new file mode 100644 index 0000000000..2dfca1f964 --- /dev/null +++ b/atom/browser/ui/cocoa/delayed_native_view_host.h @@ -0,0 +1,31 @@ +// Copyright (c) 2018 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef ATOM_BROWSER_UI_COCOA_DELAYED_NATIVE_VIEW_HOST_H_ +#define ATOM_BROWSER_UI_COCOA_DELAYED_NATIVE_VIEW_HOST_H_ + +#include "ui/views/controls/native/native_view_host.h" + +namespace atom { + +// Automatically attach the native view after the NativeViewHost is attached to +// a widget. (Attaching it directly would cause crash.) +class DelayedNativeViewHost : public views::NativeViewHost { + public: + explicit DelayedNativeViewHost(gfx::NativeView native_view); + ~DelayedNativeViewHost() override; + + // views::View: + void ViewHierarchyChanged( + const ViewHierarchyChangedDetails& details) override; + + private: + gfx::NativeView native_view_; + + DISALLOW_COPY_AND_ASSIGN(DelayedNativeViewHost); +}; + +} // namespace atom + +#endif // ATOM_BROWSER_UI_COCOA_DELAYED_NATIVE_VIEW_HOST_H_ diff --git a/filenames.gypi b/filenames.gypi index bfb2967c3b..ec0c59f0a0 100644 --- a/filenames.gypi +++ b/filenames.gypi @@ -335,6 +335,8 @@ 'atom/browser/ui/cocoa/atom_preview_item.mm', 'atom/browser/ui/cocoa/atom_touch_bar.h', 'atom/browser/ui/cocoa/atom_touch_bar.mm', + 'atom/browser/ui/cocoa/delayed_native_view_host.cc', + 'atom/browser/ui/cocoa/delayed_native_view_host.h', 'atom/browser/ui/cocoa/views_delegate_mac.h', 'atom/browser/ui/cocoa/views_delegate_mac.mm', 'atom/browser/ui/cocoa/root_view_mac.mm', From 2b24b26e5966eddd1dca4744cfd8cb5064ff6f75 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 8 May 2018 12:51:27 +0900 Subject: [PATCH 4/7] refactor: do not pass WebContents to NativeWindow --- atom/browser/api/atom_api_browser_window.cc | 8 +++++- atom/browser/api/atom_api_browser_window.h | 2 ++ atom/browser/native_window.cc | 1 + atom/browser/native_window.h | 12 ++++----- atom/browser/native_window_mac.h | 10 +------- atom/browser/native_window_mac.mm | 13 ++++------ atom/browser/native_window_views.cc | 27 ++++++++++----------- atom/browser/native_window_views.h | 4 +-- 8 files changed, 36 insertions(+), 41 deletions(-) diff --git a/atom/browser/api/atom_api_browser_window.cc b/atom/browser/api/atom_api_browser_window.cc index 298aa12254..db98e4bcf4 100644 --- a/atom/browser/api/atom_api_browser_window.cc +++ b/atom/browser/api/atom_api_browser_window.cc @@ -4,6 +4,7 @@ #include "atom/browser/api/atom_api_browser_window.h" +#include "atom/browser/api/atom_api_web_contents_view.h" #include "atom/browser/browser.h" #include "atom/browser/unresponsive_suppressor.h" #include "atom/browser/web_contents_preferences.h" @@ -67,6 +68,11 @@ BrowserWindow::BrowserWindow(v8::Isolate* isolate, api_web_contents_->AddObserver(this); Observe(api_web_contents_->web_contents()); + // Create a WebContentsView for the WebContents. + auto* web_contents_view = static_cast( + WebContentsView::New(isolate, web_contents)); + web_contents_view_.Reset(isolate, web_contents_view->GetWrapper()); + // Keep a copy of the options for later use. mate::Dictionary(isolate, web_contents->GetWrapper()) .Set("browserWindowOptions", options); @@ -77,7 +83,7 @@ BrowserWindow::BrowserWindow(v8::Isolate* isolate, // Associate with BrowserWindow. web_contents->SetOwnerWindow(window()); - window()->SetContentView(web_contents->managed_web_contents()); + window()->SetContentView(web_contents_view->view()); auto* host = web_contents->web_contents()->GetRenderViewHost(); if (host) diff --git a/atom/browser/api/atom_api_browser_window.h b/atom/browser/api/atom_api_browser_window.h index 5f9610f3a3..3af4402d19 100644 --- a/atom/browser/api/atom_api_browser_window.h +++ b/atom/browser/api/atom_api_browser_window.h @@ -109,6 +109,8 @@ class BrowserWindow : public TopLevelWindow, v8::Global web_contents_; api::WebContents* api_web_contents_; + v8::Global web_contents_view_; + base::WeakPtrFactory weak_factory_; DISALLOW_COPY_AND_ASSIGN(BrowserWindow); diff --git a/atom/browser/native_window.cc b/atom/browser/native_window.cc index 85848e3d48..528e47dbb8 100644 --- a/atom/browser/native_window.cc +++ b/atom/browser/native_window.cc @@ -48,6 +48,7 @@ gfx::Size GetExpandedWindowSize(const NativeWindow* window, gfx::Size size) { NativeWindow::NativeWindow(const mate::Dictionary& options, NativeWindow* parent) : widget_(new views::Widget), + content_view_(nullptr), has_frame_(true), transparent_(false), enable_larger_than_screen_(false), diff --git a/atom/browser/native_window.h b/atom/browser/native_window.h index 89281f65c9..123f375fe1 100644 --- a/atom/browser/native_window.h +++ b/atom/browser/native_window.h @@ -20,10 +20,6 @@ class SkRegion; -namespace brightray { -class InspectableWebContents; -} - namespace content { struct NativeWebKeyboardEvent; } @@ -60,8 +56,7 @@ class NativeWindow : public base::SupportsUserData, void InitFromOptions(const mate::Dictionary& options); - virtual void SetContentView( - brightray::InspectableWebContents* web_contents) = 0; + virtual void SetContentView(views::View* view) = 0; virtual void Close() = 0; virtual void CloseImmediately() = 0; @@ -264,6 +259,7 @@ class NativeWindow : public base::SupportsUserData, } views::Widget* widget() const { return widget_.get(); } + views::View* content_view() const { return content_view_; } bool has_frame() const { return has_frame_; } void set_has_frame(bool has_frame) { has_frame_ = has_frame; } @@ -282,6 +278,7 @@ class NativeWindow : public base::SupportsUserData, views::Widget* GetWidget() override; const views::Widget* GetWidget() const override; + void set_content_view(views::View* view) { content_view_ = view; } void set_browser_view(NativeBrowserView* browser_view) { browser_view_ = browser_view; } @@ -289,6 +286,9 @@ class NativeWindow : public base::SupportsUserData, private: std::unique_ptr widget_; + // The content view, weak ref. + views::View* content_view_; + // Whether window has standard frame. bool has_frame_; diff --git a/atom/browser/native_window_mac.h b/atom/browser/native_window_mac.h index 0fb2359e0e..fcf40f5b31 100644 --- a/atom/browser/native_window_mac.h +++ b/atom/browser/native_window_mac.h @@ -21,10 +21,6 @@ @class CustomWindowButtonView; @class FullSizeContentView; -namespace views { -class NativeViewHost; -} - namespace atom { class RootViewMac; @@ -35,7 +31,7 @@ class NativeWindowMac : public NativeWindow { ~NativeWindowMac() override; // NativeWindow: - void SetContentView(brightray::InspectableWebContents* web_contents) override; + void SetContentView(views::View* view) override; void Close() override; void CloseImmediately() override; void Focus(bool focus) override; @@ -144,7 +140,6 @@ class NativeWindowMac : public NativeWindow { }; TitleBarStyle title_bar_style() const { return title_bar_style_; } - views::View* content_view() { return content_view_; } AtomPreviewItem* preview_item() const { return preview_item_.get(); } AtomTouchBar* touch_bar() const { return touch_bar_.get(); } bool zoom_to_page_width() const { return zoom_to_page_width_; } @@ -178,9 +173,6 @@ class NativeWindowMac : public NativeWindow { // The view that fills the client area. std::unique_ptr root_view_; - // The content view, managed by widget_. - views::NativeViewHost* content_view_; - bool is_kiosk_; bool was_fullscreen_; bool zoom_to_page_width_; diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm index 0ff821ac87..e08f7e5482 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -257,7 +257,6 @@ NativeWindowMac::NativeWindowMac(const mate::Dictionary& options, NativeWindow* parent) : NativeWindow(options, parent), root_view_(new RootViewMac(this)), - content_view_(nullptr), is_kiosk_(false), was_fullscreen_(false), zoom_to_page_width_(false), @@ -512,15 +511,13 @@ NativeWindowMac::~NativeWindowMac() { [NSEvent removeMonitor:wheel_event_monitor_]; } -void NativeWindowMac::SetContentView( - brightray::InspectableWebContents* web_contents) { +void NativeWindowMac::SetContentView(views::View* view) { views::View* root_view = GetContentsView(); - if (content_view_) - root_view->RemoveChildView(content_view_); + if (content_view()) + root_view->RemoveChildView(content_view()); - content_view_ = new views::NativeViewHost(); - root_view->AddChildView(content_view_); - content_view_->Attach(web_contents->GetView()->GetNativeView()); + set_content_view(view); + root_view->AddChildView(content_view()); if (buttons_view_) { // Ensure the buttons view are always floated on the top. diff --git a/atom/browser/native_window_views.cc b/atom/browser/native_window_views.cc index a391bc09c8..16ec70fcc7 100644 --- a/atom/browser/native_window_views.cc +++ b/atom/browser/native_window_views.cc @@ -102,7 +102,6 @@ NativeWindowViews::NativeWindowViews(const mate::Dictionary& options, NativeWindow* parent) : NativeWindow(options, parent), root_view_(new RootView(this)), - content_view_(nullptr), focused_view_(nullptr), #if defined(OS_WIN) checked_for_a11y_support_(false), @@ -304,19 +303,18 @@ NativeWindowViews::~NativeWindowViews() { #endif } -void NativeWindowViews::SetContentView( - brightray::InspectableWebContents* web_contents) { - if (content_view_) { - root_view_->RemoveChildView(content_view_); +void NativeWindowViews::SetContentView(views::View* view) { + if (content_view()) { + root_view_->RemoveChildView(content_view()); if (browser_view()) { - content_view_->RemoveChildView( + content_view()->RemoveChildView( browser_view()->GetInspectableWebContentsView()->GetView()); set_browser_view(nullptr); } } - content_view_ = web_contents->GetView()->GetView(); - focused_view_ = web_contents->GetView()->GetWebView(); - root_view_->AddChildView(content_view_); + set_content_view(view); + focused_view_ = view; + root_view_->AddChildView(content_view()); root_view_->Layout(); } @@ -560,7 +558,7 @@ gfx::Rect NativeWindowViews::GetBounds() { } gfx::Rect NativeWindowViews::GetContentBounds() { - return content_view_ ? content_view_->GetBoundsInScreen() : gfx::Rect(); + return content_view() ? content_view()->GetBoundsInScreen() : gfx::Rect(); } gfx::Size NativeWindowViews::GetContentSize() { @@ -569,7 +567,7 @@ gfx::Size NativeWindowViews::GetContentSize() { return NativeWindow::GetContentSize(); #endif - return content_view_ ? content_view_->size() : gfx::Size(); + return content_view() ? content_view()->size() : gfx::Size(); } void NativeWindowViews::SetContentSizeConstraints( @@ -924,11 +922,11 @@ void NativeWindowViews::SetMenu(AtomMenuModel* menu_model) { } void NativeWindowViews::SetBrowserView(NativeBrowserView* view) { - if (!content_view_) + if (!content_view()) return; if (browser_view()) { - content_view_->RemoveChildView( + content_view()->RemoveChildView( browser_view()->GetInspectableWebContentsView()->GetView()); set_browser_view(nullptr); } @@ -940,7 +938,8 @@ void NativeWindowViews::SetBrowserView(NativeBrowserView* view) { // Add as child of the main web view to avoid (0, 0) origin from overlapping // with menu bar. set_browser_view(view); - content_view_->AddChildView(view->GetInspectableWebContentsView()->GetView()); + content_view()->AddChildView( + view->GetInspectableWebContentsView()->GetView()); } void NativeWindowViews::SetParentWindow(NativeWindow* parent) { diff --git a/atom/browser/native_window_views.h b/atom/browser/native_window_views.h index fdf5e5772e..0bfe19ced2 100644 --- a/atom/browser/native_window_views.h +++ b/atom/browser/native_window_views.h @@ -44,7 +44,7 @@ class NativeWindowViews : public NativeWindow, ~NativeWindowViews() override; // NativeWindow: - void SetContentView(brightray::InspectableWebContents* web_contents) override; + void SetContentView(views::View* view) override; void Close() override; void CloseImmediately() override; void Focus(bool focus) override; @@ -135,7 +135,6 @@ class NativeWindowViews : public NativeWindow, void SetIcon(const gfx::ImageSkia& icon); #endif - views::View* content_view() const { return content_view_; } SkRegion* draggable_region() const { return draggable_region_.get(); } #if defined(OS_WIN) @@ -197,7 +196,6 @@ class NativeWindowViews : public NativeWindow, std::unique_ptr root_view_; - views::View* content_view_; // Weak ref. views::View* focused_view_; // The view should be focused by default. // The "resizable" flag on Linux is implemented by setting size constraints, From bb2715e7a5c588dc09bbdc25fd5636c665f3dbb9 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 8 May 2018 14:47:26 +0900 Subject: [PATCH 5/7] feat: add TopLevelWindow.setContentView API --- atom/browser/api/atom_api_browser_window.cc | 7 ------- atom/browser/api/atom_api_browser_window.h | 2 -- atom/browser/api/atom_api_top_level_window.cc | 15 +++++++++++++++ atom/browser/api/atom_api_top_level_window.h | 7 ++++++- atom/browser/api/atom_api_view.cc | 4 +++- atom/browser/api/atom_api_view.h | 17 +++++++++++++++++ atom/browser/api/atom_api_web_contents_view.cc | 12 +++++++----- atom/browser/api/atom_api_web_contents_view.h | 2 +- lib/browser/api/browser-window.js | 5 ++++- lib/browser/api/view.js | 3 +++ 10 files changed, 56 insertions(+), 18 deletions(-) diff --git a/atom/browser/api/atom_api_browser_window.cc b/atom/browser/api/atom_api_browser_window.cc index db98e4bcf4..ce85e52d43 100644 --- a/atom/browser/api/atom_api_browser_window.cc +++ b/atom/browser/api/atom_api_browser_window.cc @@ -4,7 +4,6 @@ #include "atom/browser/api/atom_api_browser_window.h" -#include "atom/browser/api/atom_api_web_contents_view.h" #include "atom/browser/browser.h" #include "atom/browser/unresponsive_suppressor.h" #include "atom/browser/web_contents_preferences.h" @@ -68,11 +67,6 @@ BrowserWindow::BrowserWindow(v8::Isolate* isolate, api_web_contents_->AddObserver(this); Observe(api_web_contents_->web_contents()); - // Create a WebContentsView for the WebContents. - auto* web_contents_view = static_cast( - WebContentsView::New(isolate, web_contents)); - web_contents_view_.Reset(isolate, web_contents_view->GetWrapper()); - // Keep a copy of the options for later use. mate::Dictionary(isolate, web_contents->GetWrapper()) .Set("browserWindowOptions", options); @@ -83,7 +77,6 @@ BrowserWindow::BrowserWindow(v8::Isolate* isolate, // Associate with BrowserWindow. web_contents->SetOwnerWindow(window()); - window()->SetContentView(web_contents_view->view()); auto* host = web_contents->web_contents()->GetRenderViewHost(); if (host) diff --git a/atom/browser/api/atom_api_browser_window.h b/atom/browser/api/atom_api_browser_window.h index 3af4402d19..5f9610f3a3 100644 --- a/atom/browser/api/atom_api_browser_window.h +++ b/atom/browser/api/atom_api_browser_window.h @@ -109,8 +109,6 @@ class BrowserWindow : public TopLevelWindow, v8::Global web_contents_; api::WebContents* api_web_contents_; - v8::Global web_contents_view_; - base::WeakPtrFactory weak_factory_; DISALLOW_COPY_AND_ASSIGN(BrowserWindow); diff --git a/atom/browser/api/atom_api_top_level_window.cc b/atom/browser/api/atom_api_top_level_window.cc index 99d24e0e40..9a168504ff 100644 --- a/atom/browser/api/atom_api_top_level_window.cc +++ b/atom/browser/api/atom_api_top_level_window.cc @@ -9,6 +9,7 @@ #include "atom/browser/api/atom_api_browser_view.h" #include "atom/browser/api/atom_api_menu.h" +#include "atom/browser/api/atom_api_view.h" #include "atom/browser/api/atom_api_web_contents.h" #include "atom/common/color_util.h" #include "atom/common/native_mate_converters/callback.h" @@ -267,6 +268,11 @@ void TopLevelWindow::OnWindowMessage(UINT message, } #endif +void TopLevelWindow::SetContentView(mate::Handle view) { + content_view_.Reset(isolate(), view.ToV8()); + window_->SetContentView(view->view()); +} + void TopLevelWindow::Close() { window_->Close(); } @@ -767,6 +773,13 @@ void TopLevelWindow::CloseFilePreview() { window_->CloseFilePreview(); } +v8::Local TopLevelWindow::GetContentView() const { + if (content_view_.IsEmpty()) + return v8::Null(isolate()); + else + return v8::Local::New(isolate(), content_view_); +} + v8::Local TopLevelWindow::GetParentWindow() const { if (parent_window_.IsEmpty()) return v8::Null(isolate()); @@ -915,6 +928,7 @@ void TopLevelWindow::BuildPrototype(v8::Isolate* isolate, prototype->SetClassName(mate::StringToV8(isolate, "TopLevelWindow")); mate::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate()) .MakeDestroyable() + .SetMethod("setContentView", &TopLevelWindow::SetContentView) .SetMethod("close", &TopLevelWindow::Close) .SetMethod("focus", &TopLevelWindow::Focus) .SetMethod("blur", &TopLevelWindow::Blur) @@ -1024,6 +1038,7 @@ void TopLevelWindow::BuildPrototype(v8::Isolate* isolate, .SetMethod("setAspectRatio", &TopLevelWindow::SetAspectRatio) .SetMethod("previewFile", &TopLevelWindow::PreviewFile) .SetMethod("closeFilePreview", &TopLevelWindow::CloseFilePreview) + .SetMethod("getContentView", &TopLevelWindow::GetContentView) .SetMethod("getParentWindow", &TopLevelWindow::GetParentWindow) .SetMethod("getChildWindows", &TopLevelWindow::GetChildWindows) .SetMethod("getBrowserView", &TopLevelWindow::GetBrowserView) diff --git a/atom/browser/api/atom_api_top_level_window.h b/atom/browser/api/atom_api_top_level_window.h index caba53c0e0..bf64ac7c61 100644 --- a/atom/browser/api/atom_api_top_level_window.h +++ b/atom/browser/api/atom_api_top_level_window.h @@ -20,6 +20,8 @@ namespace atom { namespace api { +class View; + class TopLevelWindow : public mate::TrackableObject, public NativeWindowObserver { public: @@ -79,6 +81,7 @@ class TopLevelWindow : public mate::TrackableObject, #endif // Public APIs of NativeWindow. + void SetContentView(mate::Handle view); void Close(); virtual void Focus(); virtual void Blur(); @@ -179,6 +182,7 @@ class TopLevelWindow : public mate::TrackableObject, void CloseFilePreview(); // Public getters of NativeWindow. + v8::Local GetContentView() const; v8::Local GetParentWindow() const; std::vector> GetChildWindows() const; v8::Local GetBrowserView() const; @@ -215,6 +219,7 @@ class TopLevelWindow : public mate::TrackableObject, MessageCallbackMap messages_callback_map_; #endif + v8::Global content_view_; v8::Global browser_view_; v8::Global menu_; v8::Global parent_window_; @@ -231,7 +236,7 @@ class TopLevelWindow : public mate::TrackableObject, namespace mate { -template<> +template <> struct Converter { static bool FromV8(v8::Isolate* isolate, v8::Local val, diff --git a/atom/browser/api/atom_api_view.cc b/atom/browser/api/atom_api_view.cc index f986835ac3..07db4bfca1 100644 --- a/atom/browser/api/atom_api_view.cc +++ b/atom/browser/api/atom_api_view.cc @@ -25,7 +25,9 @@ View::~View() { // static mate::WrappableBase* View::New(mate::Arguments* args) { - return new View(); + auto* view = new View(); + view->InitWith(args->isolate(), args->GetThis()); + return view; } // static diff --git a/atom/browser/api/atom_api_view.h b/atom/browser/api/atom_api_view.h index 6be2423afc..b1fb38e4c4 100644 --- a/atom/browser/api/atom_api_view.h +++ b/atom/browser/api/atom_api_view.h @@ -42,4 +42,21 @@ class View : public mate::EventEmitter { } // namespace atom +namespace mate { + +template <> +struct Converter { + static bool FromV8(v8::Isolate* isolate, + v8::Local val, + views::View** out) { + atom::api::View* view; + if (!Converter::FromV8(isolate, val, &view)) + return false; + *out = view->view(); + return true; + } +}; + +} // namespace mate + #endif // ATOM_BROWSER_API_ATOM_API_VIEW_H_ diff --git a/atom/browser/api/atom_api_web_contents_view.cc b/atom/browser/api/atom_api_web_contents_view.cc index e8ed06b8ae..5bfd94dfee 100644 --- a/atom/browser/api/atom_api_web_contents_view.cc +++ b/atom/browser/api/atom_api_web_contents_view.cc @@ -42,16 +42,18 @@ WebContentsView::~WebContentsView() {} // static mate::WrappableBase* WebContentsView::New( - v8::Isolate* isolate, + mate::Arguments* args, mate::Handle web_contents) { if (!web_contents->managed_web_contents()) { const char* error = "The WebContents must be created by user"; - isolate->ThrowException( - v8::Exception::Error(mate::StringToV8(isolate, error))); + args->isolate()->ThrowException( + v8::Exception::Error(mate::StringToV8(args->isolate(), error))); return nullptr; } - return new WebContentsView(isolate, web_contents->GetWrapper(), - web_contents->managed_web_contents()); + auto* view = new WebContentsView(args->isolate(), web_contents->GetWrapper(), + web_contents->managed_web_contents()); + view->InitWith(args->isolate(), args->GetThis()); + return view; } // static diff --git a/atom/browser/api/atom_api_web_contents_view.h b/atom/browser/api/atom_api_web_contents_view.h index 0dae03db0b..e3cf96f990 100644 --- a/atom/browser/api/atom_api_web_contents_view.h +++ b/atom/browser/api/atom_api_web_contents_view.h @@ -20,7 +20,7 @@ class WebContents; class WebContentsView : public View { public: - static mate::WrappableBase* New(v8::Isolate* isolate, + static mate::WrappableBase* New(mate::Arguments* args, mate::Handle web_contents); static void BuildPrototype(v8::Isolate* isolate, diff --git a/lib/browser/api/browser-window.js b/lib/browser/api/browser-window.js index 924a99c4be..cb523496d6 100644 --- a/lib/browser/api/browser-window.js +++ b/lib/browser/api/browser-window.js @@ -1,7 +1,7 @@ 'use strict' const electron = require('electron') -const {ipcMain, TopLevelWindow} = electron +const {ipcMain, WebContentsView, TopLevelWindow} = electron const {BrowserWindow} = process.atomBinding('window') const v8Util = process.atomBinding('v8_util') @@ -14,6 +14,9 @@ BrowserWindow.prototype._init = function () { // Avoid recursive require. const {app} = electron + // Create WebContentsView. + this.setContentView(new WebContentsView(this.webContents)) + // Make new windows requested by links behave like "window.open" this.webContents.on('-new-window', (event, url, frameName, disposition, additionalFeatures, postData, diff --git a/lib/browser/api/view.js b/lib/browser/api/view.js index ba215aeada..53fb96e3fa 100644 --- a/lib/browser/api/view.js +++ b/lib/browser/api/view.js @@ -5,4 +5,7 @@ const {View} = process.atomBinding('view') Object.setPrototypeOf(View.prototype, EventEmitter.prototype) +View.prototype._init = function () { +} + module.exports = View From ea97f431451b7c1800a76aa32c514370d7f0ecf8 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 8 May 2018 15:24:53 +0900 Subject: [PATCH 6/7] check content view in SetMenuBarVisibility --- atom/browser/ui/cocoa/root_view_mac.mm | 8 +++----- atom/browser/ui/views/root_view.cc | 10 ++++------ 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/atom/browser/ui/cocoa/root_view_mac.mm b/atom/browser/ui/cocoa/root_view_mac.mm index 6f9e81e879..1ecebfe06c 100644 --- a/atom/browser/ui/cocoa/root_view_mac.mm +++ b/atom/browser/ui/cocoa/root_view_mac.mm @@ -4,7 +4,7 @@ #include "atom/browser/ui/cocoa/root_view_mac.h" -#include "atom/browser/native_window_mac.h" +#include "atom/browser/native_window.h" namespace atom { @@ -15,12 +15,10 @@ RootViewMac::RootViewMac(NativeWindow* window) : window_(window) { RootViewMac::~RootViewMac() {} void RootViewMac::Layout() { - views::View* content_view = - static_cast(window_)->content_view(); - if (!content_view) // Not ready yet. + if (!window_->content_view()) // Not ready yet. return; - content_view->SetBoundsRect(gfx::Rect(gfx::Point(), size())); + window_->content_view()->SetBoundsRect(gfx::Rect(gfx::Point(), size())); } gfx::Size RootViewMac::GetMinimumSize() const { diff --git a/atom/browser/ui/views/root_view.cc b/atom/browser/ui/views/root_view.cc index b49dd014b0..716563ff01 100644 --- a/atom/browser/ui/views/root_view.cc +++ b/atom/browser/ui/views/root_view.cc @@ -4,7 +4,7 @@ #include "atom/browser/ui/views/root_view.h" -#include "atom/browser/native_window_views.h" +#include "atom/browser/native_window.h" #include "atom/browser/ui/views/menu_bar.h" #include "content/public/browser/native_web_keyboard_event.h" @@ -86,7 +86,7 @@ bool RootView::IsMenuBarAutoHide() const { } void RootView::SetMenuBarVisibility(bool visible) { - if (!menu_bar_ || menu_bar_visible_ == visible) + if (!window_->content_view() || !menu_bar_ || menu_bar_visible_ == visible) return; // Always show the accelerator when the auto-hide menu bar shows. @@ -151,9 +151,7 @@ void RootView::ResetAltState() { } void RootView::Layout() { - views::View* content_view = - static_cast(window_)->content_view(); - if (!content_view) // Not ready yet. + if (!window_->content_view()) // Not ready yet. return; const auto menu_bar_bounds = @@ -162,7 +160,7 @@ void RootView::Layout() { if (menu_bar_) menu_bar_->SetBoundsRect(menu_bar_bounds); - content_view->SetBoundsRect( + window_->content_view()->SetBoundsRect( gfx::Rect(0, menu_bar_bounds.height(), size().width(), size().height() - menu_bar_bounds.height())); } From 2f3fcb9dbe2aa756717c83a1fa537e5fce307588 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 8 May 2018 16:02:57 +0900 Subject: [PATCH 7/7] give window a default content view Certain APIs are expecting the window to have a content view, having a default one simplifies our design. --- atom/browser/native_window_mac.mm | 3 +++ atom/browser/native_window_views.cc | 3 +++ 2 files changed, 6 insertions(+) diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm index e08f7e5482..0797b41093 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -451,6 +451,9 @@ NativeWindowMac::NativeWindowMac(const mate::Dictionary& options, // by calls to other APIs. SetMaximizable(maximizable); + // Default content view. + SetContentView(new views::View()); + // Make sure the bottom corner is rounded for non-modal windows: // http://crbug.com/396264. But do not enable it on OS X 10.9 for transparent // window, otherwise a semi-transparent frame would show. diff --git a/atom/browser/native_window_views.cc b/atom/browser/native_window_views.cc index 16ec70fcc7..a2c16c49f9 100644 --- a/atom/browser/native_window_views.cc +++ b/atom/browser/native_window_views.cc @@ -276,6 +276,9 @@ NativeWindowViews::NativeWindowViews(const mate::Dictionary& options, #endif } + // Default content view. + SetContentView(new views::View()); + gfx::Size size = bounds.size(); if (has_frame() && options.Get(options::kUseContentSize, &use_content_size_) &&