From b4e3817fa26c6c53ea930beab249c51d0370bd31 Mon Sep 17 00:00:00 2001 From: fsamuel Date: Tue, 16 Sep 2014 12:41:09 -0700 Subject: GuestViewContainer lifetime should only be managed by BrowserPlugin and not RenderFrameObserver This prevents racy teardowns that might cause BrowserPlugin to try to delete an invalid pointer BUG=none Test=Open browser sample, and close browser sample. It should not crash. TBR=lazyboy@chromium.org Review URL: https://codereview.chromium.org/574623004 Cr-Commit-Position: refs/heads/master@{#295127} --- content/renderer/browser_plugin/browser_plugin.cc | 9 +++++---- extensions/renderer/guest_view/guest_view_container.cc | 8 ++++++-- extensions/renderer/guest_view/guest_view_container.h | 1 + 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/content/renderer/browser_plugin/browser_plugin.cc b/content/renderer/browser_plugin/browser_plugin.cc index c0a2e8a..1be227d 100644 --- a/content/renderer/browser_plugin/browser_plugin.cc +++ b/content/renderer/browser_plugin/browser_plugin.cc @@ -435,12 +435,13 @@ void BrowserPlugin::destroy() { // If the plugin was initialized then it has a valid |npp_| identifier, and // the |container_| must clear references to the plugin's script objects. DCHECK(!npp_ || container_); - if (container_) + if (container_) { container_->clearScriptObjects(); - // The BrowserPlugin's WebPluginContainer is deleted immediately after this - // call returns, so let's not keep a reference to it around. - g_plugin_container_map.Get().erase(container_); + // The BrowserPlugin's WebPluginContainer is deleted immediately after this + // call returns, so let's not keep a reference to it around. + g_plugin_container_map.Get().erase(container_); + } if (compositing_helper_.get()) compositing_helper_->OnContainerDestroy(); diff --git a/extensions/renderer/guest_view/guest_view_container.cc b/extensions/renderer/guest_view/guest_view_container.cc index 05ccf68..40051b1 100644 --- a/extensions/renderer/guest_view/guest_view_container.cc +++ b/extensions/renderer/guest_view/guest_view_container.cc @@ -23,8 +23,7 @@ GuestViewContainer::GuestViewContainer( GuestViewContainer::~GuestViewContainer() { } -void GuestViewContainer::SetElementInstanceID( - int element_instance_id) { +void GuestViewContainer::SetElementInstanceID(int element_instance_id) { element_instance_id_ = element_instance_id; } @@ -42,6 +41,11 @@ void GuestViewContainer::DidReceiveData(const char* data, int data_length) { html_string_ += value; } +void GuestViewContainer::OnDestruct() { + // GuestViewContainer's lifetime is managed by BrowserPlugin so don't let + // RenderFrameObserver self-destruct here. +} + bool GuestViewContainer::OnMessageReceived(const IPC::Message& message) { if (message.type() != ExtensionMsg_CreateMimeHandlerViewGuestACK::ID) return false; diff --git a/extensions/renderer/guest_view/guest_view_container.h b/extensions/renderer/guest_view/guest_view_container.h index a5695a2..68db47f 100644 --- a/extensions/renderer/guest_view/guest_view_container.h +++ b/extensions/renderer/guest_view/guest_view_container.h @@ -26,6 +26,7 @@ class GuestViewContainer : public content::BrowserPluginDelegate, virtual void DidReceiveData(const char* data, int data_length) OVERRIDE; // RenderFrameObserver override. + virtual void OnDestruct() OVERRIDE; virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE; private: -- cgit v1.1