From 4b491c3a779dc658884f61061be6c6af08501b18 Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Mon, 17 Aug 2015 15:48:57 -0400 Subject: [PATCH] Fix assertion when loading the PDF extension with begin-frame-scheduling and OSR (issue #1686) --- libcef/browser/browser_host_impl.cc | 5 +- .../extensions/browser_extensions_util.cc | 53 +++++++---- .../extensions/browser_extensions_util.h | 15 +++- libcef/browser/render_widget_host_view_osr.cc | 87 +++++++++++-------- libcef/browser/render_widget_host_view_osr.h | 8 +- libcef/browser/web_contents_view_osr.cc | 10 ++- 6 files changed, 119 insertions(+), 59 deletions(-) diff --git a/libcef/browser/browser_host_impl.cc b/libcef/browser/browser_host_impl.cc index 0e6427bfc..1e51d2964 100644 --- a/libcef/browser/browser_host_impl.cc +++ b/libcef/browser/browser_host_impl.cc @@ -371,6 +371,9 @@ class UploadFolderHelper : DISALLOW_COPY_AND_ASSIGN(UploadFolderHelper); }; +// Returns the primary OSR host view for the specified |web_contents|. If a +// full-screen host view currently exists then it will be returned. Otherwise, +// the main host view will be returned. CefRenderWidgetHostViewOSR* GetOSRHostView(content::WebContents* web_contents) { CefRenderWidgetHostViewOSR* fs_view = static_cast( @@ -2375,7 +2378,7 @@ bool CefBrowserHostImpl::HandleContextMenu( content::WebContents* CefBrowserHostImpl::GetActionableWebContents() { if (web_contents() && extensions::ExtensionsEnabled()) { content::WebContents* guest_contents = - extensions::GetGuestForOwnerContents(web_contents()); + extensions::GetFullPageGuestForOwnerContents(web_contents()); if (guest_contents) return guest_contents; } diff --git a/libcef/browser/extensions/browser_extensions_util.cc b/libcef/browser/extensions/browser_extensions_util.cc index 583162143..7c57465fb 100644 --- a/libcef/browser/extensions/browser_extensions_util.cc +++ b/libcef/browser/extensions/browser_extensions_util.cc @@ -7,28 +7,51 @@ #include "content/browser/browser_plugin/browser_plugin_embedder.h" #include "content/browser/browser_plugin/browser_plugin_guest.h" #include "content/browser/web_contents/web_contents_impl.h" +#include "content/public/browser/browser_context.h" +#include "content/public/browser/browser_plugin_guest_manager.h" namespace extensions { -content::WebContents* GetGuestForOwnerContents(content::WebContents* guest) { - content::WebContentsImpl* guest_impl = - static_cast(guest); - content::BrowserPluginEmbedder* embedder = - guest_impl->GetBrowserPluginEmbedder(); - if (embedder) { - content::BrowserPluginGuest* guest = embedder->GetFullPageGuest(); - if (guest) - return guest->web_contents(); +namespace { + +bool InsertWebContents(std::vector* vector, + content::WebContents* web_contents) { + vector->push_back(web_contents); + return false; // Continue iterating. +} + +} // namespace + +content::WebContents* GetFullPageGuestForOwnerContents( + content::WebContents* owner) { + content::WebContentsImpl* owner_impl = + static_cast(owner); + content::BrowserPluginEmbedder* plugin_embedder = + owner_impl->GetBrowserPluginEmbedder(); + if (plugin_embedder) { + content::BrowserPluginGuest* plugin_guest = + plugin_embedder->GetFullPageGuest(); + if (plugin_guest) + return plugin_guest->web_contents(); } return NULL; } -content::WebContents* GetOwnerForGuestContents(content::WebContents* owner) { - content::WebContentsImpl* owner_impl = - static_cast(owner); - content::BrowserPluginGuest* guest = owner_impl->GetBrowserPluginGuest(); - if (guest) - return guest->embedder_web_contents(); +void GetAllGuestsForOwnerContents(content::WebContents* owner, + std::vector* guests) { + content::BrowserPluginGuestManager* plugin_guest_manager = + owner->GetBrowserContext()->GetGuestManager(); + plugin_guest_manager->ForEachGuest(owner, + base::Bind(InsertWebContents, guests)); +} + +content::WebContents* GetOwnerForGuestContents(content::WebContents* guest) { + content::WebContentsImpl* guest_impl = + static_cast(guest); + content::BrowserPluginGuest* plugin_guest = + guest_impl->GetBrowserPluginGuest(); + if (plugin_guest) + return plugin_guest->embedder_web_contents(); return NULL; } diff --git a/libcef/browser/extensions/browser_extensions_util.h b/libcef/browser/extensions/browser_extensions_util.h index 8b0db55cf..c6c894cd1 100644 --- a/libcef/browser/extensions/browser_extensions_util.h +++ b/libcef/browser/extensions/browser_extensions_util.h @@ -5,17 +5,24 @@ #ifndef CEF_LIBCEF_BROWSER_EXTENSIONS_BROWSER_EXTENSIONS_UTIL_H_ #define CEF_LIBCEF_BROWSER_EXTENSIONS_BROWSER_EXTENSIONS_UTIL_H_ +#include + namespace content { class WebContents; } namespace extensions { -// Returns the WebContents that owns the specified |guest|, if any. -content::WebContents* GetGuestForOwnerContents(content::WebContents* guest); +// Returns the full-page guest WebContents for the specified |owner|, if any. +content::WebContents* GetFullPageGuestForOwnerContents( + content::WebContents* owner); -// Returns the guest WebContents for the specified |owner|, if any. -content::WebContents* GetOwnerForGuestContents(content::WebContents* owner); +// Populates |guests| with all guest WebContents with the specified |owner|. +void GetAllGuestsForOwnerContents(content::WebContents* owner, + std::vector* guests); + +// Returns the WebContents that owns the specified |guest|, if any. +content::WebContents* GetOwnerForGuestContents(content::WebContents* guest); } // namespace extensions diff --git a/libcef/browser/render_widget_host_view_osr.cc b/libcef/browser/render_widget_host_view_osr.cc index 5da020c66..2063eb2fb 100644 --- a/libcef/browser/render_widget_host_view_osr.cc +++ b/libcef/browser/render_widget_host_view_osr.cc @@ -498,6 +498,11 @@ CefRenderWidgetHostViewOSR::~CefRenderWidgetHostViewOSR() { delegated_frame_host_.reset(NULL); compositor_.reset(NULL); root_layer_.reset(NULL); + + DCHECK(parent_host_view_ == NULL); + DCHECK(popup_host_view_ == NULL); + DCHECK(child_host_view_ == NULL); + DCHECK(guest_host_views_.empty()); } // Called for full-screen widgets. @@ -508,7 +513,7 @@ void CefRenderWidgetHostViewOSR::InitAsChild(gfx::NativeView parent_view) { if (parent_host_view_->child_host_view_) { // Cancel the previous popup widget. - parent_host_view_->child_host_view_->CancelChildWidget(); + parent_host_view_->child_host_view_->CancelWidget(); } parent_host_view_->set_child_host_view(this); @@ -690,7 +695,7 @@ void CefRenderWidgetHostViewOSR::InitAsPopup( if (parent_host_view_->popup_host_view_) { // Cancel the previous popup widget. - parent_host_view_->popup_host_view_->CancelPopupWidget(); + parent_host_view_->popup_host_view_->CancelWidget(); } parent_host_view_->set_popup_host_view(this); @@ -792,6 +797,7 @@ void CefRenderWidgetHostViewOSR::RenderProcessGone( parent_host_view_ = NULL; popup_host_view_ = NULL; child_host_view_ = NULL; + guest_host_views_.clear(); } void CefRenderWidgetHostViewOSR::Destroy() { @@ -799,15 +805,14 @@ void CefRenderWidgetHostViewOSR::Destroy() { is_destroyed_ = true; if (has_parent_) { - if (IsPopupWidget()) - CancelPopupWidget(); - else - CancelChildWidget(); + CancelWidget(); } else { if (popup_host_view_) - popup_host_view_->CancelPopupWidget(); + popup_host_view_->CancelWidget(); if (child_host_view_) - child_host_view_->CancelChildWidget(); + child_host_view_->CancelWidget(); + for (auto guest_host_view : guest_host_views_) + guest_host_view->CancelWidget(); Hide(); } } @@ -978,6 +983,8 @@ bool CefRenderWidgetHostViewOSR::OnMessageReceived(const IPC::Message& msg) { } void CefRenderWidgetHostViewOSR::OnSetNeedsBeginFrames(bool enabled) { + SetFrameRate(); + // Start/stop the timer that sends BeginFrame requests. begin_frame_timer_->SetActive(enabled); if (software_output_device_) { @@ -1094,6 +1101,10 @@ void CefRenderWidgetHostViewOSR::OnScreenInfoChanged() { // We might want to change the cursor scale factor here as well - see the // cache for the current_cursor_, as passed by UpdateCursor from the renderer // in the rwhv_aura (current_cursor_.SetScaleFactor) + + // Notify the guest hosts if any. + for (auto guest_host_view : guest_host_views_) + guest_host_view->OnScreenInfoChanged(); } void CefRenderWidgetHostViewOSR::Invalidate( @@ -1174,7 +1185,7 @@ void CefRenderWidgetHostViewOSR::SendMouseWheelEvent( // Execute asynchronously to avoid deleting the widget from inside some // other callback. CEF_POST_TASK(CEF_UIT, - base::Bind(&CefRenderWidgetHostViewOSR::CancelPopupWidget, + base::Bind(&CefRenderWidgetHostViewOSR::CancelWidget, popup_host_view_->weak_ptr_factory_.GetWeakPtr())); } } @@ -1205,6 +1216,10 @@ void CefRenderWidgetHostViewOSR::SendFocusEvent(bool focus) { void CefRenderWidgetHostViewOSR::UpdateFrameRate() { frame_rate_threshold_ms_ = 0; SetFrameRate(); + + // Notify the guest hosts if any. + for (auto guest_host_view : guest_host_views_) + guest_host_view->UpdateFrameRate(); } void CefRenderWidgetHostViewOSR::HoldResize() { @@ -1258,6 +1273,16 @@ void CefRenderWidgetHostViewOSR::OnPaint( ReleaseResize(); } +void CefRenderWidgetHostViewOSR::AddGuestHostView( + CefRenderWidgetHostViewOSR* guest_host) { + guest_host_views_.insert(guest_host); +} + +void CefRenderWidgetHostViewOSR::RemoveGuestHostView( + CefRenderWidgetHostViewOSR* guest_host) { + guest_host_views_.erase(guest_host); +} + // static int CefRenderWidgetHostViewOSR::ClampFrameRate(int frame_rate) { if (frame_rate < 1) @@ -1268,16 +1293,21 @@ int CefRenderWidgetHostViewOSR::ClampFrameRate(int frame_rate) { } void CefRenderWidgetHostViewOSR::SetFrameRate() { - DCHECK(browser_impl_.get()); - if (!browser_impl_.get()) - return; + CefRefPtr browser; + if (parent_host_view_) { + // Use the same frame rate as the embedding browser. + browser = parent_host_view_->browser_impl_; + } else { + browser = browser_impl_; + } + CHECK(browser); // Only set the frame rate one time. if (frame_rate_threshold_ms_ != 0) return; const int frame_rate = - ClampFrameRate(browser_impl_->settings().windowless_frame_rate); + ClampFrameRate(browser->settings().windowless_frame_rate); frame_rate_threshold_ms_ = 1000 / frame_rate; // Configure the VSync interval for the browser process. @@ -1377,15 +1407,13 @@ void CefRenderWidgetHostViewOSR::SendBeginFrame(base::TimeTicks frame_time, vsync_period, cc::BeginFrameArgs::NORMAL))); } -void CefRenderWidgetHostViewOSR::CancelPopupWidget() { - DCHECK(IsPopupWidget()); - +void CefRenderWidgetHostViewOSR::CancelWidget() { if (render_widget_host_) render_widget_host_->LostCapture(); Hide(); - if (browser_impl_.get()) { + if (IsPopupWidget() && browser_impl_.get()) { CefRefPtr handler = browser_impl_->client()->GetRenderHandler(); if (handler.get()) @@ -1394,25 +1422,12 @@ void CefRenderWidgetHostViewOSR::CancelPopupWidget() { } if (parent_host_view_) { - parent_host_view_->set_popup_host_view(NULL); - parent_host_view_ = NULL; - } - - if (render_widget_host_ && !is_destroyed_) { - is_destroyed_ = true; - // Results in a call to Destroy(). - render_widget_host_->Shutdown(); - } -} - -void CefRenderWidgetHostViewOSR::CancelChildWidget() { - if (render_widget_host_) - render_widget_host_->LostCapture(); - - Hide(); - - if (parent_host_view_) { - parent_host_view_->set_child_host_view(NULL); + if (parent_host_view_->popup_host_view_ == this) + parent_host_view_->set_popup_host_view(NULL); + else if (parent_host_view_->child_host_view_ == this) + parent_host_view_->set_child_host_view(NULL); + else + parent_host_view_->RemoveGuestHostView(this); parent_host_view_ = NULL; } diff --git a/libcef/browser/render_widget_host_view_osr.h b/libcef/browser/render_widget_host_view_osr.h index 30e99cc6a..24bdc4993 100644 --- a/libcef/browser/render_widget_host_view_osr.h +++ b/libcef/browser/render_widget_host_view_osr.h @@ -7,6 +7,7 @@ #define CEF_LIBCEF_BROWSER_RENDER_WIDGET_HOST_VIEW_OSR_H_ #pragma once +#include #include #include "include/cef_base.h" @@ -255,6 +256,9 @@ class CefRenderWidgetHostViewOSR gfx::Range* actual_range) const; #endif // defined(OS_MACOSX) + void AddGuestHostView(CefRenderWidgetHostViewOSR* guest_host); + void RemoveGuestHostView(CefRenderWidgetHostViewOSR* guest_host); + CefRefPtr browser_impl() const { return browser_impl_; } void set_browser_impl(CefRefPtr browser) { browser_impl_ = browser; @@ -286,8 +290,7 @@ class CefRenderWidgetHostViewOSR void SendBeginFrame(base::TimeTicks frame_time, base::TimeDelta vsync_period); - void CancelPopupWidget(); - void CancelChildWidget(); + void CancelWidget(); void OnScrollOffsetChanged(); @@ -357,6 +360,7 @@ class CefRenderWidgetHostViewOSR CefRenderWidgetHostViewOSR* parent_host_view_; CefRenderWidgetHostViewOSR* popup_host_view_; CefRenderWidgetHostViewOSR* child_host_view_; + std::set guest_host_views_; CefRefPtr browser_impl_; diff --git a/libcef/browser/web_contents_view_osr.cc b/libcef/browser/web_contents_view_osr.cc index 9f9b891d8..c0133b4e0 100644 --- a/libcef/browser/web_contents_view_osr.cc +++ b/libcef/browser/web_contents_view_osr.cc @@ -119,8 +119,16 @@ content::RenderWidgetHostViewBase* CefWebContentsViewOSR::CreateViewForWidget( if (guest_) { // Based on WebContentsViewGuest::CreateViewForWidget. + content::WebContents* embedder_web_contents = + guest_->embedder_web_contents(); + CefRenderWidgetHostViewOSR* embedder_host_view = + static_cast( + embedder_web_contents->GetRenderViewHost()->GetView()); + CefRenderWidgetHostViewOSR* platform_widget = - new CefRenderWidgetHostViewOSR(render_widget_host, NULL); + new CefRenderWidgetHostViewOSR(render_widget_host, embedder_host_view); + embedder_host_view->AddGuestHostView(platform_widget); + return new content::RenderWidgetHostViewGuest( render_widget_host, guest_,