From dd5b8769befa671c18680aea990b838bc7828063 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Thu, 16 Aug 2018 15:57:40 -0700 Subject: [PATCH] fix: use OOPIF for webview tag (#13869) * fix: use OOIF for webview tag * fix: do not call GetNativeView for webview * fix: OOIPF webview's WebContents is managed by embedder frame * fix: guest view can not be focused * fix: clear zoom controller when guest is destroyed * fix: implement the webview resize event The webview is no longer a browser plugin with the resize event, use ResizeObserver instead. * test: disable failed tests due to OOPIF webview * fix: embedder can be destroyed earlier than guest This happens when embedder is manually destroyed. * fix: don't double attach * fix: recreate iframe when webview is reattached * fix: resize event may happen very early * test: some tests are working after OOPIF webview * chore: remove unused browser plugin webview code * fix: get embedder via closure When the "destroyed" event is emitted, the entry in guestInstances would be cleared. * chore: rename browserPluginNode to internalElement * test: make the visibilityState test more robust * chore: guestinstance can not work with OOPIF webview * fix: element could be detached before got response from browser --- atom/browser/api/atom_api_web_contents.cc | 90 ++--- atom/browser/api/atom_api_web_contents.h | 7 +- atom/browser/api/atom_api_web_view_manager.cc | 1 + atom/browser/common_web_contents_delegate.cc | 6 +- atom/browser/common_web_contents_delegate.h | 3 +- atom/browser/web_contents_zoom_controller.cc | 3 + atom/browser/web_contents_zoom_controller.h | 1 + atom/browser/web_view_guest_delegate.cc | 165 ++------ atom/browser/web_view_guest_delegate.h | 94 +---- atom/renderer/api/atom_api_web_frame.cc | 56 ++- atom/renderer/api/atom_api_web_frame.h | 7 +- atom/renderer/renderer_client_base.cc | 13 +- atom/renderer/renderer_client_base.h | 4 - brightray/browser/browser_main_parts.cc | 4 +- brightray/browser/inspectable_web_contents.cc | 11 +- brightray/browser/inspectable_web_contents.h | 8 +- .../browser/inspectable_web_contents_impl.cc | 26 +- .../browser/inspectable_web_contents_impl.h | 5 +- .../mac/bry_inspectable_web_contents_view.h | 1 + .../mac/bry_inspectable_web_contents_view.mm | 20 +- .../inspectable_web_contents_view_views.cc | 3 +- lib/browser/guest-view-manager.js | 107 ++--- lib/renderer/web-view/guest-view-internal.js | 27 +- lib/renderer/web-view/web-view-attributes.js | 79 ---- lib/renderer/web-view/web-view-constants.js | 7 - lib/renderer/web-view/web-view.js | 160 ++------ spec/chromium-spec.js | 3 +- spec/webview-spec.js | 365 +----------------- 28 files changed, 268 insertions(+), 1008 deletions(-) diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index c19cbebbe6..667aa3f08f 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -120,28 +120,6 @@ struct PrintSettings { namespace mate { -template <> -struct Converter { - static bool FromV8(v8::Isolate* isolate, - v8::Local val, - atom::SetSizeParams* out) { - mate::Dictionary params; - if (!ConvertFromV8(isolate, val, ¶ms)) - return false; - bool autosize; - if (params.Get("enableAutoSize", &autosize)) - out->enable_auto_size.reset(new bool(autosize)); - gfx::Size size; - if (params.Get("min", &size)) - out->min_size.reset(new gfx::Size(size)); - if (params.Get("max", &size)) - out->max_size.reset(new gfx::Size(size)); - if (params.Get("normal", &size)) - out->normal_size.reset(new gfx::Size(size)); - return true; - } -}; - template <> struct Converter { static bool FromV8(v8::Isolate* isolate, @@ -396,7 +374,8 @@ WebContents::WebContents(v8::Isolate* isolate, GURL("chrome-guest://fake-host")); content::WebContents::CreateParams params(session->browser_context(), site_instance); - guest_delegate_.reset(new WebViewGuestDelegate); + guest_delegate_.reset( + new WebViewGuestDelegate(embedder_->web_contents(), this)); params.guest_delegate = guest_delegate_.get(); #if defined(ENABLE_OSR) @@ -448,7 +427,7 @@ void WebContents::InitWithSessionAndOptions(v8::Isolate* isolate, mate::Handle session, const mate::Dictionary& options) { Observe(web_contents); - InitWithWebContents(web_contents, session->browser_context()); + InitWithWebContents(web_contents, session->browser_context(), IsGuest()); managed_web_contents()->GetView()->SetDelegate(this); @@ -481,8 +460,6 @@ void WebContents::InitWithSessionAndOptions(v8::Isolate* isolate, web_contents->SetUserAgentOverride(GetBrowserContext()->GetUserAgent()); if (IsGuest()) { - guest_delegate_->Initialize(this); - NativeWindow* owner_window = nullptr; if (embedder_) { // New WebContents's owner_window is the embedder's owner_window. @@ -504,27 +481,18 @@ WebContents::~WebContents() { if (managed_web_contents()) { managed_web_contents()->GetView()->SetDelegate(nullptr); - // For webview we need to tell content module to do some cleanup work before - // destroying it. - if (type_ == WEB_VIEW) - guest_delegate_->Destroy(); - RenderViewDeleted(web_contents()->GetRenderViewHost()); - if (type_ == WEB_VIEW) { - DestroyWebContents(false /* async */); + if (type_ == BROWSER_WINDOW && owner_window()) { + for (ExtendedWebContentsObserver& observer : observers_) + observer.OnCloseContents(); } else { - if (type_ == BROWSER_WINDOW && owner_window()) { - for (ExtendedWebContentsObserver& observer : observers_) - observer.OnCloseContents(); - } else { - DestroyWebContents(true /* async */); - } - // The WebContentsDestroyed will not be called automatically because we - // destroy the webContents in the next tick. So we have to manually - // call it here to make sure "destroyed" event is emitted. - WebContentsDestroyed(); + DestroyWebContents(!IsGuest() /* async */); } + // The WebContentsDestroyed will not be called automatically because we + // destroy the webContents in the next tick. So we have to manually + // call it here to make sure "destroyed" event is emitted. + WebContentsDestroyed(); } } @@ -784,13 +752,6 @@ content::JavaScriptDialogManager* WebContents::GetJavaScriptDialogManager( return dialog_manager_.get(); } -void WebContents::ResizeDueToAutoResize(content::WebContents* web_contents, - const gfx::Size& new_size) { - if (IsGuest()) { - guest_delegate_->ResizeDueToAutoResize(new_size); - } -} - void WebContents::BeforeUnloadFired(const base::TimeTicks& proceed_time) { // Do nothing, we override this method just to avoid compilation error since // there are two virtual functions named BeforeUnloadFired. @@ -935,6 +896,8 @@ void WebContents::DidFinishNavigation( Emit("did-navigate", url, http_response_code, http_status_text); } } + if (IsGuest()) + Emit("load-commit", url, is_main_frame); } else { auto url = navigation_handle->GetURL(); int code = navigation_handle->GetNetErrorCode(); @@ -1075,7 +1038,8 @@ bool WebContents::OnMessageReceived(const IPC::Message& message, // 1. call webContents.destroy(); // 2. garbage collection; // 3. user closes the window of webContents; -// For webview only #1 will happen, for BrowserWindow both #1 and #3 may +// 4. the embedder detaches the frame. +// For webview only #4 will happen, for BrowserWindow both #1 and #3 may // happen. The #2 should never happen for webContents, because webview is // managed by GuestViewManager, and BrowserWindow's webContents is managed // by api::BrowserWindow. @@ -1083,6 +1047,7 @@ bool WebContents::OnMessageReceived(const IPC::Message& message, // sure "destroyed" event is emitted. For #3, the content::WebContents will // be destroyed on close, and WebContentsDestroyed would be called for it, so // we need to make sure the api::WebContents is also deleted. +// For #4, the WebContents will be destroyed by embedder. void WebContents::WebContentsDestroyed() { // Cleanup relationships with other parts. RemoveFromWeakMap(); @@ -1093,6 +1058,13 @@ void WebContents::WebContentsDestroyed() { Emit("destroyed"); + // For guest view based on OOPIF, the WebContents is released by the embedder + // frame, and we need to clear the reference to the memory. + if (IsGuest() && managed_web_contents()) { + managed_web_contents()->ReleaseWebContents(); + ResetManagedWebContents(false); + } + // Destroy the native class in next tick. base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, GetDestroyClosure()); } @@ -1129,10 +1101,6 @@ void WebContents::LoadURL(const GURL& url, const mate::Dictionary& options) { return; } - if (guest_delegate_ && !guest_delegate_->IsAttached()) { - return; - } - content::NavigationController::LoadURLParams params(url); if (!options.Get("httpReferrer", ¶ms.referrer)) { @@ -1747,15 +1715,20 @@ void WebContents::OnCursorChange(const content::WebCursor& cursor) { } } -void WebContents::SetSize(const SetSizeParams& params) { - if (guest_delegate_) - guest_delegate_->SetSize(params); +void WebContents::SetSize(v8::Local) { + // TODO(zcbenz): Remove this method in 4.0. } bool WebContents::IsGuest() const { return type_ == WEB_VIEW; } +void WebContents::AttachToIframe(content::WebContents* embedder_web_contents, + int embedder_frame_id) { + if (guest_delegate_) + guest_delegate_->AttachToIframe(embedder_web_contents, embedder_frame_id); +} + bool WebContents::IsOffScreen() const { #if defined(ENABLE_OSR) return type_ == OFF_SCREEN; @@ -2045,6 +2018,7 @@ void WebContents::BuildPrototype(v8::Isolate* isolate, .SetMethod("startDrag", &WebContents::StartDrag) .SetMethod("setSize", &WebContents::SetSize) .SetMethod("isGuest", &WebContents::IsGuest) + .SetMethod("attachToIframe", &WebContents::AttachToIframe) .SetMethod("isOffscreen", &WebContents::IsOffScreen) .SetMethod("startPainting", &WebContents::StartPainting) .SetMethod("stopPainting", &WebContents::StopPainting) diff --git a/atom/browser/api/atom_api_web_contents.h b/atom/browser/api/atom_api_web_contents.h index 6b83439650..e907ef9f3e 100644 --- a/atom/browser/api/atom_api_web_contents.h +++ b/atom/browser/api/atom_api_web_contents.h @@ -42,7 +42,6 @@ class ResourceRequestBody; namespace atom { -struct SetSizeParams; class AtomBrowserContext; class AtomJavaScriptDialogManager; class WebContentsZoomController; @@ -197,8 +196,10 @@ class WebContents : public mate::TrackableObject, void CapturePage(mate::Arguments* args); // Methods for creating . - void SetSize(const SetSizeParams& params); + void SetSize(v8::Local); bool IsGuest() const; + void AttachToIframe(content::WebContents* embedder_web_contents, + int embedder_frame_id); // Methods for offscreen rendering bool IsOffScreen() const; @@ -340,8 +341,6 @@ class WebContents : public mate::TrackableObject, const content::BluetoothChooser::EventHandler& handler) override; content::JavaScriptDialogManager* GetJavaScriptDialogManager( content::WebContents* source) override; - void ResizeDueToAutoResize(content::WebContents* web_contents, - const gfx::Size& new_size) override; // content::WebContentsObserver: void BeforeUnloadFired(const base::TimeTicks& proceed_time) override; diff --git a/atom/browser/api/atom_api_web_view_manager.cc b/atom/browser/api/atom_api_web_view_manager.cc index 27fb2cf3fc..e24d6306f6 100644 --- a/atom/browser/api/atom_api_web_view_manager.cc +++ b/atom/browser/api/atom_api_web_view_manager.cc @@ -10,6 +10,7 @@ #include "atom/common/options_switches.h" #include "content/public/browser/browser_context.h" #include "native_mate/dictionary.h" + // Must be the last in the includes list. // See https://github.com/electron/electron/issues/10363 #include "atom/common/node_includes.h" diff --git a/atom/browser/common_web_contents_delegate.cc b/atom/browser/common_web_contents_delegate.cc index d5ab67eef2..b3b1195e04 100644 --- a/atom/browser/common_web_contents_delegate.cc +++ b/atom/browser/common_web_contents_delegate.cc @@ -152,7 +152,8 @@ CommonWebContentsDelegate::~CommonWebContentsDelegate() {} void CommonWebContentsDelegate::InitWithWebContents( content::WebContents* web_contents, - AtomBrowserContext* browser_context) { + AtomBrowserContext* browser_context, + bool is_guest) { browser_context_ = browser_context; web_contents->SetDelegate(this); @@ -165,7 +166,8 @@ void CommonWebContentsDelegate::InitWithWebContents( !web_preferences || web_preferences->IsEnabled(options::kOffscreen); // Create InspectableWebContents. - web_contents_.reset(brightray::InspectableWebContents::Create(web_contents)); + web_contents_.reset( + brightray::InspectableWebContents::Create(web_contents, is_guest)); web_contents_->SetDelegate(this); } diff --git a/atom/browser/common_web_contents_delegate.h b/atom/browser/common_web_contents_delegate.h index 962f573dde..5da5d20975 100644 --- a/atom/browser/common_web_contents_delegate.h +++ b/atom/browser/common_web_contents_delegate.h @@ -42,7 +42,8 @@ class CommonWebContentsDelegate // Creates a InspectableWebContents object and takes onwership of // |web_contents|. void InitWithWebContents(content::WebContents* web_contents, - AtomBrowserContext* browser_context); + AtomBrowserContext* browser_context, + bool is_guest); // Set the window as owner window. void SetOwnerWindow(NativeWindow* owner_window); diff --git a/atom/browser/web_contents_zoom_controller.cc b/atom/browser/web_contents_zoom_controller.cc index 489321810c..35b8e4635c 100644 --- a/atom/browser/web_contents_zoom_controller.cc +++ b/atom/browser/web_contents_zoom_controller.cc @@ -228,6 +228,9 @@ void WebContentsZoomController::DidFinishNavigation( } void WebContentsZoomController::WebContentsDestroyed() { + for (Observer& observer : observers_) + observer.OnZoomControllerWebContentsDestroyed(); + observers_.Clear(); embedder_zoom_controller_ = nullptr; } diff --git a/atom/browser/web_contents_zoom_controller.h b/atom/browser/web_contents_zoom_controller.h index 478326f1b8..bafbbbfb49 100644 --- a/atom/browser/web_contents_zoom_controller.h +++ b/atom/browser/web_contents_zoom_controller.h @@ -24,6 +24,7 @@ class WebContentsZoomController virtual void OnZoomLevelChanged(content::WebContents* web_contents, double level, bool is_temporary) {} + virtual void OnZoomControllerWebContentsDestroyed() {} protected: virtual ~Observer() {} diff --git a/atom/browser/web_view_guest_delegate.cc b/atom/browser/web_view_guest_delegate.cc index ed8e2d87c7..27fbff7e9e 100644 --- a/atom/browser/web_view_guest_delegate.cc +++ b/atom/browser/web_view_guest_delegate.cc @@ -7,144 +7,60 @@ #include "atom/browser/api/atom_api_web_contents.h" #include "atom/common/native_mate_converters/gurl_converter.h" #include "content/browser/web_contents/web_contents_impl.h" -#include "content/public/browser/guest_host.h" #include "content/public/browser/navigation_handle.h" #include "content/public/browser/render_frame_host.h" +#include "content/public/browser/render_process_host.h" #include "content/public/browser/render_view_host.h" #include "content/public/browser/render_widget_host.h" #include "content/public/browser/render_widget_host_view.h" namespace atom { -namespace { +WebViewGuestDelegate::WebViewGuestDelegate(content::WebContents* embedder, + api::WebContents* api_web_contents) + : embedder_web_contents_(embedder), api_web_contents_(api_web_contents) {} -const int kDefaultWidth = 300; -const int kDefaultHeight = 300; - -} // namespace - -SetSizeParams::SetSizeParams() = default; -SetSizeParams::~SetSizeParams() = default; - -WebViewGuestDelegate::WebViewGuestDelegate() {} - -WebViewGuestDelegate::~WebViewGuestDelegate() {} - -void WebViewGuestDelegate::Initialize(api::WebContents* api_web_contents) { - api_web_contents_ = api_web_contents; - Observe(api_web_contents->GetWebContents()); -} - -void WebViewGuestDelegate::Destroy() { - // Give the content module an opportunity to perform some cleanup. - ResetZoomController(); - guest_host_->WillDestroy(); - guest_host_ = nullptr; -} - -void WebViewGuestDelegate::SetSize(const SetSizeParams& params) { - bool enable_auto_size = - params.enable_auto_size ? *params.enable_auto_size : auto_size_enabled_; - gfx::Size min_size = params.min_size ? *params.min_size : min_auto_size_; - gfx::Size max_size = params.max_size ? *params.max_size : max_auto_size_; - - if (params.normal_size) - normal_size_ = *params.normal_size; - - min_auto_size_ = min_size; - min_auto_size_.SetToMin(max_size); - max_auto_size_ = max_size; - max_auto_size_.SetToMax(min_size); - - enable_auto_size &= !min_auto_size_.IsEmpty() && !max_auto_size_.IsEmpty(); - - auto* rvh = web_contents()->GetRenderViewHost(); - if (enable_auto_size) { - // Autosize is being enabled. - rvh->EnableAutoResize(min_auto_size_, max_auto_size_); - normal_size_.SetSize(0, 0); - } else { - // Autosize is being disabled. - // Use default width/height if missing from partially defined normal size. - if (normal_size_.width() && !normal_size_.height()) - normal_size_.set_height(GetDefaultSize().height()); - if (!normal_size_.width() && normal_size_.height()) - normal_size_.set_width(GetDefaultSize().width()); - - gfx::Size new_size; - if (!normal_size_.IsEmpty()) { - new_size = normal_size_; - } else if (!guest_size_.IsEmpty()) { - new_size = guest_size_; - } else { - new_size = GetDefaultSize(); - } - - bool changed_due_to_auto_resize = false; - if (auto_size_enabled_) { - // Autosize was previously enabled. - rvh->DisableAutoResize(new_size); - changed_due_to_auto_resize = true; - } else { - // Autosize was already disabled. - guest_host_->SizeContents(new_size); - } - - UpdateGuestSize(new_size, changed_due_to_auto_resize); - } - - auto_size_enabled_ = enable_auto_size; -} - -void WebViewGuestDelegate::ResizeDueToAutoResize(const gfx::Size& new_size) { - UpdateGuestSize(new_size, auto_size_enabled_); -} - -void WebViewGuestDelegate::DidFinishNavigation( - content::NavigationHandle* navigation_handle) { - if (navigation_handle->HasCommitted() && !navigation_handle->IsErrorPage()) { - auto is_main_frame = navigation_handle->IsInMainFrame(); - auto url = navigation_handle->GetURL(); - api_web_contents_->Emit("load-commit", url, is_main_frame); - } -} - -void WebViewGuestDelegate::DidDetach() { - attached_ = false; +WebViewGuestDelegate::~WebViewGuestDelegate() { ResetZoomController(); } -void WebViewGuestDelegate::DidAttach(int guest_proxy_routing_id) { - attached_ = true; - api_web_contents_->Emit("did-attach"); +void WebViewGuestDelegate::AttachToIframe( + content::WebContents* embedder_web_contents, + int embedder_frame_id) { + embedder_web_contents_ = embedder_web_contents; + + int embedder_process_id = + embedder_web_contents_->GetMainFrame()->GetProcess()->GetID(); + auto* embedder_frame = + content::RenderFrameHost::FromID(embedder_process_id, embedder_frame_id); + DCHECK_EQ(embedder_web_contents_, + content::WebContents::FromRenderFrameHost(embedder_frame)); + + // Attach this inner WebContents |guest_web_contents| to the outer + // WebContents |embedder_web_contents|. The outer WebContents's + // frame |embedder_frame| hosts the inner WebContents. + api_web_contents_->web_contents()->AttachToOuterWebContentsFrame( + embedder_web_contents_, embedder_frame); ResetZoomController(); embedder_zoom_controller_ = WebContentsZoomController::FromWebContents(embedder_web_contents_); - auto* zoom_controller = api_web_contents_->GetZoomController(); embedder_zoom_controller_->AddObserver(this); + auto* zoom_controller = api_web_contents_->GetZoomController(); zoom_controller->SetEmbedderZoomController(embedder_zoom_controller_); + + api_web_contents_->Emit("did-attach"); +} + +void WebViewGuestDelegate::DidDetach() { + ResetZoomController(); } content::WebContents* WebViewGuestDelegate::GetOwnerWebContents() const { return embedder_web_contents_; } -void WebViewGuestDelegate::SetGuestHost(content::GuestHost* guest_host) { - guest_host_ = guest_host; -} - -void WebViewGuestDelegate::WillAttach( - content::WebContents* embedder_web_contents, - int element_instance_id, - bool is_full_page_plugin, - const base::Closure& completion_callback) { - embedder_web_contents_ = embedder_web_contents; - is_full_page_plugin_ = is_full_page_plugin; - completion_callback.Run(); -} - void WebViewGuestDelegate::OnZoomLevelChanged( content::WebContents* web_contents, double level, @@ -161,23 +77,8 @@ void WebViewGuestDelegate::OnZoomLevelChanged( } } -void WebViewGuestDelegate::UpdateGuestSize(const gfx::Size& new_size, - bool due_to_auto_resize) { - if (due_to_auto_resize) - api_web_contents_->Emit("size-changed", guest_size_.width(), - guest_size_.height(), new_size.width(), - new_size.height()); - guest_size_ = new_size; -} - -gfx::Size WebViewGuestDelegate::GetDefaultSize() const { - if (is_full_page_plugin_) { - // Full page plugins default to the size of the owner's viewport. - return embedder_web_contents_->GetRenderWidgetHostView() - ->GetVisibleViewportSize(); - } else { - return gfx::Size(kDefaultWidth, kDefaultHeight); - } +void WebViewGuestDelegate::OnZoomControllerWebContentsDestroyed() { + ResetZoomController(); } void WebViewGuestDelegate::ResetZoomController() { @@ -187,10 +88,6 @@ void WebViewGuestDelegate::ResetZoomController() { } } -bool WebViewGuestDelegate::CanBeEmbeddedInsideCrossProcessFrames() { - return true; -} - content::RenderWidgetHost* WebViewGuestDelegate::GetOwnerRenderWidgetHost() { return embedder_web_contents_->GetRenderViewHost()->GetWidget(); } diff --git a/atom/browser/web_view_guest_delegate.h b/atom/browser/web_view_guest_delegate.h index 784ca99275..6d9f440588 100644 --- a/atom/browser/web_view_guest_delegate.h +++ b/atom/browser/web_view_guest_delegate.h @@ -7,7 +7,6 @@ #include "atom/browser/web_contents_zoom_controller.h" #include "content/public/browser/browser_plugin_guest_delegate.h" -#include "content/public/browser/web_contents_observer.h" namespace atom { @@ -15,80 +14,33 @@ namespace api { class WebContents; } -// A struct of parameters for SetSize(). The parameters are all declared as -// scoped pointers since they are all optional. Null pointers indicate that the -// parameter has not been provided, and the last used value should be used. Note -// that when |enable_auto_size| is true, providing |normal_size| is not -// meaningful. This is because the normal size of the guestview is overridden -// whenever autosizing occurs. -struct SetSizeParams { - SetSizeParams(); - ~SetSizeParams(); - - std::unique_ptr enable_auto_size; - std::unique_ptr min_size; - std::unique_ptr max_size; - std::unique_ptr normal_size; -}; - class WebViewGuestDelegate : public content::BrowserPluginGuestDelegate, - public content::WebContentsObserver, public WebContentsZoomController::Observer { public: - WebViewGuestDelegate(); + WebViewGuestDelegate(content::WebContents* embedder, + api::WebContents* api_web_contents); ~WebViewGuestDelegate() override; - void Initialize(api::WebContents* api_web_contents); - - // Called when the WebContents is going to be destroyed. - void Destroy(); - - // Used to toggle autosize mode for this GuestView, and set both the automatic - // and normal sizes. - void SetSize(const SetSizeParams& params); - - // Invoked when the contents auto-resized and the container should match it. - void ResizeDueToAutoResize(const gfx::Size& new_size); - - // Return true if attached. - bool IsAttached() const { return attached_; } + // Attach to the iframe. + void AttachToIframe(content::WebContents* embedder_web_contents, + int embedder_frame_id); protected: - // content::WebContentsObserver: - void DidFinishNavigation( - content::NavigationHandle* navigation_handle) override; - // content::BrowserPluginGuestDelegate: - void DidAttach(int guest_proxy_routing_id) final; void DidDetach() final; content::WebContents* GetOwnerWebContents() const final; - void SetGuestHost(content::GuestHost* guest_host) final; - void WillAttach(content::WebContents* embedder_web_contents, - int element_instance_id, - bool is_full_page_plugin, - const base::Closure& completion_callback) final; - bool CanBeEmbeddedInsideCrossProcessFrames() override; - content::RenderWidgetHost* GetOwnerRenderWidgetHost() override; - content::SiteInstance* GetOwnerSiteInstance() override; + content::RenderWidgetHost* GetOwnerRenderWidgetHost() final; + content::SiteInstance* GetOwnerSiteInstance() final; content::WebContents* CreateNewGuestWindow( - const content::WebContents::CreateParams& create_params) override; + const content::WebContents::CreateParams& create_params) final; // WebContentsZoomController::Observer: void OnZoomLevelChanged(content::WebContents* web_contents, double level, bool is_temporary) override; + void OnZoomControllerWebContentsDestroyed() override; private: - // This method is invoked when the contents auto-resized to give the container - // an opportunity to match it if it wishes. - // - // This gives the derived class an opportunity to inform its container element - // or perform other actions. - void UpdateGuestSize(const gfx::Size& new_size, bool due_to_auto_resize); - - // Returns the default size of the guestview. - gfx::Size GetDefaultSize() const; - void ResetZoomController(); // The WebContents that attaches this guest view. @@ -98,34 +50,6 @@ class WebViewGuestDelegate : public content::BrowserPluginGuestDelegate, // to subscribe for zoom changes. WebContentsZoomController* embedder_zoom_controller_ = nullptr; - // The size of the container element. - gfx::Size element_size_; - - // The size of the guest content. Note: In autosize mode, the container - // element may not match the size of the guest. - gfx::Size guest_size_; - - // A pointer to the guest_host. - content::GuestHost* guest_host_ = nullptr; - - // Indicates whether autosize mode is enabled or not. - bool auto_size_enabled_ = false; - - // The maximum size constraints of the container element in autosize mode. - gfx::Size max_auto_size_; - - // The minimum size constraints of the container element in autosize mode. - gfx::Size min_auto_size_; - - // The size that will be used when autosize mode is disabled. - gfx::Size normal_size_; - - // Whether the guest view is inside a plugin document. - bool is_full_page_plugin_ = false; - - // Whether attached. - bool attached_ = false; - api::WebContents* api_web_contents_ = nullptr; DISALLOW_COPY_AND_ASSIGN(WebViewGuestDelegate); diff --git a/atom/renderer/api/atom_api_web_frame.cc b/atom/renderer/api/atom_api_web_frame.cc index 68bb084a2f..f0f7ef249d 100644 --- a/atom/renderer/api/atom_api_web_frame.cc +++ b/atom/renderer/api/atom_api_web_frame.cc @@ -13,6 +13,7 @@ #include "atom/renderer/api/atom_api_spell_check_client.h" #include "base/memory/memory_pressure_listener.h" #include "content/public/renderer/render_frame.h" +#include "content/public/renderer/render_frame_observer.h" #include "content/public/renderer/render_frame_visitor.h" #include "content/public/renderer/render_view.h" #include "native_mate/dictionary.h" @@ -62,6 +63,29 @@ namespace api { namespace { +content::RenderFrame* GetRenderFrame(v8::Local value) { + v8::Local context = + v8::Local::Cast(value)->CreationContext(); + if (context.IsEmpty()) + return nullptr; + blink::WebLocalFrame* frame = blink::WebLocalFrame::FrameForContext(context); + if (!frame) + return nullptr; + return content::RenderFrame::FromWebFrame(frame); +} + +class RenderFrameStatus : public content::RenderFrameObserver { + public: + explicit RenderFrameStatus(content::RenderFrame* render_frame) + : content::RenderFrameObserver(render_frame) {} + ~RenderFrameStatus() final {} + + bool is_ok() { return render_frame() != nullptr; } + + // RenderFrameObserver implementation. + void OnDestruct() final {} +}; + class ScriptExecutionCallback : public blink::WebScriptExecutionCallback { public: using CompletionCallback = @@ -170,20 +194,23 @@ v8::Local WebFrame::RegisterEmbedderCustomElement( blink::WebString::FromUTF16(name), options); } -void WebFrame::RegisterElementResizeCallback( - int element_instance_id, - const GuestViewContainer::ResizeCallback& callback) { - auto* guest_view_container = GuestViewContainer::FromID(element_instance_id); - if (guest_view_container) - guest_view_container->RegisterElementResizeCallback(callback); -} +int WebFrame::GetWebFrameId(v8::Local content_window) { + // Get the WebLocalFrame before (possibly) executing any user-space JS while + // getting the |params|. We track the status of the RenderFrame via an + // observer in case it is deleted during user code execution. + content::RenderFrame* render_frame = GetRenderFrame(content_window); + RenderFrameStatus render_frame_status(render_frame); -void WebFrame::AttachGuest(int id) { - content::RenderFrame::FromWebFrame(web_frame_)->AttachGuest(id); -} + if (!render_frame_status.is_ok()) + return -1; -void WebFrame::DetachGuest(int id) { - content::RenderFrame::FromWebFrame(web_frame_)->DetachGuest(id); + blink::WebLocalFrame* frame = render_frame->GetWebFrame(); + // Parent must exist. + blink::WebFrame* parent_frame = frame->Parent(); + DCHECK(parent_frame); + DCHECK(parent_frame->IsWebLocalFrame()); + + return render_frame->GetRoutingID(); } void WebFrame::SetSpellCheckProvider(mate::Arguments* args, @@ -464,10 +491,7 @@ void WebFrame::BuildPrototype(v8::Isolate* isolate, &WebFrame::SetLayoutZoomLevelLimits) .SetMethod("registerEmbedderCustomElement", &WebFrame::RegisterEmbedderCustomElement) - .SetMethod("registerElementResizeCallback", - &WebFrame::RegisterElementResizeCallback) - .SetMethod("attachGuest", &WebFrame::AttachGuest) - .SetMethod("detachGuest", &WebFrame::DetachGuest) + .SetMethod("getWebFrameId", &WebFrame::GetWebFrameId) .SetMethod("setSpellCheckProvider", &WebFrame::SetSpellCheckProvider) .SetMethod("registerURLSchemeAsBypassingCSP", &WebFrame::RegisterURLSchemeAsBypassingCSP) diff --git a/atom/renderer/api/atom_api_web_frame.h b/atom/renderer/api/atom_api_web_frame.h index 3c28b19ff7..4cc6386abc 100644 --- a/atom/renderer/api/atom_api_web_frame.h +++ b/atom/renderer/api/atom_api_web_frame.h @@ -9,7 +9,6 @@ #include #include -#include "atom/renderer/guest_view_container.h" #include "native_mate/handle.h" #include "native_mate/wrappable.h" #include "third_party/WebKit/public/platform/WebCache.h" @@ -54,11 +53,7 @@ class WebFrame : public mate::Wrappable { v8::Local RegisterEmbedderCustomElement( const base::string16& name, v8::Local options); - void RegisterElementResizeCallback( - int element_instance_id, - const GuestViewContainer::ResizeCallback& callback); - void AttachGuest(int element_instance_id); - void DetachGuest(int element_instance_id); + int GetWebFrameId(v8::Local content_window); // Set the provider that will be used by SpellCheckClient for spell check. void SetSpellCheckProvider(mate::Arguments* args, diff --git a/atom/renderer/renderer_client_base.cc b/atom/renderer/renderer_client_base.cc index 46aa2c8d43..69490420ef 100644 --- a/atom/renderer/renderer_client_base.cc +++ b/atom/renderer/renderer_client_base.cc @@ -14,7 +14,6 @@ #include "atom/renderer/atom_render_frame_observer.h" #include "atom/renderer/atom_render_view_observer.h" #include "atom/renderer/content_settings_observer.h" -#include "atom/renderer/guest_view_container.h" #include "atom/renderer/preferences_manager.h" #include "base/command_line.h" #include "base/process/process_handle.h" @@ -24,6 +23,7 @@ #include "chrome/renderer/printing/print_web_view_helper.h" #include "chrome/renderer/tts_dispatcher.h" #include "content/public/common/content_constants.h" +#include "content/public/renderer/render_frame.h" #include "content/public/renderer/render_view.h" #include "native_mate/dictionary.h" #include "third_party/WebKit/Source/platform/weborigin/SchemeRegistry.h" @@ -224,17 +224,6 @@ bool RendererClientBase::OverrideCreatePlugin( return true; } -content::BrowserPluginDelegate* RendererClientBase::CreateBrowserPluginDelegate( - content::RenderFrame* render_frame, - const std::string& mime_type, - const GURL& original_url) { - if (mime_type == content::kBrowserPluginMimeType) { - return new GuestViewContainer(render_frame); - } else { - return nullptr; - } -} - void RendererClientBase::AddSupportedKeySystems( std::vector>* key_systems) { AddChromeKeySystems(key_systems); diff --git a/atom/renderer/renderer_client_base.h b/atom/renderer/renderer_client_base.h index 2b13f8b038..f04be0a0fb 100644 --- a/atom/renderer/renderer_client_base.h +++ b/atom/renderer/renderer_client_base.h @@ -46,10 +46,6 @@ class RendererClientBase : public content::ContentRendererClient { bool OverrideCreatePlugin(content::RenderFrame* render_frame, const blink::WebPluginParams& params, blink::WebPlugin** plugin) override; - content::BrowserPluginDelegate* CreateBrowserPluginDelegate( - content::RenderFrame* render_frame, - const std::string& mime_type, - const GURL& original_url) override; void AddSupportedKeySystems( std::vector>* key_systems) override; diff --git a/brightray/browser/browser_main_parts.cc b/brightray/browser/browser_main_parts.cc index 8fc4c1adc9..1e52a869b4 100644 --- a/brightray/browser/browser_main_parts.cc +++ b/brightray/browser/browser_main_parts.cc @@ -186,10 +186,8 @@ void BrowserMainParts::InitializeFeatureList() { auto disable_features = cmd_line->GetSwitchValueASCII(switches::kDisableFeatures); - // TODO(deepak1556): Disable guest webcontents based on OOPIF feature. // Disable surface synchronization and async wheel events to make OSR work. - disable_features += - ",GuestViewCrossProcessFrames,SurfaceSynchronization,AsyncWheelEvents"; + disable_features += ",SurfaceSynchronization,AsyncWheelEvents"; auto feature_list = std::make_unique(); feature_list->InitializeFromCommandLine(enable_features, disable_features); diff --git a/brightray/browser/inspectable_web_contents.cc b/brightray/browser/inspectable_web_contents.cc index f389f75db6..ab6d87d6c6 100644 --- a/brightray/browser/inspectable_web_contents.cc +++ b/brightray/browser/inspectable_web_contents.cc @@ -5,14 +5,9 @@ namespace brightray { InspectableWebContents* InspectableWebContents::Create( - const content::WebContents::CreateParams& create_params) { - auto* contents = content::WebContents::Create(create_params); - return Create(contents); -} - -InspectableWebContents* InspectableWebContents::Create( - content::WebContents* web_contents) { - return new InspectableWebContentsImpl(web_contents); + content::WebContents* web_contents, + bool is_guest) { + return new InspectableWebContentsImpl(web_contents, is_guest); } } // namespace brightray diff --git a/brightray/browser/inspectable_web_contents.h b/brightray/browser/inspectable_web_contents.h index 7b0a010b9e..4abaa03c5f 100644 --- a/brightray/browser/inspectable_web_contents.h +++ b/brightray/browser/inspectable_web_contents.h @@ -20,12 +20,10 @@ class InspectableWebContentsView; class InspectableWebContents { public: - static InspectableWebContents* Create( - const content::WebContents::CreateParams&); - // The returned InspectableWebContents takes ownership of the passed-in // WebContents. - static InspectableWebContents* Create(content::WebContents*); + static InspectableWebContents* Create(content::WebContents* web_contents, + bool is_guest); virtual ~InspectableWebContents() {} @@ -37,6 +35,8 @@ class InspectableWebContents { virtual void SetDelegate(InspectableWebContentsDelegate* delegate) = 0; virtual InspectableWebContentsDelegate* GetDelegate() const = 0; + virtual bool IsGuest() const = 0; + virtual void ReleaseWebContents() = 0; virtual void SetDevToolsWebContents(content::WebContents* devtools) = 0; virtual void SetDockState(const std::string& state) = 0; virtual void ShowDevTools() = 0; diff --git a/brightray/browser/inspectable_web_contents_impl.cc b/brightray/browser/inspectable_web_contents_impl.cc index 6784180a7f..32b5743b8d 100644 --- a/brightray/browser/inspectable_web_contents_impl.cc +++ b/brightray/browser/inspectable_web_contents_impl.cc @@ -202,15 +202,20 @@ void InspectableWebContentsImpl::RegisterPrefs(PrefRegistrySimple* registry) { } InspectableWebContentsImpl::InspectableWebContentsImpl( - content::WebContents* web_contents) + content::WebContents* web_contents, + bool is_guest) : frontend_loaded_(false), can_dock_(true), delegate_(nullptr), + pref_service_( + static_cast(web_contents->GetBrowserContext()) + ->prefs()), web_contents_(web_contents), + is_guest_(is_guest), + view_(CreateInspectableContentsView(this)), weak_factory_(this) { - auto* context = - static_cast(web_contents_->GetBrowserContext()); - pref_service_ = context->prefs(); + if (is_guest) + return; auto* bounds_dict = pref_service_->GetDictionary(kDevToolsBoundsPref); if (bounds_dict) { DictionaryToRect(*bounds_dict, &devtools_bounds_); @@ -235,8 +240,6 @@ InspectableWebContentsImpl::InspectableWebContentsImpl( display.y() + (display.height() - devtools_bounds_.height()) / 2); } } - - view_.reset(CreateInspectableContentsView(this)); } InspectableWebContentsImpl::~InspectableWebContentsImpl() { @@ -282,6 +285,14 @@ InspectableWebContentsDelegate* InspectableWebContentsImpl::GetDelegate() return delegate_; } +bool InspectableWebContentsImpl::IsGuest() const { + return is_guest_; +} + +void InspectableWebContentsImpl::ReleaseWebContents() { + web_contents_.release(); +} + void InspectableWebContentsImpl::SetDockState(const std::string& state) { if (state == "detach") { can_dock_ = false; @@ -332,7 +343,8 @@ void InspectableWebContentsImpl::CloseDevTools() { managed_devtools_web_contents_.reset(); } embedder_message_dispatcher_.reset(); - web_contents_->Focus(); + if (!IsGuest()) + web_contents_->Focus(); } } diff --git a/brightray/browser/inspectable_web_contents_impl.h b/brightray/browser/inspectable_web_contents_impl.h index c75f46fed4..3f0fe0093c 100644 --- a/brightray/browser/inspectable_web_contents_impl.h +++ b/brightray/browser/inspectable_web_contents_impl.h @@ -39,7 +39,7 @@ class InspectableWebContentsImpl public: static void RegisterPrefs(PrefRegistrySimple* pref_registry); - explicit InspectableWebContentsImpl(content::WebContents*); + InspectableWebContentsImpl(content::WebContents* web_contents, bool is_guest); ~InspectableWebContentsImpl() override; InspectableWebContentsView* GetView() const override; @@ -48,6 +48,8 @@ class InspectableWebContentsImpl void SetDelegate(InspectableWebContentsDelegate* delegate) override; InspectableWebContentsDelegate* GetDelegate() const override; + bool IsGuest() const override; + void ReleaseWebContents() override; void SetDevToolsWebContents(content::WebContents* devtools) override; void SetDockState(const std::string& state) override; void ShowDevTools() override; @@ -214,6 +216,7 @@ class InspectableWebContentsImpl // The external devtools assigned by SetDevToolsWebContents. content::WebContents* external_devtools_web_contents_ = nullptr; + bool is_guest_; std::unique_ptr view_; using ExtensionsAPIs = std::map; diff --git a/brightray/browser/mac/bry_inspectable_web_contents_view.h b/brightray/browser/mac/bry_inspectable_web_contents_view.h index 2a99ff0e9f..e0693507fe 100644 --- a/brightray/browser/mac/bry_inspectable_web_contents_view.h +++ b/brightray/browser/mac/bry_inspectable_web_contents_view.h @@ -14,6 +14,7 @@ using brightray::InspectableWebContentsViewMac; @private brightray::InspectableWebContentsViewMac* inspectableWebContentsView_; + base::scoped_nsobject fake_view_; base::scoped_nsobject devtools_window_; BOOL devtools_visible_; BOOL devtools_docked_; diff --git a/brightray/browser/mac/bry_inspectable_web_contents_view.mm b/brightray/browser/mac/bry_inspectable_web_contents_view.mm index b72a489ba7..1a9139e7af 100644 --- a/brightray/browser/mac/bry_inspectable_web_contents_view.mm +++ b/brightray/browser/mac/bry_inspectable_web_contents_view.mm @@ -32,11 +32,17 @@ name:NSWindowDidBecomeMainNotification object:nil]; - auto* contents = - inspectableWebContentsView_->inspectable_web_contents()->GetWebContents(); - auto contentsView = contents->GetNativeView(); - [contentsView setAutoresizingMask:NSViewWidthSizable | NSViewHeightSizable]; - [self addSubview:contentsView]; + if (inspectableWebContentsView_->inspectable_web_contents()->IsGuest()) { + fake_view_.reset([[NSView alloc] init]); + [fake_view_ setAutoresizingMask:NSViewWidthSizable | NSViewHeightSizable]; + [self addSubview:fake_view_]; + } else { + auto* contents = inspectableWebContentsView_->inspectable_web_contents() + ->GetWebContents(); + auto contentsView = contents->GetNativeView(); + [contentsView setAutoresizingMask:NSViewWidthSizable | NSViewHeightSizable]; + [self addSubview:contentsView]; + } // See https://code.google.com/p/chromium/issues/detail?id=348490. [self setWantsLayer:YES]; @@ -75,7 +81,7 @@ if (visible && devtools_docked_) { webContents->SetAllowOtherViews(true); devToolsWebContents->SetAllowOtherViews(true); - } else { + } else if (!inspectable_web_contents->IsGuest()) { webContents->SetAllowOtherViews(false); } @@ -194,7 +200,7 @@ - (void)viewDidBecomeFirstResponder:(NSNotification*)notification { auto* inspectable_web_contents = inspectableWebContentsView_->inspectable_web_contents(); - if (!inspectable_web_contents) + if (!inspectable_web_contents || inspectable_web_contents->IsGuest()) return; auto* webContents = inspectable_web_contents->GetWebContents(); auto webContentsView = webContents->GetNativeView(); diff --git a/brightray/browser/views/inspectable_web_contents_view_views.cc b/brightray/browser/views/inspectable_web_contents_view_views.cc index e95ffd05ee..7cca54f031 100644 --- a/brightray/browser/views/inspectable_web_contents_view_views.cc +++ b/brightray/browser/views/inspectable_web_contents_view_views.cc @@ -81,7 +81,8 @@ InspectableWebContentsViewViews::InspectableWebContentsViewViews( title_(base::ASCIIToUTF16("Developer Tools")) { set_owned_by_client(); - if (inspectable_web_contents_->GetWebContents()->GetNativeView()) { + if (!inspectable_web_contents_->IsGuest() && + inspectable_web_contents_->GetWebContents()->GetNativeView()) { views::WebView* contents_web_view = new views::WebView(nullptr); contents_web_view->SetWebContents( inspectable_web_contents_->GetWebContents()); diff --git a/lib/browser/guest-view-manager.js b/lib/browser/guest-view-manager.js index 3e13e6ec3f..1e4029e39a 100644 --- a/lib/browser/guest-view-manager.js +++ b/lib/browser/guest-view-manager.js @@ -46,11 +46,6 @@ let nextGuestInstanceId = 0 const guestInstances = {} const embedderElementsMap = {} -// Moves the last element of array to the first one. -const moveLastToFirst = function (list) { - list.unshift(list.pop()) -} - // Generate guestInstanceId. const getNextGuestInstanceId = function () { return ++nextGuestInstanceId @@ -73,10 +68,15 @@ const createGuest = function (embedder, params) { embedder: embedder } - watchEmbedder(embedder) + // Clear the guest from map when it is destroyed. + guest.once('destroyed', () => { + if (guestInstanceId in guestInstances) { + detachGuest(embedder, guestInstanceId) + } + }) // Init guest web view after attached. - guest.on('did-attach', function (event) { + guest.once('did-attach', function (event) { params = this.attachParams delete this.attachParams @@ -88,21 +88,6 @@ const createGuest = function (embedder, params) { return } - this.setSize({ - normal: { - width: params.elementWidth, - height: params.elementHeight - }, - enableAutoSize: params.autosize, - min: { - width: params.minwidth, - height: params.minheight - }, - max: { - width: params.maxwidth, - height: params.maxheight - } - }) if (params.src) { const opts = {} if (params.httpreferrer) { @@ -118,8 +103,7 @@ const createGuest = function (embedder, params) { }) const sendToEmbedder = (channel, ...args) => { - const embedder = getEmbedder(guestInstanceId) - if (embedder != null) { + if (!embedder.isDestroyed()) { embedder.send(`${channel}-${guest.viewInstanceId}`, ...args) } } @@ -139,11 +123,6 @@ const createGuest = function (embedder, params) { sendToEmbedder('ELECTRON_GUEST_VIEW_INTERNAL_IPC_MESSAGE', channel, ...args) }) - // Autosize. - guest.on('size-changed', function (_, ...args) { - sendToEmbedder('ELECTRON_GUEST_VIEW_INTERNAL_SIZE_CHANGED', ...args) - }) - // Notify guest of embedder window visibility when it is ready // FIXME Remove once https://github.com/electron/electron/issues/6828 is fixed guest.on('dom-ready', function () { @@ -176,7 +155,7 @@ const createGuest = function (embedder, params) { } // Attach the guest to an element of embedder. -const attachGuest = function (event, elementInstanceId, guestInstanceId, params) { +const attachGuest = function (event, embedderFrameId, elementInstanceId, guestInstanceId, params) { const embedder = event.sender // Destroy the old guest when attaching. const key = `${embedder.id}-${elementInstanceId}` @@ -187,7 +166,10 @@ const attachGuest = function (event, elementInstanceId, guestInstanceId, params) return } - destroyGuest(embedder, oldGuestInstanceId) + const oldGuestInstance = guestInstances[oldGuestInstanceId] + if (oldGuestInstance) { + oldGuestInstance.guest.destroy() + } } const guestInstance = guestInstances[guestInstanceId] @@ -260,11 +242,10 @@ const attachGuest = function (event, elementInstanceId, guestInstanceId, params) embedder.emit('will-attach-webview', event, webPreferences, params) if (event.defaultPrevented) { if (guest.viewInstanceId == null) guest.viewInstanceId = params.instanceId - destroyGuest(embedder, guestInstanceId) + guest.destroy() return } - webViewManager.addGuest(guestInstanceId, elementInstanceId, embedder, guest, webPreferences) guest.attachParams = params embedderElementsMap[key] = guestInstanceId @@ -273,21 +254,19 @@ const attachGuest = function (event, elementInstanceId, guestInstanceId, params) guestInstance.elementInstanceId = elementInstanceId watchEmbedder(embedder) + + webViewManager.addGuest(guestInstanceId, elementInstanceId, embedder, guest, webPreferences) + guest.attachToIframe(embedder, embedderFrameId) } -// Destroy an existing guest instance. -const destroyGuest = function (embedder, guestInstanceId) { - if (!(guestInstanceId in guestInstances)) { - return - } - +// Remove an guest-embedder relationship. +const detachGuest = function (embedder, guestInstanceId) { const guestInstance = guestInstances[guestInstanceId] if (embedder !== guestInstance.embedder) { return } webViewManager.removeGuest(embedder, guestInstanceId) - guestInstance.guest.destroy() delete guestInstances[guestInstanceId] const key = `${embedder.id}-${guestInstance.elementInstanceId}` @@ -305,7 +284,7 @@ const watchEmbedder = function (embedder) { // Forward embedder window visiblity change events to guest const onVisibilityChange = function (visibilityState) { - for (const guestInstanceId of Object.keys(guestInstances)) { + for (const guestInstanceId in guestInstances) { const guestInstance = guestInstances[guestInstanceId] guestInstance.visibilityState = visibilityState if (guestInstance.embedder === embedder) { @@ -315,33 +294,20 @@ const watchEmbedder = function (embedder) { } embedder.on('-window-visibility-change', onVisibilityChange) - const destroyEvents = ['will-destroy', 'crashed', 'did-navigate'] - const destroy = function () { - for (const guestInstanceId of Object.keys(guestInstances)) { - if (guestInstances[guestInstanceId].embedder === embedder) { - destroyGuest(embedder, parseInt(guestInstanceId)) + embedder.once('will-destroy', () => { + // Usually the guestInstances is cleared when guest is destroyed, but it + // may happen that the embedder gets manually destroyed earlier than guest, + // and the embedder will be invalid in the usual code path. + for (const guestInstanceId in guestInstances) { + const guestInstance = guestInstances[guestInstanceId] + if (guestInstance.embedder === embedder) { + detachGuest(embedder, parseInt(guestInstanceId)) } } - - for (const event of destroyEvents) { - embedder.removeListener(event, destroy) - } + // Clear the listeners. embedder.removeListener('-window-visibility-change', onVisibilityChange) - watchedEmbedders.delete(embedder) - } - - for (const event of destroyEvents) { - embedder.once(event, destroy) - - // Users might also listen to the crashed event, so we must ensure the guest - // is destroyed before users' listener gets called. It is done by moving our - // listener to the first one in queue. - const listeners = embedder._events[event] - if (Array.isArray(listeners)) { - moveLastToFirst(listeners) - } - } + }) } ipcMain.on('ELECTRON_GUEST_VIEW_MANAGER_CREATE_GUEST', function (event, params, requestId) { @@ -352,17 +318,8 @@ ipcMain.on('ELECTRON_GUEST_VIEW_MANAGER_CREATE_GUEST_SYNC', function (event, par event.returnValue = createGuest(event.sender, params) }) -ipcMain.on('ELECTRON_GUEST_VIEW_MANAGER_ATTACH_GUEST', function (event, elementInstanceId, guestInstanceId, params) { - attachGuest(event, elementInstanceId, guestInstanceId, params) -}) - -ipcMain.on('ELECTRON_GUEST_VIEW_MANAGER_DESTROY_GUEST', function (event, guestInstanceId) { - destroyGuest(event.sender, guestInstanceId) -}) - -ipcMain.on('ELECTRON_GUEST_VIEW_MANAGER_SET_SIZE', function (event, guestInstanceId, params) { - const guest = getGuest(guestInstanceId) - if (guest != null) guest.setSize(params) +ipcMain.on('ELECTRON_GUEST_VIEW_MANAGER_ATTACH_GUEST', function (event, embedderFrameId, elementInstanceId, guestInstanceId, params) { + attachGuest(event, embedderFrameId, elementInstanceId, guestInstanceId, params) }) // Returns WebContents from its guest id. diff --git a/lib/renderer/web-view/guest-view-internal.js b/lib/renderer/web-view/guest-view-internal.js index b9b7a8ea39..6e42c769b7 100644 --- a/lib/renderer/web-view/guest-view-internal.js +++ b/lib/renderer/web-view/guest-view-internal.js @@ -61,7 +61,6 @@ const dispatchEvent = function (webView, eventName, eventKey, ...args) { module.exports = { registerEvents: function (webView, viewInstanceId) { ipcRenderer.on(`ELECTRON_GUEST_VIEW_INTERNAL_DESTROY_GUEST-${viewInstanceId}`, function () { - webFrame.detachGuest(webView.internalInstanceId) webView.guestInstanceId = undefined webView.reset() const domEvent = new Event('destroyed') @@ -78,22 +77,11 @@ module.exports = { domEvent.args = args webView.dispatchEvent(domEvent) }) - - ipcRenderer.on(`ELECTRON_GUEST_VIEW_INTERNAL_SIZE_CHANGED-${viewInstanceId}`, function (event, ...args) { - const domEvent = new Event('size-changed') - const props = ['oldWidth', 'oldHeight', 'newWidth', 'newHeight'] - for (let i = 0; i < props.length; i++) { - const prop = props[i] - domEvent[prop] = args[i] - } - webView.onSizeChanged(domEvent) - }) }, deregisterEvents: function (viewInstanceId) { ipcRenderer.removeAllListeners(`ELECTRON_GUEST_VIEW_INTERNAL_DESTROY_GUEST-${viewInstanceId}`) ipcRenderer.removeAllListeners(`ELECTRON_GUEST_VIEW_INTERNAL_DISPATCH_EVENT-${viewInstanceId}`) ipcRenderer.removeAllListeners(`ELECTRON_GUEST_VIEW_INTERNAL_IPC_MESSAGE-${viewInstanceId}`) - ipcRenderer.removeAllListeners(`ELECTRON_GUEST_VIEW_INTERNAL_SIZE_CHANGED-${viewInstanceId}`) }, createGuest: function (params, callback) { requestId++ @@ -103,14 +91,11 @@ module.exports = { createGuestSync: function (params) { return ipcRenderer.sendSync('ELECTRON_GUEST_VIEW_MANAGER_CREATE_GUEST_SYNC', params) }, - attachGuest: function (elementInstanceId, guestInstanceId, params) { - ipcRenderer.send('ELECTRON_GUEST_VIEW_MANAGER_ATTACH_GUEST', elementInstanceId, guestInstanceId, params) - webFrame.attachGuest(elementInstanceId) - }, - destroyGuest: function (guestInstanceId) { - ipcRenderer.send('ELECTRON_GUEST_VIEW_MANAGER_DESTROY_GUEST', guestInstanceId) - }, - setSize: function (guestInstanceId, params) { - ipcRenderer.send('ELECTRON_GUEST_VIEW_MANAGER_SET_SIZE', guestInstanceId, params) + attachGuest: function (elementInstanceId, guestInstanceId, params, contentWindow) { + const embedderFrameId = webFrame.getWebFrameId(contentWindow) + if (embedderFrameId < 0) { // this error should not happen. + throw new Error('Invalid embedder frame') + } + ipcRenderer.send('ELECTRON_GUEST_VIEW_MANAGER_ATTACH_GUEST', embedderFrameId, elementInstanceId, guestInstanceId, params) } } diff --git a/lib/renderer/web-view/web-view-attributes.js b/lib/renderer/web-view/web-view-attributes.js index 204046bd60..ad9970bd0b 100644 --- a/lib/renderer/web-view/web-view-attributes.js +++ b/lib/renderer/web-view/web-view-attributes.js @@ -1,7 +1,6 @@ 'use strict' const WebViewImpl = require('./web-view') -const guestViewInternal = require('./guest-view-internal') const webViewConstants = require('./web-view-constants') const {remote} = require('electron') @@ -74,39 +73,6 @@ class BooleanAttribute extends WebViewAttribute { } } -// Attribute used to define the demension limits of autosizing. -class AutosizeDimensionAttribute extends WebViewAttribute { - getValue () { - return parseInt(this.webViewImpl.webviewNode.getAttribute(this.name)) || 0 - } - - handleMutation () { - if (!this.webViewImpl.guestInstanceId) { - return - } - guestViewInternal.setSize(this.webViewImpl.guestInstanceId, { - enableAutoSize: this.webViewImpl.attributes[webViewConstants.ATTRIBUTE_AUTOSIZE].getValue(), - min: { - width: parseInt(this.webViewImpl.attributes[webViewConstants.ATTRIBUTE_MINWIDTH].getValue() || 0), - height: parseInt(this.webViewImpl.attributes[webViewConstants.ATTRIBUTE_MINHEIGHT].getValue() || 0) - }, - max: { - width: parseInt(this.webViewImpl.attributes[webViewConstants.ATTRIBUTE_MAXWIDTH].getValue() || 0), - height: parseInt(this.webViewImpl.attributes[webViewConstants.ATTRIBUTE_MAXHEIGHT].getValue() || 0) - } - }) - } -} - -// Attribute that specifies whether the webview should be autosized. -class AutosizeAttribute extends BooleanAttribute { - constructor (webViewImpl) { - super(webViewConstants.ATTRIBUTE_AUTOSIZE, webViewImpl) - } -} - -AutosizeAttribute.prototype.handleMutation = AutosizeDimensionAttribute.prototype.handleMutation - // Attribute representing the state of the storage partition. class PartitionAttribute extends WebViewAttribute { constructor (webViewImpl) { @@ -130,43 +96,6 @@ class PartitionAttribute extends WebViewAttribute { } } -// An attribute that controls the guest instance this webview is connected to -class GuestInstanceAttribute extends WebViewAttribute { - constructor (webViewImpl) { - super(webViewConstants.ATTRIBUTE_GUESTINSTANCE, webViewImpl) - } - - // Retrieves and returns the attribute's value. - getValue () { - if (this.webViewImpl.webviewNode.hasAttribute(this.name)) { - return parseInt(this.webViewImpl.webviewNode.getAttribute(this.name)) - } - } - - // Sets the attribute's value. - setValue (value) { - if (!value) { - this.webViewImpl.webviewNode.removeAttribute(this.name) - } else if (!isNaN(value)) { - this.webViewImpl.webviewNode.setAttribute(this.name, value) - } - } - - handleMutation (oldValue, newValue) { - if (!newValue) { - this.webViewImpl.reset() - return - } - - const intVal = parseInt(newValue) - if (!isNaN(newValue) && remote.getGuestWebContents(intVal)) { - this.webViewImpl.attachGuestInstance(intVal) - } else { - this.setValueIgnoreMutation(oldValue) - } - } -} - // Attribute that handles the location and navigation of the webview. class SrcAttribute extends WebViewAttribute { constructor (webViewImpl) { @@ -314,7 +243,6 @@ class WebPreferencesAttribute extends WebViewAttribute { // Sets up all of the webview attributes. WebViewImpl.prototype.setupWebViewAttributes = function () { this.attributes = {} - this.attributes[webViewConstants.ATTRIBUTE_AUTOSIZE] = new AutosizeAttribute(this) this.attributes[webViewConstants.ATTRIBUTE_PARTITION] = new PartitionAttribute(this) this.attributes[webViewConstants.ATTRIBUTE_SRC] = new SrcAttribute(this) this.attributes[webViewConstants.ATTRIBUTE_HTTPREFERRER] = new HttpReferrerAttribute(this) @@ -326,12 +254,5 @@ WebViewImpl.prototype.setupWebViewAttributes = function () { this.attributes[webViewConstants.ATTRIBUTE_PRELOAD] = new PreloadAttribute(this) this.attributes[webViewConstants.ATTRIBUTE_BLINKFEATURES] = new BlinkFeaturesAttribute(this) this.attributes[webViewConstants.ATTRIBUTE_DISABLEBLINKFEATURES] = new DisableBlinkFeaturesAttribute(this) - this.attributes[webViewConstants.ATTRIBUTE_GUESTINSTANCE] = new GuestInstanceAttribute(this) - this.attributes[webViewConstants.ATTRIBUTE_DISABLEGUESTRESIZE] = new BooleanAttribute(webViewConstants.ATTRIBUTE_DISABLEGUESTRESIZE, this) this.attributes[webViewConstants.ATTRIBUTE_WEBPREFERENCES] = new WebPreferencesAttribute(this) - - const autosizeAttributes = [webViewConstants.ATTRIBUTE_MAXHEIGHT, webViewConstants.ATTRIBUTE_MAXWIDTH, webViewConstants.ATTRIBUTE_MINHEIGHT, webViewConstants.ATTRIBUTE_MINWIDTH] - autosizeAttributes.forEach((attribute) => { - this.attributes[attribute] = new AutosizeDimensionAttribute(attribute, this) - }) } diff --git a/lib/renderer/web-view/web-view-constants.js b/lib/renderer/web-view/web-view-constants.js index bf2601822d..327114be50 100644 --- a/lib/renderer/web-view/web-view-constants.js +++ b/lib/renderer/web-view/web-view-constants.js @@ -1,10 +1,5 @@ module.exports = { // Attributes. - ATTRIBUTE_AUTOSIZE: 'autosize', - ATTRIBUTE_MAXHEIGHT: 'maxheight', - ATTRIBUTE_MAXWIDTH: 'maxwidth', - ATTRIBUTE_MINHEIGHT: 'minheight', - ATTRIBUTE_MINWIDTH: 'minwidth', ATTRIBUTE_NAME: 'name', ATTRIBUTE_PARTITION: 'partition', ATTRIBUTE_SRC: 'src', @@ -17,8 +12,6 @@ module.exports = { ATTRIBUTE_USERAGENT: 'useragent', ATTRIBUTE_BLINKFEATURES: 'blinkfeatures', ATTRIBUTE_DISABLEBLINKFEATURES: 'disableblinkfeatures', - ATTRIBUTE_GUESTINSTANCE: 'guestinstance', - ATTRIBUTE_DISABLEGUESTRESIZE: 'disableguestresize', ATTRIBUTE_WEBPREFERENCES: 'webpreferences', // Internal attribute. diff --git a/lib/renderer/web-view/web-view.js b/lib/renderer/web-view/web-view.js index be9700921b..1152f3b959 100644 --- a/lib/renderer/web-view/web-view.js +++ b/lib/renderer/web-view/web-view.js @@ -6,8 +6,6 @@ const v8Util = process.atomBinding('v8_util') const guestViewInternal = require('./guest-view-internal') const webViewConstants = require('./web-view-constants') -const hasProp = {}.hasOwnProperty - // An unique ID that can represent current context. const contextId = v8Util.getHiddenValue(global, 'contextId') @@ -23,27 +21,27 @@ class WebViewImpl { constructor (webviewNode) { this.webviewNode = webviewNode v8Util.setHiddenValue(this.webviewNode, 'internal', this) - this.attached = false this.elementAttached = false this.beforeFirstNavigation = true // on* Event handlers. this.on = {} - this.browserPluginNode = this.createBrowserPluginNode() + this.internalElement = this.createInternalElement() const shadowRoot = this.webviewNode.attachShadow({mode: 'open'}) shadowRoot.innerHTML = '' this.setupWebViewAttributes() this.setupFocusPropagation() this.viewInstanceId = getNextId() - shadowRoot.appendChild(this.browserPluginNode) + shadowRoot.appendChild(this.internalElement) } - createBrowserPluginNode () { - // We create BrowserPlugin as a custom element in order to observe changes - // to attributes synchronously. - const browserPluginNode = new WebViewImpl.BrowserPlugin() - v8Util.setHiddenValue(browserPluginNode, 'internal', this) - return browserPluginNode + createInternalElement () { + const iframeElement = document.createElement('iframe') + iframeElement.style.width = '100%' + iframeElement.style.height = '100%' + iframeElement.style.border = '0' + v8Util.setHiddenValue(iframeElement, 'internal', this) + return iframeElement } // Resets some state upon reattaching element to the DOM. @@ -55,7 +53,6 @@ class WebViewImpl { // heard back from createGuest yet. We will not reset the flag in this case so // that we don't end up allocating a second guest. if (this.guestInstanceId) { - guestViewInternal.destroyGuest(this.guestInstanceId) this.guestInstanceId = void 0 } @@ -63,9 +60,12 @@ class WebViewImpl { this.beforeFirstNavigation = true this.attributes[webViewConstants.ATTRIBUTE_PARTITION].validPartitionId = true - // Set guestinstance last since this can trigger the attachedCallback to fire - // when moving the webview using element.replaceChild - this.attributes[webViewConstants.ATTRIBUTE_GUESTINSTANCE].setValueIgnoreMutation(undefined) + // Since attachment swaps a local frame for a remote frame, we need our + // internal iframe element to be local again before we can reattach. + const newFrame = this.createInternalElement() + const oldFrame = this.internalElement + this.internalElement = newFrame + oldFrame.parentNode.replaceChild(newFrame, oldFrame) } // Sets the .request property. @@ -87,12 +87,12 @@ class WebViewImpl { // Focus the BrowserPlugin when the takes focus. this.webviewNode.addEventListener('focus', () => { - this.browserPluginNode.focus() + this.internalElement.focus() }) // Blur the BrowserPlugin when the loses focus. this.webviewNode.addEventListener('blur', () => { - this.browserPluginNode.blur() + this.internalElement.blur() }) } @@ -110,62 +110,11 @@ class WebViewImpl { this.attributes[attributeName].handleMutation(oldValue, newValue) } - handleBrowserPluginAttributeMutation (attributeName, oldValue, newValue) { - if (attributeName === webViewConstants.ATTRIBUTE_INTERNALINSTANCEID && !oldValue && !!newValue) { - this.browserPluginNode.removeAttribute(webViewConstants.ATTRIBUTE_INTERNALINSTANCEID) - this.internalInstanceId = parseInt(newValue) - - // Track when the element resizes using the element resize callback. - webFrame.registerElementResizeCallback(this.internalInstanceId, this.onElementResize.bind(this)) - if (this.guestInstanceId) { - guestViewInternal.attachGuest(this.internalInstanceId, this.guestInstanceId, this.buildParams()) - } - } - } - - onSizeChanged (webViewEvent) { - const {newHeight, newWidth} = webViewEvent - const node = this.webviewNode - const width = node.offsetWidth - - // Check the current bounds to make sure we do not resize - // outside of current constraints. - const maxWidth = this.attributes[webViewConstants.ATTRIBUTE_MAXWIDTH].getValue() | width - const maxHeight = this.attributes[webViewConstants.ATTRIBUTE_MAXHEIGHT].getValue() | width - let minWidth = this.attributes[webViewConstants.ATTRIBUTE_MINWIDTH].getValue() | width - let minHeight = this.attributes[webViewConstants.ATTRIBUTE_MINHEIGHT].getValue() | width - minWidth = Math.min(minWidth, maxWidth) - minHeight = Math.min(minHeight, maxHeight) - if (!this.attributes[webViewConstants.ATTRIBUTE_AUTOSIZE].getValue() || (newWidth >= minWidth && newWidth <= maxWidth && newHeight >= minHeight && newHeight <= maxHeight)) { - node.style.width = `${newWidth}px` - node.style.height = `${newHeight}px` - - // Only fire the DOM event if the size of the has actually - // changed. - this.dispatchEvent(webViewEvent) - } - } - - onElementResize (newSize) { - // Dispatch the 'resize' event. - const resizeEvent = new Event('resize', { - bubbles: true - }) - - // Using client size values, because when a webview is transformed `newSize` - // is incorrect - newSize.width = this.webviewNode.clientWidth - newSize.height = this.webviewNode.clientHeight - - resizeEvent.newWidth = newSize.width - resizeEvent.newHeight = newSize.height + onElementResize () { + const resizeEvent = new Event('resize', { bubbles: true }) + resizeEvent.newWidth = this.webviewNode.clientWidth + resizeEvent.newHeight = this.webviewNode.clientHeight this.dispatchEvent(resizeEvent) - if (this.guestInstanceId && - !this.attributes[webViewConstants.ATTRIBUTE_DISABLEGUESTRESIZE].getValue()) { - guestViewInternal.setSize(this.guestInstanceId, { - normal: newSize - }) - } } createGuest () { @@ -175,6 +124,7 @@ class WebViewImpl { } createGuestSync () { + this.beforeFirstNavigation = false this.attachGuestInstance(guestViewInternal.createGuestSync(this.buildParams())) } @@ -225,64 +175,30 @@ class WebViewImpl { userAgentOverride: this.userAgentOverride } for (const attributeName in this.attributes) { - if (hasProp.call(this.attributes, attributeName)) { + if (this.attributes.hasOwnProperty(attributeName)) { params[attributeName] = this.attributes[attributeName].getValue() } } - - // When the WebView is not participating in layout (display:none) - // then getBoundingClientRect() would report a width and height of 0. - // However, in the case where the WebView has a fixed size we can - // use that value to initially size the guest so as to avoid a relayout of - // the on display:block. - const css = window.getComputedStyle(this.webviewNode, null) - const elementRect = this.webviewNode.getBoundingClientRect() - params.elementWidth = parseInt(elementRect.width) || parseInt(css.getPropertyValue('width')) - params.elementHeight = parseInt(elementRect.height) || parseInt(css.getPropertyValue('height')) return params } attachGuestInstance (guestInstanceId) { + if (!this.elementAttached) { + // The element could be detached before we got response from browser. + return + } + this.internalInstanceId = getNextId() this.guestInstanceId = guestInstanceId - this.attributes[webViewConstants.ATTRIBUTE_GUESTINSTANCE].setValueIgnoreMutation(guestInstanceId) this.webContents = remote.getGuestWebContents(this.guestInstanceId) - if (!this.internalInstanceId) { - return true - } - return guestViewInternal.attachGuest(this.internalInstanceId, this.guestInstanceId, this.buildParams()) + guestViewInternal.attachGuest(this.internalInstanceId, this.guestInstanceId, this.buildParams(), this.internalElement.contentWindow) + // ResizeObserver is a browser global not recognized by "standard". + /* globals ResizeObserver */ + // TODO(zcbenz): Should we deprecate the "resize" event? Wait, it is not + // even documented. + this.resizeObserver = new ResizeObserver(this.onElementResize.bind(this)).observe(this.internalElement) } } -// Registers browser plugin custom element. -const registerBrowserPluginElement = function () { - const proto = Object.create(HTMLObjectElement.prototype) - proto.createdCallback = function () { - this.setAttribute('type', 'application/browser-plugin') - this.setAttribute('id', `browser-plugin-${getNextId()}`) - - // The node fills in the container. - this.style.flex = '1 1 auto' - } - proto.attributeChangedCallback = function (name, oldValue, newValue) { - const internal = v8Util.getHiddenValue(this, 'internal') - if (internal) { - internal.handleBrowserPluginAttributeMutation(name, oldValue, newValue) - } - } - proto.attachedCallback = function () { - // Load the plugin immediately. - return this.nonExistentAttribute - } - WebViewImpl.BrowserPlugin = webFrame.registerEmbedderCustomElement('browserplugin', { - 'extends': 'object', - prototype: proto - }) - delete proto.createdCallback - delete proto.attachedCallback - delete proto.detachedCallback - delete proto.attributeChangedCallback -} - // Registers custom element. const registerWebViewElement = function () { const proto = Object.create(HTMLObjectElement.prototype) @@ -313,12 +229,7 @@ const registerWebViewElement = function () { if (!internal.elementAttached) { guestViewInternal.registerEvents(internal, internal.viewInstanceId) internal.elementAttached = true - const instance = internal.attributes[webViewConstants.ATTRIBUTE_GUESTINSTANCE].getValue() - if (instance) { - internal.attachGuestInstance(instance) - } else { - internal.attributes[webViewConstants.ATTRIBUTE_SRC].parse() - } + internal.attributes[webViewConstants.ATTRIBUTE_SRC].parse() } } @@ -450,7 +361,6 @@ const listener = function (event) { if (document.readyState === 'loading') { return } - registerBrowserPluginElement() registerWebViewElement() window.removeEventListener(event.type, listener, useCapture) } diff --git a/spec/chromium-spec.js b/spec/chromium-spec.js index 98975eccc2..d90269769d 100644 --- a/spec/chromium-spec.js +++ b/spec/chromium-spec.js @@ -299,8 +299,7 @@ describe('chromium feature', () => { b = window.open(windowUrl, '', 'nodeIntegration=no,show=no') }) - // TODO(codebytere): re-enable this test - xit('disables webviewTag when node integration is disabled on the parent window', (done) => { + it('disables webviewTag when node integration is disabled on the parent window', (done) => { let b listener = (event) => { assert.equal(event.data.isWebViewUndefined, true) diff --git a/spec/webview-spec.js b/spec/webview-spec.js index 7d24d163c5..db75a4e375 100644 --- a/spec/webview-spec.js +++ b/spec/webview-spec.js @@ -5,7 +5,7 @@ const path = require('path') const http = require('http') const url = require('url') const {ipcRenderer, remote} = require('electron') -const {app, session, getGuestWebContents, ipcMain, BrowserWindow, webContents} = remote +const {app, session, ipcMain, BrowserWindow} = remote const {closeWindow} = require('./window-helpers') const {emittedOnce, waitForEvent} = require('./events-helpers') @@ -64,8 +64,7 @@ describe(' tag', function () { return closeTheWindow() }) - // TODO(codebytere): re-enable this test - xit('works without script tag in page', async () => { + it('works without script tag in page', async () => { const w = await openTheWindow({show: false}) w.loadURL('file://' + fixtures + '/pages/webview-no-script.html') await emittedOnce(ipcMain, 'pong') @@ -693,7 +692,8 @@ describe(' tag', function () { }) }) - describe('setDevToolsWebContents() API', () => { + // FIXME(zcbenz): Disabled because of moving to OOPIF webview. + xdescribe('setDevToolsWebContents() API', () => { it('sets webContents of webview as devtools', async () => { const webview2 = new WebView() loadWebView(webview2) @@ -990,9 +990,7 @@ describe(' tag', function () { }) }) - // TODO(jkleinsc): this test causes the test suite to hang on Windows release - // builds. Temporarily disabling so that release build tests will finish. - xdescribe('found-in-page event', () => { + describe('found-in-page event', () => { it('emits when a request is made', (done) => { let requestId = null let activeMatchOrdinal = [] @@ -1149,12 +1147,15 @@ describe(' tag', function () { it('updates when the window is shown after the ready-to-show event', async () => { const w = await openTheWindow({ show: false }) + const readyToShowSignal = emittedOnce(w, 'ready-to-show') + const pongSignal1 = emittedOnce(ipcMain, 'pong') w.loadURL(`file://${fixtures}/pages/webview-visibilitychange.html`) - - await emittedOnce(w, 'ready-to-show') + await pongSignal1 + const pongSignal2 = emittedOnce(ipcMain, 'pong') + await readyToShowSignal w.show() - const [, visibilityState, hidden] = await emittedOnce(ipcMain, 'pong') + const [, visibilityState, hidden] = await pongSignal2 assert(!hidden) assert.equal(visibilityState, 'visible') }) @@ -1255,232 +1256,6 @@ describe(' tag', function () { expect(tabId).to.be.not.equal(w.webContents.id) }) - // TODO(alexeykuzmin): Some tests rashe a renderer process. - // Fix them and enable the tests. - xdescribe('guestinstance attribute', () => { - it('before loading there is no attribute', () => { - loadWebView(webview) // Don't wait for loading to finish. - assert(!webview.hasAttribute('guestinstance')) - }) - - it('loading a page sets the guest view', async () => { - await loadWebView(webview, { - src: `file://${fixtures}/api/blank.html` - }) - - const instance = webview.getAttribute('guestinstance') - assert.equal(instance, parseInt(instance)) - - const guest = getGuestWebContents(parseInt(instance)) - assert.equal(guest, webview.getWebContents()) - }) - - it('deleting the attribute destroys the webview', async () => { - await loadWebView(webview, { - src: `file://${fixtures}/api/blank.html` - }) - - const instance = parseInt(webview.getAttribute('guestinstance')) - const waitForDestroy = waitForEvent(webview, 'destroyed') - webview.removeAttribute('guestinstance') - - await waitForDestroy - expect(getGuestWebContents(instance)).to.equal(undefined) - }) - - it('setting the attribute on a new webview moves the contents', (done) => { - const loadListener = () => { - const webContents = webview.getWebContents() - const instance = webview.getAttribute('guestinstance') - - const destroyListener = () => { - assert.equal(webContents, webview2.getWebContents()) - - // Make sure that events are hooked up to the right webview now - webview2.addEventListener('console-message', (e) => { - assert.equal(e.message, 'a') - document.body.removeChild(webview2) - done() - }) - - webview2.src = `file://${fixtures}/pages/a.html` - } - webview.addEventListener('destroyed', destroyListener, {once: true}) - - const webview2 = new WebView() - loadWebView(webview2, { - guestinstance: instance - }) - } - webview.addEventListener('did-finish-load', loadListener, {once: true}) - loadWebView(webview, { - src: `file://${fixtures}/api/blank.html` - }) - }) - - it('setting the attribute to an invalid guestinstance does nothing', async () => { - await loadWebView(webview, { - src: `file://${fixtures}/api/blank.html` - }) - webview.setAttribute('guestinstance', 55) - - // Make sure that events are still hooked up to the webview - const waitForMessage = waitForEvent(webview, 'console-message') - webview.src = `file://${fixtures}/pages/a.html` - const {message} = await waitForMessage - assert.equal(message, 'a') - }) - - it('setting the attribute on an existing webview moves the contents', (done) => { - const load1Listener = () => { - const webContents = webview.getWebContents() - const instance = webview.getAttribute('guestinstance') - let destroyedInstance - - const destroyListener = () => { - assert.equal(webContents, webview2.getWebContents()) - assert.equal(null, getGuestWebContents(parseInt(destroyedInstance))) - - // Make sure that events are hooked up to the right webview now - webview2.addEventListener('console-message', (e) => { - assert.equal(e.message, 'a') - document.body.removeChild(webview2) - done() - }) - - webview2.src = 'file://' + fixtures + '/pages/a.html' - } - webview.addEventListener('destroyed', destroyListener, {once: true}) - - const webview2 = new WebView() - const load2Listener = () => { - destroyedInstance = webview2.getAttribute('guestinstance') - assert.notEqual(instance, destroyedInstance) - - webview2.setAttribute('guestinstance', instance) - } - webview2.addEventListener('did-finish-load', load2Listener, {once: true}) - loadWebView(webview2, { - src: `file://${fixtures}/api/blank.html` - }) - } - - webview.addEventListener('did-finish-load', load1Listener, {once: true}) - loadWebView(webview, { - src: `file://${fixtures}/api/blank.html` - }) - }) - - it('moving a guest back to its original webview should work', (done) => { - loadWebView(webview, { - src: `file://${fixtures}/api/blank.html` - }).then(() => { - const webContents = webview.getWebContents() - const instance = webview.getAttribute('guestinstance') - - const destroy1Listener = () => { - assert.equal(webContents, webview2.getWebContents()) - assert.notStrictEqual(webContents, webview.getWebContents()) - - const destroy2Listener = () => { - assert.equal(webContents, webview.getWebContents()) - assert.notStrictEqual(webContents, webview2.getWebContents()) - - // Make sure that events are hooked up to the right webview now - webview.addEventListener('console-message', (e) => { - document.body.removeChild(webview2) - assert.equal(e.message, 'a') - done() - }) - - webview.src = `file://${fixtures}/pages/a.html` - } - webview2.addEventListener('destroyed', destroy2Listener, {once: true}) - webview.setAttribute('guestinstance', instance) - } - webview.addEventListener('destroyed', destroy1Listener, {once: true}) - - const webview2 = new WebView() - loadWebView(webview2, {guestinstance: instance}) - }) - }) - - // FIXME(alexeykuzmin): This test only passes if the previous test ^ - // is run alongside. - it('setting the attribute on a webview in a different window moves the contents', (done) => { - loadWebView(webview, { - src: `file://${fixtures}/api/blank.html` - }).then(() => { - const instance = webview.getAttribute('guestinstance') - - w = new BrowserWindow({ show: false }) - w.webContents.once('did-finish-load', () => { - ipcMain.once('pong', () => { - assert(!webview.hasAttribute('guestinstance')) - done() - }) - - w.webContents.send('guestinstance', instance) - }) - w.loadURL(`file://${fixtures}/pages/webview-move-to-window.html`) - }) - }) - - it('does not delete the guestinstance attribute when moving the webview to another parent node', (done) => { - webview.addEventListener('dom-ready', function domReadyListener () { - webview.addEventListener('did-attach', () => { - assert(webview.guestinstance != null) - assert(webview.getWebContents() != null) - done() - }) - - document.body.replaceChild(webview, div) - }) - webview.src = `file://${fixtures}/pages/a.html` - - const div = document.createElement('div') - div.appendChild(webview) - document.body.appendChild(div) - }) - - it('does not destroy the webContents when hiding/showing the webview (regression)', (done) => { - webview.addEventListener('dom-ready', function () { - const instance = webview.getAttribute('guestinstance') - assert(instance != null) - - // Wait for event directly since attach happens asynchronously over IPC - ipcMain.once('ELECTRON_GUEST_VIEW_MANAGER_ATTACH_GUEST', () => { - assert(webview.getWebContents() != null) - assert.equal(instance, webview.getAttribute('guestinstance')) - done() - }) - - webview.style.display = 'none' - webview.offsetHeight // eslint-disable-line - webview.style.display = 'block' - }, {once: true}) - loadWebView(webview, {src: `file://${fixtures}/pages/a.html`}) - }) - - it('does not reload the webContents when hiding/showing the webview (regression)', (done) => { - webview.addEventListener('dom-ready', function () { - webview.addEventListener('did-start-loading', () => { - done(new Error('webview started loading unexpectedly')) - }) - - // Wait for event directly since attach happens asynchronously over IPC - webview.addEventListener('did-attach', () => { - done() - }) - - webview.style.display = 'none' - webview.offsetHeight // eslint-disable-line - webview.style.display = 'block' - }, {once: true}) - loadWebView(webview, {src: `file://${fixtures}/pages/a.html`}) - }) - }) - describe('DOM events', () => { let div @@ -1498,136 +1273,34 @@ describe(' tag', function () { }) it('emits resize events', async () => { + const firstResizeSignal = waitForEvent(webview, 'resize') + const domReadySignal = waitForEvent(webview, 'dom-ready') + webview.src = `file://${fixtures}/pages/a.html` div.appendChild(webview) document.body.appendChild(div) - const firstResizeEvent = await waitForEvent(webview, 'resize') + const firstResizeEvent = await firstResizeSignal expect(firstResizeEvent.target).to.equal(webview) expect(firstResizeEvent.newWidth).to.equal(100) expect(firstResizeEvent.newHeight).to.equal(10) - await waitForEvent(webview, 'dom-ready') + await domReadySignal + + const secondResizeSignal = waitForEvent(webview, 'resize') const newWidth = 1234 const newHeight = 789 div.style.width = `${newWidth}px` div.style.height = `${newHeight}px` - const secondResizeEvent = await waitForEvent(webview, 'resize') + const secondResizeEvent = await secondResizeSignal expect(secondResizeEvent.target).to.equal(webview) expect(secondResizeEvent.newWidth).to.equal(newWidth) expect(secondResizeEvent.newHeight).to.equal(newHeight) }) }) - describe('disableguestresize attribute', () => { - it('does not have attribute by default', () => { - loadWebView(webview) - assert(!webview.hasAttribute('disableguestresize')) - }) - - it('resizes guest when attribute is not present', async () => { - const INITIAL_SIZE = 200 - const w = await openTheWindow( - {show: false, width: INITIAL_SIZE, height: INITIAL_SIZE}) - w.loadURL(`file://${fixtures}/pages/webview-guest-resize.html`) - await emittedOnce(ipcMain, 'webview-loaded') - - const elementResize = emittedOnce(ipcMain, 'webview-element-resize') - .then(([, width, height]) => { - assert.equal(width, CONTENT_SIZE) - assert.equal(height, CONTENT_SIZE) - }) - - const guestResize = emittedOnce(ipcMain, 'webview-guest-resize') - .then(([, width, height]) => { - assert.equal(width, CONTENT_SIZE) - assert.equal(height, CONTENT_SIZE) - }) - - const CONTENT_SIZE = 300 - assert(CONTENT_SIZE !== INITIAL_SIZE) - w.setContentSize(CONTENT_SIZE, CONTENT_SIZE) - - return Promise.all([elementResize, guestResize]) - }) - - // TODO(alexeykuzmin): [Ch66] Enable the test. - xit('does not resize guest when attribute is present', async () => { - const INITIAL_SIZE = 200 - const w = await openTheWindow( - {show: false, width: INITIAL_SIZE, height: INITIAL_SIZE}) - w.loadURL(`file://${fixtures}/pages/webview-no-guest-resize.html`) - await emittedOnce(ipcMain, 'webview-loaded') - - const noGuestResizePromise = Promise.race([ - emittedOnce(ipcMain, 'webview-guest-resize'), - new Promise(resolve => setTimeout(() => resolve(), 500)) - ]).then((eventData = null) => { - if (eventData !== null) { - // Means we got the 'webview-guest-resize' event before the time out. - return Promise.reject(new Error('Unexpected guest resize message')) - } - }) - - const CONTENT_SIZE = 300 - assert(CONTENT_SIZE !== INITIAL_SIZE) - w.setContentSize(CONTENT_SIZE, CONTENT_SIZE) - - return noGuestResizePromise - }) - - it('dispatches element resize event even when attribute is present', async () => { - const INITIAL_SIZE = 200 - const w = await openTheWindow( - {show: false, width: INITIAL_SIZE, height: INITIAL_SIZE}) - w.loadURL(`file://${fixtures}/pages/webview-no-guest-resize.html`) - await emittedOnce(ipcMain, 'webview-loaded') - - const whenElementResized = emittedOnce(ipcMain, 'webview-element-resize') - const CONTENT_SIZE = 300 - assert(CONTENT_SIZE !== INITIAL_SIZE) - w.setContentSize(CONTENT_SIZE, CONTENT_SIZE) - const [, width, height] = await whenElementResized - - expect(width).to.equal(CONTENT_SIZE) - expect(height).to.equal(CONTENT_SIZE) - }) - - // TODO(alexeykuzmin): [Ch66] Enable the test. - xit('can be manually resized with setSize even when attribute is present', async () => { - const INITIAL_SIZE = 200 - const w = await openTheWindow( - {show: false, width: INITIAL_SIZE, height: INITIAL_SIZE}) - w.loadURL(`file://${fixtures}/pages/webview-no-guest-resize.html`) - await emittedOnce(ipcMain, 'webview-loaded') - - const GUEST_WIDTH = 10 - const GUEST_HEIGHT = 20 - - const guestResizePromise = emittedOnce(ipcMain, 'webview-guest-resize') - .then(([, width, height]) => { - expect(width).to.be.equal(GUEST_WIDTH) - expect(height).to.be.equal(GUEST_HEIGHT) - }) - - const wc = webContents.getAllWebContents().find((wc) => { - return wc.hostWebContents && - wc.hostWebContents.id === w.webContents.id - }) - assert(wc) - wc.setSize({ - normal: { - width: GUEST_WIDTH, - height: GUEST_HEIGHT - } - }) - - return guestResizePromise - }) - }) - describe('zoom behavior', () => { const zoomScheme = remote.getGlobal('zoomScheme') const webviewSession = session.fromPartition('webview-temp')