Remove GuestWebContentsCreated

chrome/browser/guest_view/guest_view_base.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/guest_view/guest_view_base.h"
 
 #include "base/lazy_instance.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/guest_view/guest_view_constants.h"
 #include "chrome/browser/guest_view/guest_view_manager.h"
@@ skipping 58 matching lines @@
 GuestViewBase::GuestViewBase(int guest_instance_id)
     : embedder_web_contents_(NULL),
       embedder_render_process_id_(0),
       browser_context_(NULL),
       guest_instance_id_(guest_instance_id),
       view_instance_id_(guestview::kInstanceIDNone),
       initialized_(false),
       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);
   webcontents_guestview_map.Get().insert(
       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;
 }
 
 // static
 GuestViewBase* GuestViewBase::FromWebContents(WebContents* web_contents) {
   WebContentsGuestViewMap* guest_map = webcontents_guestview_map.Pointer();
   WebContentsGuestViewMap::iterator it = guest_map->find(web_contents);
   return it == guest_map->end() ? NULL : it->second;
@@ skipping 41 matching lines @@
 }
 
 base::WeakPtr<GuestViewBase> GuestViewBase::AsWeakPtr() {
   return weak_ptr_factory_.GetWeakPtr();
 }
 
 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();
   delete guest_web_contents();
 }
 
 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());
 
   // Give the derived class an opportunity to perform some actions.
   DidAttach();
 
   // GuestViewBase::Attach is called prior to initialization (and initial
   // navigation) of the guest in the content layer in order to permit mapping
   // the necessary associations between the <*view> element and its guest. This
   // is needed by the <webview> WebRequest API to allow intercepting resource
   // requests during navigation. However, queued events should be fired after
   // content layer initialization in order to ensure that load events (such as
   // 'loadstop') fire in embedder after the contentWindow is available.
   if (!in_extension())
     return;
 
   base::MessageLoop::current()->PostTask(
       FROM_HERE,
       base::Bind(&GuestViewBase::SendQueuedEvents,
                  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();
     return;
   }
   opener_ = base::WeakPtr<GuestViewBase>();
 }
 
 void GuestViewBase::RegisterDestructionCallback(
     const DestructionCallback& callback) {
@@ skipping 70 matching lines @@
 
 void GuestViewBase::SendQueuedEvents() {
   if (!attached())
     return;
   while (!pending_events_.empty()) {
     linked_ptr<Event> event_ptr = pending_events_.front();
     pending_events_.pop_front();
     DispatchEvent(event_ptr.release());
   }
 }