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

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

Issue 640293005: Revert of 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 0dba70c50476ea42af872ef241d381d454f3691c..151cefbbfd75d5de2ad5cb4040d4acba5004cfe2 100644
--- a/extensions/browser/guest_view/guest_view_base.cc
+++ b/extensions/browser/guest_view/guest_view_base.cc
@@ -57,15 +57,16 @@
// This observer ensures that the GuestViewBase destroys itself when its
// embedder goes away.
-class GuestViewBase::EmbedderLifetimeObserver : public WebContentsObserver {
+class GuestViewBase::EmbedderWebContentsObserver : public WebContentsObserver {
public:
- EmbedderLifetimeObserver(GuestViewBase* guest,
- content::WebContents* embedder_web_contents)
- : WebContentsObserver(embedder_web_contents),
+ explicit EmbedderWebContentsObserver(GuestViewBase* guest)
+ : WebContentsObserver(guest->embedder_web_contents()),
destroyed_(false),
- guest_(guest) {}
-
- virtual ~EmbedderLifetimeObserver() {}
+ guest_(guest) {
+ }
+
+ virtual ~EmbedderWebContentsObserver() {
+ }
// WebContentsObserver implementation.
virtual void WebContentsDestroyed() override {
@@ -97,7 +98,7 @@
guest_->Destroy();
}
- DISALLOW_COPY_AND_ASSIGN(EmbedderLifetimeObserver);
+ DISALLOW_COPY_AND_ASSIGN(EmbedderWebContentsObserver);
};
GuestViewBase::GuestViewBase(content::BrowserContext* browser_context,
@@ -158,29 +159,21 @@
base::Bind(&GuestViewBase::CompleteInit,
weak_ptr_factory_.GetWeakPtr(),
embedder_extension_id,
- embedder_web_contents,
+ embedder_process_id,
callback));
}
void GuestViewBase::InitWithWebContents(
const std::string& embedder_extension_id,
- content::WebContents* embedder_web_contents,
+ int embedder_render_process_id,
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();
-
- // 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));
+ embedder_render_process_host->AddObserver(this);
WebContentsObserver::Observe(guest_web_contents);
guest_web_contents->SetDelegate(this);
@@ -285,6 +278,23 @@
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();
@@ -314,6 +324,11 @@
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();
@@ -355,16 +370,13 @@
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;
-
- // 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));
- }
-
+ embedder_web_contents_observer_.reset(
+ new EmbedderWebContentsObserver(this));
element_instance_id_ = element_instance_id;
WillAttachToEmbedder();
@@ -476,7 +488,7 @@
}
void GuestViewBase::CompleteInit(const std::string& embedder_extension_id,
- content::WebContents* embedder_web_contents,
+ int embedder_render_process_id,
const WebContentsCreatedCallback& callback,
content::WebContents* guest_web_contents) {
if (!guest_web_contents) {
@@ -486,8 +498,9 @@
callback.Run(NULL);
return;
}
- InitWithWebContents(
- embedder_extension_id, embedder_web_contents, guest_web_contents);
+ InitWithWebContents(embedder_extension_id,
+ embedder_render_process_id,
+ 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