Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(28)

Unified Diff: extensions/browser/guest_view/guest_view_base.cc

Issue 642573002: Remove the RenderProcessHost observer and attach the WebContentsObserver earlier to the GuestView. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@tests_other0
Patch Set: Created 6 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « extensions/browser/guest_view/guest_view_base.h ('k') | extensions/browser/guest_view/guest_view_manager.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: extensions/browser/guest_view/guest_view_base.cc
diff --git a/extensions/browser/guest_view/guest_view_base.cc b/extensions/browser/guest_view/guest_view_base.cc
index 151cefbbfd75d5de2ad5cb4040d4acba5004cfe2..0dba70c50476ea42af872ef241d381d454f3691c 100644
--- a/extensions/browser/guest_view/guest_view_base.cc
+++ b/extensions/browser/guest_view/guest_view_base.cc
@@ -57,16 +57,15 @@ scoped_ptr<base::DictionaryValue> GuestViewBase::Event::GetArguments() {
// This observer ensures that the GuestViewBase destroys itself when its
// embedder goes away.
-class GuestViewBase::EmbedderWebContentsObserver : public WebContentsObserver {
+class GuestViewBase::EmbedderLifetimeObserver : public WebContentsObserver {
public:
- explicit EmbedderWebContentsObserver(GuestViewBase* guest)
- : WebContentsObserver(guest->embedder_web_contents()),
+ EmbedderLifetimeObserver(GuestViewBase* guest,
+ content::WebContents* embedder_web_contents)
+ : WebContentsObserver(embedder_web_contents),
destroyed_(false),
- guest_(guest) {
- }
+ guest_(guest) {}
- virtual ~EmbedderWebContentsObserver() {
- }
+ virtual ~EmbedderLifetimeObserver() {}
// WebContentsObserver implementation.
virtual void WebContentsDestroyed() override {
@@ -98,7 +97,7 @@ class GuestViewBase::EmbedderWebContentsObserver : public WebContentsObserver {
guest_->Destroy();
}
- DISALLOW_COPY_AND_ASSIGN(EmbedderWebContentsObserver);
+ DISALLOW_COPY_AND_ASSIGN(EmbedderLifetimeObserver);
};
GuestViewBase::GuestViewBase(content::BrowserContext* browser_context,
@@ -159,21 +158,29 @@ void GuestViewBase::Init(const std::string& embedder_extension_id,
base::Bind(&GuestViewBase::CompleteInit,
weak_ptr_factory_.GetWeakPtr(),
embedder_extension_id,
- embedder_process_id,
+ embedder_web_contents,
callback));
}
void GuestViewBase::InitWithWebContents(
const std::string& embedder_extension_id,
- int embedder_render_process_id,
+ content::WebContents* embedder_web_contents,
content::WebContents* guest_web_contents) {
DCHECK(guest_web_contents);
+ DCHECK(embedder_web_contents);
+ int embedder_render_process_id =
+ embedder_web_contents->GetRenderProcessHost()->GetID();
content::RenderProcessHost* embedder_render_process_host =
content::RenderProcessHost::FromID(embedder_render_process_id);
embedder_extension_id_ = embedder_extension_id;
embedder_render_process_id_ = embedder_render_process_host->GetID();
- embedder_render_process_host->AddObserver(this);
+
+ // At this point, we have just created the guest WebContents, we need to add
+ // an observer to the embedder WebContents. This observer will be responsible
+ // for destroying the guest WebContents if the embedder goes away.
+ embedder_web_contents_observer_.reset(
+ new EmbedderLifetimeObserver(this, embedder_web_contents));
WebContentsObserver::Observe(guest_web_contents);
guest_web_contents->SetDelegate(this);
@@ -278,23 +285,6 @@ bool GuestViewBase::IsDragAndDropEnabled() const {
return false;
}
-void GuestViewBase::RenderProcessExited(content::RenderProcessHost* host,
- base::ProcessHandle handle,
- base::TerminationStatus status,
- int exit_code) {
- // GuestViewBase tracks the lifetime of its embedder render process until it
- // is attached to a particular embedder WebContents. At that point, its
- // lifetime is restricted in scope to the lifetime of its embedder
- // WebContents.
- CHECK(!attached());
- CHECK_EQ(host->GetID(), embedder_render_process_id());
-
- // This code path may be reached if the embedder WebContents is killed for
- // whatever reason immediately after a called to GuestViewInternal.createGuest
- // and before attaching the new guest to a frame.
- Destroy();
-}
-
void GuestViewBase::DidAttach(int guest_proxy_routing_id) {
// Give the derived class an opportunity to perform some actions.
DidAttachToEmbedder();
@@ -324,11 +314,6 @@ void GuestViewBase::GuestSizeChanged(const gfx::Size& old_size,
void GuestViewBase::Destroy() {
DCHECK(web_contents());
- content::RenderProcessHost* host =
- content::RenderProcessHost::FromID(embedder_render_process_id());
- if (host)
- host->RemoveObserver(this);
-
// Give the derived class an opportunity to perform some cleanup.
WillDestroy();
@@ -370,13 +355,16 @@ void GuestViewBase::RegisterDestructionCallback(
void GuestViewBase::WillAttach(content::WebContents* embedder_web_contents,
int element_instance_id) {
- // After attachment, this GuestViewBase's lifetime is restricted to the
- // lifetime of its embedder WebContents. Observing the RenderProcessHost
- // of the embedder is no longer necessary.
- embedder_web_contents->GetRenderProcessHost()->RemoveObserver(this);
embedder_web_contents_ = embedder_web_contents;
- embedder_web_contents_observer_.reset(
- new EmbedderWebContentsObserver(this));
+
+ // If we are attaching to a different WebContents than the one that created
+ // the guest, we need to create a new LifetimeObserver.
+ if (embedder_web_contents !=
+ embedder_web_contents_observer_->web_contents()) {
+ embedder_web_contents_observer_.reset(
+ new EmbedderLifetimeObserver(this, embedder_web_contents));
+ }
+
element_instance_id_ = element_instance_id;
WillAttachToEmbedder();
@@ -488,7 +476,7 @@ void GuestViewBase::SendQueuedEvents() {
}
void GuestViewBase::CompleteInit(const std::string& embedder_extension_id,
- int embedder_render_process_id,
+ content::WebContents* embedder_web_contents,
const WebContentsCreatedCallback& callback,
content::WebContents* guest_web_contents) {
if (!guest_web_contents) {
@@ -498,9 +486,8 @@ void GuestViewBase::CompleteInit(const std::string& embedder_extension_id,
callback.Run(NULL);
return;
}
- InitWithWebContents(embedder_extension_id,
- embedder_render_process_id,
- guest_web_contents);
+ InitWithWebContents(
+ embedder_extension_id, embedder_web_contents, guest_web_contents);
callback.Run(guest_web_contents);
}
« no previous file with comments | « extensions/browser/guest_view/guest_view_base.h ('k') | extensions/browser/guest_view/guest_view_manager.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698