Commit 70ab264c authored by fsamuel@chromium.org's avatar fsamuel@chromium.org

Move guest lifetime management to chrome

Now, GuestViewBase takes full responsibility for managing the lifetime of the guest WebContents. The only signal coming from content is that indicating that the BrowserPlugin has been destroyed.

BUG=364141, 330264
TBR=jam@chromium.org for trivial browser_plugin_guest_delegate.h code deletion (w00t!)

Review URL: https://codereview.chromium.org/306003002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@273759 0039d316-1c4b-4281-b951-d872f2087c98
parent 6b86f957
......@@ -17,8 +17,7 @@ AdViewGuest::AdViewGuest(int guest_instance_id,
const std::string& extension_id)
: GuestView<AdViewGuest>(guest_instance_id,
guest_web_contents,
extension_id),
WebContentsObserver(guest_web_contents) {
extension_id) {
}
// static
......
......@@ -7,17 +7,14 @@
#include "base/values.h"
#include "chrome/browser/guest_view/guest_view.h"
#include "content/public/browser/web_contents_observer.h"
// An AdViewGuest is a WebContentsObserver on the guest WebContents of a
// <adview> tag. It provides the browser-side implementation of the <adview>
// API and manages the lifetime of <adview> extension events. AdViewGuest is
// created on attachment. When a guest WebContents is associated with
// a particular embedder WebContents, we call this "attachment".
// An AdViewGuest provides the browser-side implementation of the <adview> API
// and manages the dispatch of <adview> extension events. AdViewGuest is created
// on attachment. When a guest WebContents is associated with a particular
// embedder WebContents, we call this "attachment".
// TODO(fsamuel): There might be an opportunity here to refactor and reuse code
// between AdViewGuest and WebViewGuest.
class AdViewGuest : public GuestView<AdViewGuest>,
public content::WebContentsObserver {
class AdViewGuest : public GuestView<AdViewGuest> {
public:
AdViewGuest(int guest_instance_id,
content::WebContents* guest_web_contents,
......
......@@ -49,6 +49,7 @@ class GuestView : public GuestViewBase {
: GuestViewBase(guest_instance_id,
guest_web_contents,
embedder_extension_id) {}
virtual ~GuestView() {}
private:
DISALLOW_COPY_AND_ASSIGN(GuestView);
......
......@@ -40,10 +40,35 @@ scoped_ptr<base::DictionaryValue> GuestViewBase::Event::GetArguments() {
return args_.Pass();
}
// This observer ensures that the GuestViewBase destroys itself when its
// embedder goes away.
class GuestViewBase::EmbedderWebContentsObserver : public WebContentsObserver {
public:
explicit EmbedderWebContentsObserver(GuestViewBase* guest)
: WebContentsObserver(guest->embedder_web_contents()),
guest_(guest) {
}
virtual ~EmbedderWebContentsObserver() {
}
// WebContentsObserver implementation.
virtual void WebContentsDestroyed() OVERRIDE {
guest_->embedder_web_contents_ = NULL;
guest_->EmbedderDestroyed();
guest_->Destroy();
}
private:
GuestViewBase* guest_;
DISALLOW_COPY_AND_ASSIGN(EmbedderWebContentsObserver);
};
GuestViewBase::GuestViewBase(int guest_instance_id,
WebContents* guest_web_contents,
const std::string& embedder_extension_id)
: guest_web_contents_(guest_web_contents),
: WebContentsObserver(guest_web_contents),
embedder_web_contents_(NULL),
embedder_extension_id_(embedder_extension_id),
embedder_render_process_id_(0),
......@@ -155,6 +180,8 @@ base::WeakPtr<GuestViewBase> GuestViewBase::AsWeakPtr() {
void GuestViewBase::Attach(content::WebContents* embedder_web_contents,
const base::DictionaryValue& args) {
embedder_web_contents_ = embedder_web_contents;
embedder_web_contents_observer_.reset(
new EmbedderWebContentsObserver(this));
embedder_render_process_id_ =
embedder_web_contents->GetRenderProcessHost()->GetID();
args.GetInteger(guestview::kParameterInstanceId, &view_instance_id_);
......@@ -178,7 +205,7 @@ void GuestViewBase::Attach(content::WebContents* embedder_web_contents,
void GuestViewBase::Destroy() {
if (!destruction_callback_.is_null())
destruction_callback_.Run(guest_web_contents());
destruction_callback_.Run();
delete guest_web_contents();
}
......@@ -196,6 +223,10 @@ void GuestViewBase::RegisterDestructionCallback(
destruction_callback_ = callback;
}
void GuestViewBase::WebContentsDestroyed() {
delete this;
}
bool GuestViewBase::ShouldFocusPageAfterCrash() {
// Focus is managed elsewhere.
return false;
......
......@@ -12,6 +12,7 @@
#include "content/public/browser/browser_plugin_guest_delegate.h"
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_contents_delegate.h"
#include "content/public/browser/web_contents_observer.h"
struct RendererContentSettingRules;
......@@ -20,7 +21,8 @@ struct RendererContentSettingRules;
// WebContents and an embedder WebContents. It receives events issued from
// the guest and relays them to the embedder.
class GuestViewBase : public content::BrowserPluginGuestDelegate,
public content::WebContentsDelegate {
public content::WebContentsDelegate,
public content::WebContentsObserver {
public:
class Event {
public:
......@@ -74,6 +76,11 @@ class GuestViewBase : public content::BrowserPluginGuestDelegate,
virtual const char* GetViewType() const = 0;
// This method can be overridden by subclasses. It indicates that this guest's
// embedder has been destroyed and the guest will be destroyed shortly. This
// method gives derived classes the opportunity to perform some cleanup.
virtual void EmbedderDestroyed() {}
bool IsViewType(const char* const view_type) const {
return !strcmp(GetViewType(), view_type);
}
......@@ -89,7 +96,7 @@ class GuestViewBase : public content::BrowserPluginGuestDelegate,
// Returns the guest WebContents.
content::WebContents* guest_web_contents() const {
return guest_web_contents_;
return web_contents();
}
// Returns the extra parameters associated with this GuestView passed
......@@ -127,6 +134,9 @@ class GuestViewBase : public content::BrowserPluginGuestDelegate,
void SetOpener(GuestViewBase* opener);
// WebContentsObserver implementation.
virtual void WebContentsDestroyed() OVERRIDE;
// WebContentsDelegate implementation.
virtual bool ShouldFocusPageAfterCrash() OVERRIDE;
virtual bool PreHandleGestureEvent(
......@@ -147,9 +157,10 @@ class GuestViewBase : public content::BrowserPluginGuestDelegate,
void DispatchEvent(Event* event);
private:
class EmbedderWebContentsObserver;
void SendQueuedEvents();
content::WebContents* const guest_web_contents_;
content::WebContents* embedder_web_contents_;
const std::string embedder_extension_id_;
int embedder_render_process_id_;
......@@ -176,6 +187,8 @@ class GuestViewBase : public content::BrowserPluginGuestDelegate,
// are passed along to new guests that are created from this guest.
scoped_ptr<base::DictionaryValue> extra_params_;
scoped_ptr<EmbedderWebContentsObserver> embedder_web_contents_observer_;
// This is used to ensure pending tasks will not fire after this object is
// destroyed.
base::WeakPtrFactory<GuestViewBase> weak_ptr_factory_;
......
......@@ -178,7 +178,6 @@ WebViewGuest::WebViewGuest(int guest_instance_id,
: GuestView<WebViewGuest>(guest_instance_id,
guest_web_contents,
embedder_extension_id),
WebContentsObserver(guest_web_contents),
script_executor_(new extensions::ScriptExecutor(guest_web_contents,
&script_observers_)),
pending_context_menu_request_id_(0),
......@@ -335,24 +334,22 @@ void WebViewGuest::Attach(WebContents* embedder_web_contents,
AddWebViewToExtensionRendererState();
}
bool WebViewGuest::HandleContextMenu(
const content::ContextMenuParams& params) {
ContextMenuDelegate* menu_delegate =
ContextMenuDelegate::FromWebContents(guest_web_contents());
DCHECK(menu_delegate);
pending_menu_ = menu_delegate->BuildMenu(guest_web_contents(), params);
// Pass it to embedder.
int request_id = ++pending_context_menu_request_id_;
scoped_ptr<base::DictionaryValue> args(new base::DictionaryValue());
scoped_ptr<base::ListValue> items =
MenuModelToValue(pending_menu_->menu_model());
args->Set(webview::kContextMenuItems, items.release());
args->SetInteger(webview::kRequestId, request_id);
DispatchEvent(new GuestViewBase::Event(webview::kEventContextMenu,
args.Pass()));
return true;
void WebViewGuest::EmbedderDestroyed() {
// TODO(fsamuel): WebRequest event listeners for <webview> should survive
// reparenting of a <webview> within a single embedder. Right now, we keep
// around the browser state for the listener for the lifetime of the embedder.
// Ideally, the lifetime of the listeners should match the lifetime of the
// <webview> DOM node. Once http://crbug.com/156219 is resolved we can move
// the call to RemoveWebViewEventListenersOnIOThread back to
// WebViewGuest::WebContentsDestroyed.
content::BrowserThread::PostTask(
content::BrowserThread::IO,
FROM_HERE,
base::Bind(
&RemoveWebViewEventListenersOnIOThread,
browser_context(), embedder_extension_id(),
embedder_render_process_id(),
view_instance_id()));
}
bool WebViewGuest::AddMessageToConsole(WebContents* source,
......@@ -402,24 +399,6 @@ void WebViewGuest::DidAttach() {
}
}
void WebViewGuest::EmbedderDestroyed() {
// TODO(fsamuel): WebRequest event listeners for <webview> should survive
// reparenting of a <webview> within a single embedder. Right now, we keep
// around the browser state for the listener for the lifetime of the embedder.
// Ideally, the lifetime of the listeners should match the lifetime of the
// <webview> DOM node. Once http://crbug.com/156219 is resolved we can move
// the call to RemoveWebViewEventListenersOnIOThread back to
// WebViewGuest::WebContentsDestroyed.
content::BrowserThread::PostTask(
content::BrowserThread::IO,
FROM_HERE,
base::Bind(
&RemoveWebViewEventListenersOnIOThread,
browser_context(), embedder_extension_id(),
embedder_render_process_id(),
view_instance_id()));
}
void WebViewGuest::FindReply(WebContents* source,
int request_id,
int number_of_matches,
......@@ -430,6 +409,26 @@ void WebViewGuest::FindReply(WebContents* source,
active_match_ordinal, final_update);
}
bool WebViewGuest::HandleContextMenu(
const content::ContextMenuParams& params) {
ContextMenuDelegate* menu_delegate =
ContextMenuDelegate::FromWebContents(guest_web_contents());
DCHECK(menu_delegate);
pending_menu_ = menu_delegate->BuildMenu(guest_web_contents(), params);
// Pass it to embedder.
int request_id = ++pending_context_menu_request_id_;
scoped_ptr<base::DictionaryValue> args(new base::DictionaryValue());
scoped_ptr<base::ListValue> items =
MenuModelToValue(pending_menu_->menu_model());
args->Set(webview::kContextMenuItems, items.release());
args->SetInteger(webview::kRequestId, request_id);
DispatchEvent(new GuestViewBase::Event(webview::kEventContextMenu,
args.Pass()));
return true;
}
void WebViewGuest::HandleKeyboardEvent(
WebContents* source,
const content::NativeWebKeyboardEvent& event) {
......@@ -884,6 +883,7 @@ void WebViewGuest::WebContentsDestroyed() {
embedder_extension_id(), view_instance_id()));
RemoveWebViewFromExtensionRendererState(web_contents());
GuestViewBase::WebContentsDestroyed();
}
void WebViewGuest::UserAgentOverrideSet(const std::string& user_agent) {
......
......@@ -16,7 +16,6 @@
#include "chrome/common/extensions/api/webview.h"
#include "content/public/browser/javascript_dialog_manager.h"
#include "content/public/browser/notification_registrar.h"
#include "content/public/browser/web_contents_observer.h"
#include "third_party/WebKit/public/web/WebFindOptions.h"
#if defined(OS_CHROMEOS)
......@@ -36,16 +35,14 @@ namespace ui {
class SimpleMenuModel;
} // namespace ui
// A WebViewGuest is a WebContentsObserver on the guest WebContents of a
// <webview> tag. It provides the browser-side implementation of the <webview>
// API and manages the lifetime of <webview> extension events. WebViewGuest is
// A WebViewGuest provides the browser-side implementation of the <webview> API
// and manages the dispatch of <webview> extension events. WebViewGuest is
// created on attachment. That is, when a guest WebContents is associated with
// a particular embedder WebContents. This happens on either initial navigation
// or through the use of the New Window API, when a new window is attached to
// a particular <webview>.
class WebViewGuest : public GuestView<WebViewGuest>,
public content::NotificationObserver,
public content::WebContentsObserver {
public content::NotificationObserver {
public:
WebViewGuest(int guest_instance_id,
content::WebContents* guest_web_contents,
......@@ -69,10 +66,7 @@ class WebViewGuest : public GuestView<WebViewGuest>,
// GuestViewBase implementation.
virtual void Attach(content::WebContents* embedder_web_contents,
const base::DictionaryValue& args) OVERRIDE;
// BrowserPluginGuestDelegate public implementation.
virtual bool HandleContextMenu(
const content::ContextMenuParams& params) OVERRIDE;
virtual void EmbedderDestroyed() OVERRIDE;
// WebContentsDelegate implementation.
virtual bool AddMessageToConsole(content::WebContents* source,
......@@ -89,6 +83,8 @@ class WebViewGuest : public GuestView<WebViewGuest>,
const gfx::Rect& selection_rect,
int active_match_ordinal,
bool final_update) OVERRIDE;
virtual bool HandleContextMenu(
const content::ContextMenuParams& params) OVERRIDE;
virtual void HandleKeyboardEvent(
content::WebContents* source,
const content::NativeWebKeyboardEvent& event) OVERRIDE;
......@@ -126,9 +122,8 @@ class WebViewGuest : public GuestView<WebViewGuest>,
const GURL& target_url,
content::WebContents* new_contents) OVERRIDE;
// GuestDelegate implementation.
// BrowserPluginGuestDelegate implementation.
virtual void DidAttach() OVERRIDE;
virtual void EmbedderDestroyed() OVERRIDE;
virtual bool IsDragAndDropEnabled() OVERRIDE;
virtual void SizeChanged(const gfx::Size& old_size, const gfx::Size& new_size)
OVERRIDE;
......
......@@ -43,10 +43,6 @@ namespace content {
// static
BrowserPluginHostFactory* BrowserPluginGuest::factory_ = NULL;
namespace {
} // namespace
class BrowserPluginGuest::EmbedderWebContentsObserver
: public WebContentsObserver {
public:
......@@ -58,11 +54,7 @@ class BrowserPluginGuest::EmbedderWebContentsObserver
virtual ~EmbedderWebContentsObserver() {
}
// WebContentsObserver:
virtual void WebContentsDestroyed() OVERRIDE {
browser_plugin_guest_->EmbedderDestroyed();
}
// WebContentsObserver implementation.
virtual void WasShown() OVERRIDE {
browser_plugin_guest_->EmbedderVisibilityChanged(true);
}
......@@ -99,13 +91,15 @@ BrowserPluginGuest::BrowserPluginGuest(
last_text_input_type_(ui::TEXT_INPUT_TYPE_NONE),
last_input_mode_(ui::TEXT_INPUT_MODE_DEFAULT),
last_can_compose_inline_(true),
delegate_(NULL),
weak_ptr_factory_(this) {
DCHECK(web_contents);
}
void BrowserPluginGuest::WillDestroy(WebContents* web_contents) {
DCHECK_EQ(web_contents, GetWebContents());
void BrowserPluginGuest::WillDestroy() {
is_in_destruction_ = true;
embedder_web_contents_ = NULL;
delegate_ = NULL;
}
base::WeakPtr<BrowserPluginGuest> BrowserPluginGuest::AsWeakPtr() {
......@@ -119,13 +113,6 @@ bool BrowserPluginGuest::LockMouse(bool allowed) {
return embedder_web_contents()->GotResponseToLockMouseRequest(allowed);
}
void BrowserPluginGuest::EmbedderDestroyed() {
embedder_web_contents_ = NULL;
if (delegate_)
delegate_->EmbedderDestroyed();
Destroy();
}
void BrowserPluginGuest::Destroy() {
if (!delegate_)
return;
......@@ -301,7 +288,7 @@ BrowserPluginGuest* BrowserPluginGuest::Create(
delegate->RegisterDestructionCallback(
base::Bind(&BrowserPluginGuest::WillDestroy,
base::Unretained(guest)));
guest->SetDelegate(delegate);
guest->set_delegate(delegate);
}
return guest;
}
......@@ -407,11 +394,6 @@ void BrowserPluginGuest::EndSystemDrag() {
guest_rvh->DragSourceSystemDragEnded();
}
void BrowserPluginGuest::SetDelegate(BrowserPluginGuestDelegate* delegate) {
DCHECK(!delegate_);
delegate_.reset(delegate);
}
void BrowserPluginGuest::SendQueuedMessages() {
if (!attached())
return;
......
......@@ -106,10 +106,6 @@ class CONTENT_EXPORT BrowserPluginGuest : public WebContentsObserver {
// the mouse has been successfully locked.
bool LockMouse(bool allowed);
// Called when the embedder WebContents is destroyed to give the
// BrowserPluginGuest an opportunity to clean up after itself.
void EmbedderDestroyed();
// Called when the embedder WebContents changes visibility.
void EmbedderVisibilityChanged(bool visible);
......@@ -199,8 +195,10 @@ class CONTENT_EXPORT BrowserPluginGuest : public WebContentsObserver {
// Called when the drag started by this guest ends at an OS-level.
void EndSystemDrag();
// |this| takes ownership of |delegate|.
void SetDelegate(BrowserPluginGuestDelegate* delegate);
void set_delegate(BrowserPluginGuestDelegate* delegate) {
DCHECK(!delegate_);
delegate_ = delegate;
}
void RespondToPermissionRequest(int request_id,
bool should_allow,
......@@ -226,7 +224,7 @@ class CONTENT_EXPORT BrowserPluginGuest : public WebContentsObserver {
bool has_render_view,
WebContentsImpl* web_contents);
void WillDestroy(WebContents* web_contents);
void WillDestroy();
bool InAutoSizeBounds(const gfx::Size& size) const;
......@@ -400,7 +398,7 @@ class CONTENT_EXPORT BrowserPluginGuest : public WebContentsObserver {
// once the guest is attached to a particular embedder.
std::queue<IPC::Message*> pending_messages_;
scoped_ptr<BrowserPluginGuestDelegate> delegate_;
BrowserPluginGuestDelegate* delegate_;
// Weak pointer used to ask GeolocationPermissionContext about geolocation
// permission.
......
......@@ -11,14 +11,43 @@
namespace content {
// This observer ensures that the TestBrowserPluginGuestDelegate destroys itself
// when its embedder goes away.
class TestBrowserPluginGuestDelegate::EmbedderWebContentsObserver :
public WebContentsObserver {
public:
explicit EmbedderWebContentsObserver(TestBrowserPluginGuestDelegate* guest)
: WebContentsObserver(guest->GetEmbedderWebContents()),
guest_(guest) {
}
virtual ~EmbedderWebContentsObserver() {
}
// WebContentsObserver implementation.
virtual void WebContentsDestroyed() OVERRIDE {
guest_->Destroy();
}
private:
TestBrowserPluginGuestDelegate* guest_;
DISALLOW_COPY_AND_ASSIGN(EmbedderWebContentsObserver);
};
TestBrowserPluginGuestDelegate::TestBrowserPluginGuestDelegate(
BrowserPluginGuest* guest) :
WebContentsObserver(guest->GetWebContents()),
guest_(guest) {
}
TestBrowserPluginGuestDelegate::~TestBrowserPluginGuestDelegate() {
}
WebContents* TestBrowserPluginGuestDelegate::GetEmbedderWebContents() const {
return guest_->embedder_web_contents();
}
void TestBrowserPluginGuestDelegate::LoadURLWithParams(
const GURL& url,
const Referrer& referrer,
......@@ -31,9 +60,19 @@ void TestBrowserPluginGuestDelegate::LoadURLWithParams(
web_contents->GetController().LoadURLWithParams(load_url_params);
}
void TestBrowserPluginGuestDelegate::WebContentsDestroyed() {
delete this;
}
void TestBrowserPluginGuestDelegate::DidAttach() {
embedder_web_contents_observer_.reset(
new EmbedderWebContentsObserver(this));
}
void TestBrowserPluginGuestDelegate::Destroy() {
if (!destruction_callback_.is_null())
destruction_callback_.Run(guest_->GetWebContents());
destruction_callback_.Run();
delete guest_->GetWebContents();
}
......
......@@ -8,6 +8,7 @@
#include "content/public/browser/browser_plugin_guest_delegate.h"
#include "base/callback.h"
#include "content/public/browser/web_contents_observer.h"
#include "content/public/common/page_transition_types.h"
#include "url/gurl.h"
......@@ -16,26 +17,34 @@ namespace content {
class BrowserPluginGuest;
struct Referrer;
class TestBrowserPluginGuestDelegate : public BrowserPluginGuestDelegate {
class TestBrowserPluginGuestDelegate : public BrowserPluginGuestDelegate,
public WebContentsObserver {
public:
explicit TestBrowserPluginGuestDelegate(BrowserPluginGuest* guest);
virtual ~TestBrowserPluginGuestDelegate();
void ResetStates();
WebContents* GetEmbedderWebContents() const;
private:
class EmbedderWebContentsObserver;
void LoadURLWithParams(const GURL& url,
const Referrer& referrer,
PageTransition transition_type,
WebContents* web_contents);
// WebContentsObserver implementation.
virtual void WebContentsDestroyed() OVERRIDE;
// Overridden from BrowserPluginGuestDelegate:
virtual void DidAttach() OVERRIDE;
virtual void Destroy() OVERRIDE;
virtual void NavigateGuest(const std::string& src) OVERRIDE;
virtual void RegisterDestructionCallback(
const DestructionCallback& callback) OVERRIDE;
BrowserPluginGuest* guest_;
scoped_ptr<EmbedderWebContentsObserver> embedder_web_contents_observer_;
DestructionCallback destruction_callback_;
......
......@@ -108,7 +108,7 @@ WebContents* TestGuestManager::CreateGuest(
WebContentsImpl* guest_web_contents = static_cast<WebContentsImpl*>(
WebContents::Create(create_params));
BrowserPluginGuest* guest = guest_web_contents->GetBrowserPluginGuest();
guest_web_contents->GetBrowserPluginGuest()->SetDelegate(
guest_web_contents->GetBrowserPluginGuest()->set_delegate(
new TestBrowserPluginGuestDelegate(guest));
AddGuest(instance_id, guest_web_contents);
return guest_web_contents;
......
......@@ -26,9 +26,6 @@ class CONTENT_EXPORT BrowserPluginGuestDelegate {
// Notification that the embedder has completed attachment.
virtual void DidAttach() {}
// Informs the delegate that the embedder has been destroyed.
virtual void EmbedderDestroyed() {}
// Requests setting the zoom level to the provided |zoom_level|.
virtual void SetZoom(double zoom_factor) {}
......@@ -54,7 +51,7 @@ class CONTENT_EXPORT BrowserPluginGuestDelegate {
// Registers a |callback| with the delegate that the delegate would call when
// it is about to be destroyed.
typedef base::Callback<void(WebContents*)> DestructionCallback;
typedef base::Callback<void()> DestructionCallback;
virtual void RegisterDestructionCallback(
const DestructionCallback& callback) {}
};
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment