Remove GuestWebContentsCreated

chrome/browser/guest_view/guest_view_base.cc

Index: chrome/browser/guest_view/guest_view_base.cc
diff --git a/chrome/browser/guest_view/guest_view_base.cc b/chrome/browser/guest_view/guest_view_base.cc
index 6448ac9dcfe86d10909629b0ab2e45d164f6eb2d..4a288ac61f4e216e70acb702731b6917430e6fb5 100644
--- a/chrome/browser/guest_view/guest_view_base.cc
+++ b/chrome/browser/guest_view/guest_view_base.cc
@@ -76,13 +76,34 @@ GuestViewBase::GuestViewBase(int guest_instance_id)
       weak_ptr_factory_(this) {
 }
 
-void GuestViewBase::Init(WebContents* guest_web_contents,
-                         const std::string& embedder_extension_id) {
+void GuestViewBase::Init(
+    const std::string& embedder_extension_id,
+    content::RenderProcessHost* embedder_render_process_host,
+    const base::DictionaryValue& create_params) {
   if (initialized_)
     return;
   initialized_ = true;
+
+  CreateWebContents(embedder_extension_id,
+                    embedder_render_process_host,
+                    create_params,
+                    base::Bind(&GuestViewBase::InitWithWebContents,
+                               AsWeakPtr(),
+                               embedder_extension_id,
+                               embedder_render_process_host));
+}
+
+void GuestViewBase::InitWithWebContents(
+    const std::string& embedder_extension_id,
+    content::RenderProcessHost* embedder_render_process_host,
+    content::WebContents* guest_web_contents) {
+  if (!guest_web_contents)

[lazyboy, 2014/06/17 23:46:45]

Should this be DCHECK instead?

[Fady Samuel, 2014/06/18 21:08:33]

Done.

+    return;
+
   browser_context_ = guest_web_contents->GetBrowserContext();
   embedder_extension_id_ = embedder_extension_id;
+  embedder_render_process_id_ = embedder_render_process_host->GetID();
+  embedder_render_process_host->AddObserver(this);
 
   WebContentsObserver::Observe(guest_web_contents);
   guest_web_contents->SetDelegate(this);
@@ -90,18 +111,17 @@ void GuestViewBase::Init(WebContents* guest_web_contents,
       std::make_pair(guest_web_contents, this));
   GuestViewManager::FromBrowserContext(browser_context_)->
       AddGuest(guest_instance_id_, guest_web_contents);
+
+  // Give the derived class an opportunity to perform additional initialization.
+  Init();
 }
 
 // static
 GuestViewBase* GuestViewBase::Create(
     int guest_instance_id,
-    WebContents* guest_web_contents,
-    const std::string& embedder_extension_id,
     const std::string& view_type) {
   if (view_type == "webview") {
-    return new WebViewGuest(guest_instance_id,
-                            guest_web_contents,
-                            embedder_extension_id);
+    return new WebViewGuest(guest_instance_id);
   }
   NOTREACHED();
   return NULL;
@@ -163,7 +183,28 @@ 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

[lazyboy, 2014/06/17 23:46:45]

This comment should also be class level comment for GuestViewBase.

[Fady Samuel, 2014/06/18 21:08:33]

Done.

+  // 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::Destroy() {
+  content::RenderProcessHost* host =
+      content::RenderProcessHost::FromID(embedder_render_process_id());
+  if (host)
+    host->RemoveObserver(this);
   WillDestroy();
   if (!destruction_callback_.is_null())
     destruction_callback_.Run();
@@ -172,11 +213,13 @@ void GuestViewBase::Destroy() {
 
 void GuestViewBase::DidAttach(content::WebContents* embedder_web_contents,
                               const base::DictionaryValue& extra_params) {
+  // After attachment, this GuestViewBase's lifetime is restricted to the

[lazyboy, 2014/06/17 23:46:45]

Same as above, lifetime should also be documented class level.

[Fady Samuel, 2014/06/18 21:08:33]

Done.

+  // 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));
-  embedder_render_process_id_ =
-      embedder_web_contents->GetRenderProcessHost()->GetID();
   extra_params.GetInteger(guestview::kParameterInstanceId, &view_instance_id_);
   extra_params_.reset(extra_params.DeepCopy());
 
@@ -199,6 +242,10 @@ void GuestViewBase::DidAttach(content::WebContents* embedder_web_contents,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
+int GuestViewBase::GetGuestInstanceID() const {

[lazyboy, 2014/06/17 23:46:45]

Why do you add this method? to make it virtual? In that case we can get rid of
guest_instance_id()

[Fady Samuel, 2014/06/18 21:08:33]

Done.

+  return guest_instance_id();
+}
+
 void GuestViewBase::SetOpener(GuestViewBase* guest) {
   if (guest && guest->IsViewType(GetViewType())) {
     opener_ = guest->AsWeakPtr();