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

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
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..973a4db252e31fb034760b32b32ac6f5e45b007b 100644
--- a/extensions/browser/guest_view/guest_view_base.cc
+++ b/extensions/browser/guest_view/guest_view_base.cc
@@ -59,11 +59,12 @@ scoped_ptr<base::DictionaryValue> GuestViewBase::Event::GetArguments() {
// embedder goes away.
class GuestViewBase::EmbedderWebContentsObserver : public WebContentsObserver {
Fady Samuel 2014/10/08 21:45:32 Maybe if we rename this to EmbedderLifetimeObserve
lfg 2014/10/08 23:25:36 Done.
public:
- explicit EmbedderWebContentsObserver(GuestViewBase* guest)
- : WebContentsObserver(guest->embedder_web_contents()),
+ explicit EmbedderWebContentsObserver(
Fady Samuel 2014/10/08 21:45:32 remove explicit
lfg 2014/10/08 23:25:36 Done.
+ GuestViewBase* guest,
+ content::WebContents* embedder_web_contents)
+ : WebContentsObserver(embedder_web_contents),
destroyed_(false),
- guest_(guest) {
- }
+ guest_(guest) {}
virtual ~EmbedderWebContentsObserver() {
}
@@ -160,12 +161,14 @@ void GuestViewBase::Init(const std::string& embedder_extension_id,
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,
Fady Samuel 2014/10/08 21:45:32 Looks like we can get rid of this. You can instea
lfg 2014/10/08 23:25:36 Done.
+ content::WebContents* embedder_web_contents,
content::WebContents* guest_web_contents) {
DCHECK(guest_web_contents);
content::RenderProcessHost* embedder_render_process_host =
@@ -173,7 +176,12 @@ void GuestViewBase::InitWithWebContents(
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 EmbedderWebContentsObserver(this, embedder_web_contents));
WebContentsObserver::Observe(guest_web_contents);
guest_web_contents->SetDelegate(this);
@@ -278,23 +286,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 +315,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 +356,7 @@ 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));
element_instance_id_ = element_instance_id;
WillAttachToEmbedder();
@@ -489,6 +469,7 @@ void GuestViewBase::SendQueuedEvents() {
void GuestViewBase::CompleteInit(const std::string& embedder_extension_id,
int embedder_render_process_id,
Fady Samuel 2014/10/08 21:45:32 Do we need to pass in this field as well? It seems
lfg 2014/10/08 23:25:36 Done.
+ content::WebContents* embedder_web_contents,
const WebContentsCreatedCallback& callback,
content::WebContents* guest_web_contents) {
if (!guest_web_contents) {
@@ -500,6 +481,7 @@ void GuestViewBase::CompleteInit(const std::string& embedder_extension_id,
}
InitWithWebContents(embedder_extension_id,
embedder_render_process_id,
Fady Samuel 2014/10/08 21:45:32 Remove this.
lfg 2014/10/08 23:25:36 Done.
+ embedder_web_contents,
guest_web_contents);
callback.Run(guest_web_contents);
}

Powered by Google App Engine
This is Rietveld 408576698