Remove ContentBrowserClient::GuestWebContentsAttached

BUG=364141

Review URL: https://codereview.chromium.org/334923002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@278846 0039d316-1c4b-4281-b951-d872f2087c98

19 files changed, 318 insertions, 246 deletions

diff --git a/chrome/browser/apps/web_view_browsertest.cc b/chrome/browser/apps/web_view_browsertest.cc
index 3477495..90a0c89 100644
--- a/chrome/browser/apps/web_view_browsertest.cc
+++ b/chrome/browser/apps/web_view_browsertest.cc
@@ -10,6 +10,8 @@
 #include "chrome/browser/apps/app_browsertest_util.h"
 #include "chrome/browser/chrome_content_browser_client.h"
 #include "chrome/browser/extensions/extension_test_message_listener.h"
+#include "chrome/browser/guest_view/guest_view_manager.h"
+#include "chrome/browser/guest_view/guest_view_manager_factory.h"
 #include "chrome/browser/prerender/prerender_link_manager.h"
 #include "chrome/browser/prerender/prerender_link_manager_factory.h"
 #include "chrome/browser/profiles/profile.h"
@@ -92,10 +94,11 @@ class TestInterstitialPageDelegate : public content::InterstitialPageDelegate {
   virtual std::string GetHTMLContents() OVERRIDE { return std::string(); }
 };
 
-// Used to get notified when a guest is created.
-class GuestContentBrowserClient : public chrome::ChromeContentBrowserClient {
+class TestGuestViewManager : public GuestViewManager {
  public:
-  GuestContentBrowserClient() : web_contents_(NULL) {}
+  explicit TestGuestViewManager(content::BrowserContext* context) :
+      GuestViewManager(context),
+      web_contents_(NULL) {}
 
   content::WebContents* WaitForGuestCreated() {
     if (web_contents_)
@@ -107,13 +110,10 @@ class GuestContentBrowserClient : public chrome::ChromeContentBrowserClient {
   }
 
  private:
-  // ChromeContentBrowserClient implementation:
-  virtual void GuestWebContentsAttached(
-      content::WebContents* guest_web_contents,
-      content::WebContents* embedder_web_contents,
-      const base::DictionaryValue& extra_params) OVERRIDE {
-    ChromeContentBrowserClient::GuestWebContentsAttached(
-        guest_web_contents, embedder_web_contents, extra_params);
+  // GuestViewManager override:
+  virtual void AddGuest(int guest_instance_id,
+                        content::WebContents* guest_web_contents) OVERRIDE{
+    GuestViewManager::AddGuest(guest_instance_id, guest_web_contents);
     web_contents_ = guest_web_contents;
 
     if (message_loop_runner_)
@@ -124,6 +124,32 @@ class GuestContentBrowserClient : public chrome::ChromeContentBrowserClient {
   scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
 };
 
+// Test factory for creating test instances of GuestViewManager.
+class TestGuestViewManagerFactory : public GuestViewManagerFactory {
+ public:
+  TestGuestViewManagerFactory() :
+      test_guest_view_manager_(NULL) {}
+
+  virtual ~TestGuestViewManagerFactory() {}
+
+  virtual GuestViewManager* CreateGuestViewManager(
+      content::BrowserContext* context) OVERRIDE {
+    return GetManager(context);
+  }
+
+  TestGuestViewManager* GetManager(content::BrowserContext* context) {
+    if (!test_guest_view_manager_) {
+      test_guest_view_manager_ = new TestGuestViewManager(context);
+    }
+    return test_guest_view_manager_;
+  }
+
+ private:
+  TestGuestViewManager* test_guest_view_manager_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestGuestViewManagerFactory);
+};
+
 class WebContentsHiddenObserver : public content::WebContentsObserver {
  public:
   WebContentsHiddenObserver(content::WebContents* web_contents,
@@ -651,17 +677,13 @@ class WebViewTest : public extensions::PlatformAppBrowserTest {
   }
 
   void LoadAppWithGuest(const std::string& app_path) {
-    GuestContentBrowserClient new_client;
-    content::ContentBrowserClient* old_client =
-        SetBrowserClientForTesting(&new_client);
 
     ExtensionTestMessageListener launched_listener("WebViewTest.LAUNCHED",
                                                    false);
     launched_listener.set_failure_message("WebViewTest.FAILURE");
     LoadAndLaunchPlatformApp(app_path.c_str(), &launched_listener);
 
-    guest_web_contents_ = new_client.WaitForGuestCreated();
-    SetBrowserClientForTesting(old_client);
+    guest_web_contents_ = GetGuestViewManager()->WaitForGuestCreated();
   }
 
   void SendMessageToEmbedder(const std::string& message) {
@@ -699,8 +721,13 @@ class WebViewTest : public extensions::PlatformAppBrowserTest {
     return embedder_web_contents_;
   }
 
+  TestGuestViewManager* GetGuestViewManager() {
+    return factory_.GetManager(browser()->profile());
+  }
+
   WebViewTest() : guest_web_contents_(NULL),
                   embedder_web_contents_(NULL) {
+    GuestViewManager::set_factory_for_testing(&factory_);
   }
 
  private:
@@ -716,6 +743,7 @@ class WebViewTest : public extensions::PlatformAppBrowserTest {
   scoped_ptr<content::FakeSpeechRecognitionManager>
       fake_speech_recognition_manager_;
 
+  TestGuestViewManagerFactory factory_;
   // Note that these are only set if you launch app using LoadAppWithGuest().
   content::WebContents* guest_web_contents_;
   content::WebContents* embedder_web_contents_;
@@ -1181,10 +1209,6 @@ IN_PROC_BROWSER_TEST_F(WebViewTest, MAYBE_InterstitialTeardown) {
 
   LoadAndLaunchPlatformApp("web_view/interstitial_teardown", "EmbedderLoaded");
 
-  GuestContentBrowserClient new_client;
-  content::ContentBrowserClient* old_client =
-      SetBrowserClientForTesting(&new_client);
-
   // Now load the guest.
   content::WebContents* embedder_web_contents = GetFirstAppWindowWebContents();
   ExtensionTestMessageListener second("GuestAddedToDom", false);
@@ -1194,8 +1218,8 @@ IN_PROC_BROWSER_TEST_F(WebViewTest, MAYBE_InterstitialTeardown) {
   ASSERT_TRUE(second.WaitUntilSatisfied());
 
   // Wait for interstitial page to be shown in guest.
-  content::WebContents* guest_web_contents = new_client.WaitForGuestCreated();
-  SetBrowserClientForTesting(old_client);
+  content::WebContents* guest_web_contents =
+      GetGuestViewManager()->WaitForGuestCreated();
   ASSERT_TRUE(guest_web_contents->GetRenderProcessHost()->IsIsolatedGuest());
   WaitForInterstitial(guest_web_contents);
 
diff --git a/chrome/browser/apps/web_view_interactive_browsertest.cc b/chrome/browser/apps/web_view_interactive_browsertest.cc
index 8543024..cd1b152 100644
--- a/chrome/browser/apps/web_view_interactive_browsertest.cc
+++ b/chrome/browser/apps/web_view_interactive_browsertest.cc
@@ -11,6 +11,8 @@
 #include "chrome/browser/chrome_content_browser_client.h"
 #include "chrome/browser/extensions/extension_test_message_listener.h"
 #include "chrome/browser/guest_view/guest_view_base.h"
+#include "chrome/browser/guest_view/guest_view_manager.h"
+#include "chrome/browser/guest_view/guest_view_manager_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/renderer_context_menu/render_view_context_menu_browsertest_util.h"
 #include "chrome/browser/renderer_context_menu/render_view_context_menu_test_util.h"
@@ -33,6 +35,62 @@
 
 using apps::AppWindow;
 
+class TestGuestViewManager : public GuestViewManager {
+ public:
+  explicit TestGuestViewManager(content::BrowserContext* context) :
+      GuestViewManager(context),
+      web_contents_(NULL) {}
+
+  content::WebContents* WaitForGuestCreated() {
+    if (web_contents_)
+      return web_contents_;
+
+    message_loop_runner_ = new content::MessageLoopRunner;
+    message_loop_runner_->Run();
+    return web_contents_;
+  }
+
+ private:
+  // GuestViewManager override:
+  virtual void AddGuest(int guest_instance_id,
+                        content::WebContents* guest_web_contents) OVERRIDE{
+    GuestViewManager::AddGuest(guest_instance_id, guest_web_contents);
+    web_contents_ = guest_web_contents;
+
+    if (message_loop_runner_)
+      message_loop_runner_->Quit();
+  }
+
+  content::WebContents* web_contents_;
+  scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
+};
+
+// Test factory for creating test instances of GuestViewManager.
+class TestGuestViewManagerFactory : public GuestViewManagerFactory {
+ public:
+  TestGuestViewManagerFactory() :
+      test_guest_view_manager_(NULL) {}
+
+  virtual ~TestGuestViewManagerFactory() {}
+
+  virtual GuestViewManager* CreateGuestViewManager(
+      content::BrowserContext* context) OVERRIDE {
+    return GetManager(context);
+  }
+
+  TestGuestViewManager* GetManager(content::BrowserContext* context) {
+    if (!test_guest_view_manager_) {
+      test_guest_view_manager_ = new TestGuestViewManager(context);
+    }
+    return test_guest_view_manager_;
+  }
+
+ private:
+  TestGuestViewManager* test_guest_view_manager_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestGuestViewManagerFactory);
+};
+
 class WebViewInteractiveTest
     : public extensions::PlatformAppBrowserTest {
  public:
@@ -41,7 +99,13 @@ class WebViewInteractiveTest
         embedder_web_contents_(NULL),
         corner_(gfx::Point()),
         mouse_click_result_(false),
-        first_click_(true) {}
+        first_click_(true) {
+    GuestViewManager::set_factory_for_testing(&factory_);
+  }
+
+  TestGuestViewManager* GetGuestViewManager() {
+    return factory_.GetManager(browser()->profile());
+  }
 
   void MoveMouseInsideWindowWithListener(gfx::Point point,
                                          const std::string& message) {
@@ -186,10 +250,6 @@ class WebViewInteractiveTest
   void TestHelper(const std::string& test_name,
                   const std::string& app_location,
                   TestServer test_server) {
-    GuestContentBrowserClient new_client;
-    content::ContentBrowserClient* old_client =
-        SetBrowserClientForTesting(&new_client);
-
     content::WebContents* embedder_web_contents = NULL;
     scoped_ptr<ExtensionTestMessageListener> done_listener(
         RunAppHelper(
@@ -198,10 +258,7 @@ class WebViewInteractiveTest
     ASSERT_TRUE(done_listener);
     ASSERT_TRUE(done_listener->WaitUntilSatisfied());
 
-    guest_web_contents_ = new_client.WaitForGuestCreated();
-    // Reset the browser client so that we do not notice any unexpected
-    // behavior.
-    SetBrowserClientForTesting(old_client);
+    guest_web_contents_ = GetGuestViewManager()->WaitForGuestCreated();
   }
 
   void RunTest(const std::string& app_name) {
@@ -447,6 +504,7 @@ class WebViewInteractiveTest
   }
 
  protected:
+  TestGuestViewManagerFactory factory_;
   content::WebContents* guest_web_contents_;
   content::WebContents* embedder_web_contents_;
   gfx::Point corner_;
@@ -455,39 +513,6 @@ class WebViewInteractiveTest
   // Only used in drag/drop test.
   base::Closure quit_closure_;
   std::string last_drop_data_;
-
- private:
-  // Used to get notified when a guest is created.
-  class GuestContentBrowserClient : public chrome::ChromeContentBrowserClient {
-   public:
-    GuestContentBrowserClient() : web_contents_(NULL) {}
-
-    content::WebContents* WaitForGuestCreated() {
-      if (web_contents_)
-        return web_contents_;
-
-      message_loop_runner_ = new content::MessageLoopRunner;
-      message_loop_runner_->Run();
-      return web_contents_;
-    }
-
-   private:
-    // ChromeContentBrowserClient implementation:
-    virtual void GuestWebContentsAttached(
-        content::WebContents* guest_web_contents,
-        content::WebContents* embedder_web_contents,
-        const base::DictionaryValue& extra_params) OVERRIDE {
-      ChromeContentBrowserClient::GuestWebContentsAttached(
-          guest_web_contents, embedder_web_contents, extra_params);
-      web_contents_ = guest_web_contents;
-
-      if (message_loop_runner_)
-        message_loop_runner_->Quit();
-    }
-
-    content::WebContents* web_contents_;
-    scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
-  };
 };
 
 // ui_test_utils::SendMouseMoveSync doesn't seem to work on OS_MACOSX, and
diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
index 4b32ccd..68726c4 100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -904,25 +904,6 @@ void ChromeContentBrowserClient::GuestWebContentsCreated(
 #endif  // defined(ENABLE_EXTENSIONS)
 }
 
-void ChromeContentBrowserClient::GuestWebContentsAttached(
-    WebContents* guest_web_contents,
-    WebContents* embedder_web_contents,
-    const base::DictionaryValue& extra_params) {
-#if defined(ENABLE_EXTENSIONS)
-  GuestViewBase* guest = GuestViewBase::FromWebContents(guest_web_contents);
-  if (!guest) {
-    // It's ok to return here, since we could be running a browser plugin
-    // outside an extension, and don't need to attach a
-    // BrowserPluginGuestDelegate in that case;
-    // e.g. running with flag --enable-browser-plugin-for-all-view-types.
-    return;
-  }
-  guest->Attach(embedder_web_contents, extra_params);
-#else
-  NOTREACHED();
-#endif  // defined(ENABLE_EXTENSIONS)
-}
-
 void ChromeContentBrowserClient::RenderProcessWillLaunch(
     content::RenderProcessHost* host) {
   int id = host->GetID();
diff --git a/chrome/browser/chrome_content_browser_client.h b/chrome/browser/chrome_content_browser_client.h
index fdcae43..7dc8748 100644
--- a/chrome/browser/chrome_content_browser_client.h
+++ b/chrome/browser/chrome_content_browser_client.h
@@ -76,10 +76,6 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
       content::WebContents* opener_web_contents,
       content::BrowserPluginGuestDelegate** guest_delegate,
       scoped_ptr<base::DictionaryValue> extra_params) OVERRIDE;
-  virtual void GuestWebContentsAttached(
-      content::WebContents* guest_web_contents,
-      content::WebContents* embedder_web_contents,
-      const base::DictionaryValue& extra_params) OVERRIDE;
   virtual void RenderProcessWillLaunch(
       content::RenderProcessHost* host) OVERRIDE;
   virtual bool ShouldUseProcessPerSite(content::BrowserContext* browser_context,
diff --git a/chrome/browser/guest_view/guest_view.h b/chrome/browser/guest_view/guest_view.h
index b9fe688..a5fc994 100644
--- a/chrome/browser/guest_view/guest_view.h
+++ b/chrome/browser/guest_view/guest_view.h
@@ -55,12 +55,8 @@ class GuestView : public GuestViewBase {
   }
 
  protected:
-  GuestView(int guest_instance_id,
-            content::WebContents* guest_web_contents,
-            const std::string& embedder_extension_id)
-      : GuestViewBase(guest_instance_id,
-                      guest_web_contents,
-                      embedder_extension_id) {}
+  explicit GuestView(int guest_instance_id)
+      : GuestViewBase(guest_instance_id) {}
   virtual ~GuestView() {}
 
  private:
diff --git a/chrome/browser/guest_view/guest_view_base.cc b/chrome/browser/guest_view/guest_view_base.cc
index a130745..ce04a13 100644
--- a/chrome/browser/guest_view/guest_view_base.cc
+++ b/chrome/browser/guest_view/guest_view_base.cc
@@ -66,17 +66,25 @@ class GuestViewBase::EmbedderWebContentsObserver : public WebContentsObserver {
   DISALLOW_COPY_AND_ASSIGN(EmbedderWebContentsObserver);
 };
 
-GuestViewBase::GuestViewBase(int guest_instance_id,
-                             WebContents* guest_web_contents,
-                             const std::string& embedder_extension_id)
-    : WebContentsObserver(guest_web_contents),
-      embedder_web_contents_(NULL),
-      embedder_extension_id_(embedder_extension_id),
+GuestViewBase::GuestViewBase(int guest_instance_id)
+    : embedder_web_contents_(NULL),
       embedder_render_process_id_(0),
-      browser_context_(guest_web_contents->GetBrowserContext()),
+      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) {
+  if (initialized_)
+    return;
+  initialized_ = true;
+  browser_context_ = guest_web_contents->GetBrowserContext();
+  embedder_extension_id_ = embedder_extension_id;
+
+  WebContentsObserver::Observe(guest_web_contents);
   guest_web_contents->SetDelegate(this);
   webcontents_guestview_map.Get().insert(
       std::make_pair(guest_web_contents, this));
@@ -155,32 +163,6 @@ bool GuestViewBase::IsDragAndDropEnabled() const {
   return false;
 }
 
-void GuestViewBase::Attach(content::WebContents* embedder_web_contents,
-                           const base::DictionaryValue& args) {
-  embedder_web_contents_ = embedder_web_contents;
-  embedder_web_contents_observer_.reset(
-      new EmbedderWebContentsObserver(this));
-  embedder_render_process_id_ =
-      embedder_web_contents->GetRenderProcessHost()->GetID();
-  args.GetInteger(guestview::kParameterInstanceId, &view_instance_id_);
-  extra_params_.reset(args.DeepCopy());
-
-  // 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()));
-}
-
 void GuestViewBase::Destroy() {
   WillDestroy();
   if (!destruction_callback_.is_null())
@@ -188,6 +170,12 @@ void GuestViewBase::Destroy() {
   delete guest_web_contents();
 }
 
+void GuestViewBase::DidAttach() {
+  // Give the derived class an opportunity to perform some actions.
+  DidAttachToEmbedder();
+
+  SendQueuedEvents();
+}
 
 void GuestViewBase::SetOpener(GuestViewBase* guest) {
   if (guest && guest->IsViewType(GetViewType())) {
@@ -202,6 +190,19 @@ void GuestViewBase::RegisterDestructionCallback(
   destruction_callback_ = callback;
 }
 
+void GuestViewBase::WillAttach(content::WebContents* embedder_web_contents,
+                               const base::DictionaryValue& extra_params) {
+  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());
+
+  WillAttachToEmbedder();
+}
+
 void GuestViewBase::DidStopLoading(content::RenderViewHost* render_view_host) {
   if (!IsDragAndDropEnabled()) {
     const char script[] = "window.addEventListener('dragstart', function() { "
diff --git a/chrome/browser/guest_view/guest_view_base.h b/chrome/browser/guest_view/guest_view_base.h
index 95c19e1..1dff75c 100644
--- a/chrome/browser/guest_view/guest_view_base.h
+++ b/chrome/browser/guest_view/guest_view_base.h
@@ -64,10 +64,24 @@ class GuestViewBase : public content::BrowserPluginGuestDelegate,
 
   virtual const char* GetViewType() const = 0;
 
+  // This method is called after the guest has been attached to an embedder and
+  // suspended resource loads have been resumed.
+  //
+  // This method can be overriden by subclasses. This gives the derived class
+  // an opportunity to perform setup actions after attachment.
+  virtual void DidAttachToEmbedder() {}
+
   // This method can be overridden by subclasses. This method is called when
   // the initial set of frames within the page have completed loading.
   virtual void DidStopLoading() {}
 
+  // This method is called immediately before suspended resource loads have been
+  // resumed on attachment to an embedder.
+  //
+  // This method can be overriden by subclasses. This gives the derived class
+  // an opportunity to perform setup actions before attachment.
+  virtual void WillAttachToEmbedder() {}
+
   // This method is called when the guest WebContents is about to be destroyed.
   //
   // This method can be overridden by subclasses. This gives the derived class
@@ -93,15 +107,17 @@ class GuestViewBase : public content::BrowserPluginGuestDelegate,
   // this behavior to enable drag-and-drop.
   virtual bool IsDragAndDropEnabled() const;
 
+  // Once a guest WebContents is ready, this initiates the association of |this|
+  // GuestView with |guest_web_contents|.
+  void Init(content::WebContents* guest_web_contents,
+            const std::string& embedder_extension_id);
+
   bool IsViewType(const char* const view_type) const {
     return !strcmp(GetViewType(), view_type);
   }
 
   base::WeakPtr<GuestViewBase> AsWeakPtr();
 
-  virtual void Attach(content::WebContents* embedder_web_contents,
-                      const base::DictionaryValue& args);
-
   content::WebContents* embedder_web_contents() const {
     return embedder_web_contents_;
   }
@@ -148,13 +164,16 @@ class GuestViewBase : public content::BrowserPluginGuestDelegate,
 
   // BrowserPluginGuestDelegate implementation.
   virtual void Destroy() OVERRIDE FINAL;
+  virtual void DidAttach() OVERRIDE FINAL;
   virtual void RegisterDestructionCallback(
       const DestructionCallback& callback) OVERRIDE FINAL;
+  virtual void WillAttach(
+      content::WebContents* embedder_web_contents,
+      const base::DictionaryValue& extra_params) OVERRIDE FINAL;
 
  protected:
-  GuestViewBase(int guest_instance_id,
-                content::WebContents* guest_web_contents,
-                const std::string& embedder_extension_id);
+  explicit GuestViewBase(int guest_instance_id);
+
   virtual ~GuestViewBase();
 
   // Dispatches an event |event_name| to the embedder with the |event| fields.
@@ -177,9 +196,9 @@ class GuestViewBase : public content::BrowserPluginGuestDelegate,
       const blink::WebGestureEvent& event) OVERRIDE FINAL;
 
   content::WebContents* embedder_web_contents_;
-  const std::string embedder_extension_id_;
+  std::string embedder_extension_id_;
   int embedder_render_process_id_;
-  content::BrowserContext* const browser_context_;
+  content::BrowserContext* browser_context_;
   // |guest_instance_id_| is a profile-wide unique identifier for a guest
   // WebContents.
   const int guest_instance_id_;
@@ -187,6 +206,8 @@ class GuestViewBase : public content::BrowserPluginGuestDelegate,
   // embedder RenderViewHost for a particular <*view> instance.
   int view_instance_id_;
 
+  bool initialized_;
+
   // This is a queue of Events that are destined to be sent to the embedder once
   // the guest is attached to a particular embedder.
   std::deque<linked_ptr<Event> > pending_events_;
diff --git a/chrome/browser/guest_view/guest_view_manager.cc b/chrome/browser/guest_view/guest_view_manager.cc
index 39c0cea..981b4ac 100644
--- a/chrome/browser/guest_view/guest_view_manager.cc
+++ b/chrome/browser/guest_view/guest_view_manager.cc
@@ -8,6 +8,7 @@
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/guest_view/guest_view_base.h"
 #include "chrome/browser/guest_view/guest_view_constants.h"
+#include "chrome/browser/guest_view/guest_view_manager_factory.h"
 #include "chrome/browser/guest_view/web_view/web_view_guest.h"
 #include "chrome/browser/profiles/profile.h"
 #include "content/public/browser/browser_context.h"
@@ -24,6 +25,9 @@ using content::BrowserContext;
 using content::SiteInstance;
 using content::WebContents;
 
+// static
+GuestViewManagerFactory* GuestViewManager::factory_ = NULL;
+
 GuestViewManager::GuestViewManager(content::BrowserContext* context)
     : current_instance_id_(0), last_instance_id_removed_(0), context_(context) {
 }
@@ -37,7 +41,11 @@ GuestViewManager* GuestViewManager::FromBrowserContext(
       static_cast<GuestViewManager*>(context->GetUserData(
           guestview::kGuestViewManagerKeyName));
   if (!guest_manager) {
-    guest_manager = new GuestViewManager(context);
+    if (factory_) {
+      guest_manager = factory_->CreateGuestViewManager(context);
+    } else {
+      guest_manager = new GuestViewManager(context);
+    }
     context->SetUserData(guestview::kGuestViewManagerKeyName, guest_manager);
   }
   return guest_manager;
diff --git a/chrome/browser/guest_view/guest_view_manager.h b/chrome/browser/guest_view/guest_view_manager.h
index 5a904cb..55eab59 100644
--- a/chrome/browser/guest_view/guest_view_manager.h
+++ b/chrome/browser/guest_view/guest_view_manager.h
@@ -15,6 +15,7 @@
 #include "content/public/browser/web_contents.h"
 
 class GuestViewBase;
+class GuestViewManagerFactory;
 class GuestWebContentsObserver;
 class GURL;
 
@@ -22,6 +23,10 @@ namespace content {
 class BrowserContext;
 }  // namespace content
 
+namespace guestview {
+class TestGuestViewManager;
+}  // namespace guestview
+
 class GuestViewManager : public content::BrowserPluginGuestManager,
                          public base::SupportsUserData::Data {
  public:
@@ -30,6 +35,11 @@ class GuestViewManager : public content::BrowserPluginGuestManager,
 
   static GuestViewManager* FromBrowserContext(content::BrowserContext* context);
 
+  // Overrides factory for testing. Default (NULL) value indicates regular
+  // (non-test) environment.
+  static void set_factory_for_testing(GuestViewManagerFactory* factory) {
+    GuestViewManager::factory_ = factory;
+  }
   // Returns the guest WebContents associated with the given |guest_instance_id|
   // if the provided |embedder_render_process_id| is allowed to access it.
   // If the embedder is not allowed access, the embedder will be killed, and
@@ -52,14 +62,15 @@ class GuestViewManager : public content::BrowserPluginGuestManager,
   virtual bool ForEachGuest(content::WebContents* embedder_web_contents,
                             const GuestCallback& callback) OVERRIDE;
 
- private:
+ protected:
   friend class GuestViewBase;
   friend class GuestWebContentsObserver;
-  friend class TestGuestViewManager;
+  friend class guestview::TestGuestViewManager;
   FRIEND_TEST_ALL_PREFIXES(GuestViewManagerTest, AddRemove);
 
-  void AddGuest(int guest_instance_id,
-                content::WebContents* guest_web_contents);
+  // Can be overriden in tests.
+  virtual void AddGuest(int guest_instance_id,
+                        content::WebContents* guest_web_contents);
 
   void RemoveGuest(int guest_instance_id);
 
@@ -86,6 +97,9 @@ class GuestViewManager : public content::BrowserPluginGuestManager,
   static bool CanEmbedderAccessGuest(int embedder_render_process_id,
                                      GuestViewBase* guest);
 
+  // Static factory instance (always NULL for non-test).
+  static GuestViewManagerFactory* factory_;
+
   // Contains guests' WebContents, mapping from their instance ids.
   typedef std::map<int, content::WebContents*> GuestInstanceMap;
   GuestInstanceMap guest_web_contents_by_instance_id_;
diff --git a/chrome/browser/guest_view/guest_view_manager_factory.h b/chrome/browser/guest_view/guest_view_manager_factory.h
new file mode 100644
index 0000000..dbd6cb8
--- /dev/null
+++ b/chrome/browser/guest_view/guest_view_manager_factory.h
@@ -0,0 +1,18 @@
+// 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.
+
+#ifndef CHROME_BROWSER_GUEST_VIEW_GUEST_VIEW_MANAGER_FACTORY_H_
+#define CHROME_BROWSER_GUEST_VIEW_GUEST_VIEW_MANAGER_FACTORY_H_
+
+class GuestViewManagerFactory {
+ public:
+  virtual GuestViewManager* CreateGuestViewManager(
+      content::BrowserContext* context) = 0;
+
+ protected:
+  virtual ~GuestViewManagerFactory() {}
+};
+
+#endif  // CHROME_BROWSER_GUEST_VIEW_GUEST_VIEW_MANAGER_FACTORY_H_
+
diff --git a/chrome/browser/guest_view/guest_view_manager_unittest.cc b/chrome/browser/guest_view/guest_view_manager_unittest.cc
index 21ca720..05b53d5 100644
--- a/chrome/browser/guest_view/guest_view_manager_unittest.cc
+++ b/chrome/browser/guest_view/guest_view_manager_unittest.cc
@@ -12,22 +12,7 @@
 using content::WebContents;
 using content::WebContentsTester;
 
-class GuestViewManagerTest : public testing::Test {
- public:
-  GuestViewManagerTest() {}
-  virtual ~GuestViewManagerTest() {}
-
-  scoped_ptr<WebContents> CreateWebContents() {
-    return scoped_ptr<WebContents>(
-        WebContentsTester::CreateTestWebContents(&profile_, NULL));
-  }
-
- private:
-  content::TestBrowserThreadBundle thread_bundle_;
-  TestingProfile profile_;
-
-  DISALLOW_COPY_AND_ASSIGN(GuestViewManagerTest);
-};
+namespace guestview {
 
 // This class allows us to access some private variables in
 // GuestViewManager.
@@ -49,9 +34,33 @@ class TestGuestViewManager : public GuestViewManager {
   DISALLOW_COPY_AND_ASSIGN(TestGuestViewManager);
 };
 
+} // namespace guestview
+
+namespace {
+
+class GuestViewManagerTest : public testing::Test {
+ public:
+  GuestViewManagerTest() {}
+  virtual ~GuestViewManagerTest() {}
+
+  scoped_ptr<WebContents> CreateWebContents() {
+    return scoped_ptr<WebContents>(
+        WebContentsTester::CreateTestWebContents(&profile_, NULL));
+  }
+
+ private:
+  content::TestBrowserThreadBundle thread_bundle_;
+  TestingProfile profile_;
+
+  DISALLOW_COPY_AND_ASSIGN(GuestViewManagerTest);
+};
+
+}  // namespace
+
 TEST_F(GuestViewManagerTest, AddRemove) {
   TestingProfile profile;
-  scoped_ptr<TestGuestViewManager> manager(new TestGuestViewManager(&profile));
+  scoped_ptr<guestview::TestGuestViewManager> manager(
+      new guestview::TestGuestViewManager(&profile));
 
   scoped_ptr<WebContents> web_contents1(CreateWebContents());
   scoped_ptr<WebContents> web_contents2(CreateWebContents());
diff --git a/chrome/browser/guest_view/web_view/web_view_guest.cc b/chrome/browser/guest_view/web_view/web_view_guest.cc
index a2a86a4..2d6bb11 100644
--- a/chrome/browser/guest_view/web_view/web_view_guest.cc
+++ b/chrome/browser/guest_view/web_view/web_view_guest.cc
@@ -189,19 +189,17 @@ void AttachWebViewHelpers(WebContents* contents) {
 WebViewGuest::WebViewGuest(int guest_instance_id,
                            WebContents* guest_web_contents,
                            const std::string& embedder_extension_id)
-   :  GuestView<WebViewGuest>(guest_instance_id,
-                              guest_web_contents,
-                              embedder_extension_id),
+   :  GuestView<WebViewGuest>(guest_instance_id),
       script_executor_(new extensions::ScriptExecutor(guest_web_contents,
                                                       &script_observers_)),
       pending_context_menu_request_id_(0),
       next_permission_request_id_(0),
       is_overriding_user_agent_(false),
-      pending_reload_on_attachment_(false),
       main_frame_id_(0),
       chromevox_injected_(false),
       find_helper_(this),
       javascript_dialog_helper_(this) {
+  Init(guest_web_contents, embedder_extension_id);
   notification_registrar_.Add(
       this, content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
       content::Source<WebContents>(guest_web_contents));
@@ -387,28 +385,48 @@ scoped_ptr<base::ListValue> WebViewGuest::MenuModelToValue(
   return items.Pass();
 }
 
-void WebViewGuest::Attach(WebContents* embedder_web_contents,
-                          const base::DictionaryValue& args) {
+void WebViewGuest::DidAttachToEmbedder() {
   std::string name;
-  args.GetString(webview::kName, &name);
-  // If the guest window's name is empty, then the WebView tag's name is
-  // assigned. Otherwise, the guest window's name takes precedence over the
-  // WebView tag's name.
-  if (name_.empty())
-    name_ = name;
+  if (extra_params()->GetString(webview::kName, &name)) {
+    // If the guest window's name is empty, then the WebView tag's name is
+    // assigned. Otherwise, the guest window's name takes precedence over the
+    // WebView tag's name.
+    if (name_.empty())
+      name_ = name;
+  }
   ReportFrameNameChange(name_);
 
   std::string user_agent_override;
-  if (args.GetString(webview::kParameterUserAgentOverride,
-                     &user_agent_override)) {
+  if (extra_params()->GetString(webview::kParameterUserAgentOverride,
+                                &user_agent_override)) {
     SetUserAgentOverride(user_agent_override);
   } else {
     SetUserAgentOverride("");
   }
 
-  GuestViewBase::Attach(embedder_web_contents, args);
+  std::string src;
+  if (extra_params()->GetString("src", &src) && !src.empty())
+    NavigateGuest(src);
 
-  AddWebViewToExtensionRendererState();
+  if (GetOpener()) {
+    // We need to do a navigation here if the target URL has changed between
+    // the time the WebContents was created and the time it was attached.
+    // We also need to do an initial navigation if a RenderView was never
+    // created for the new window in cases where there is no referrer.
+    PendingWindowMap::iterator it =
+        GetOpener()->pending_new_windows_.find(this);
+    if (it != GetOpener()->pending_new_windows_.end()) {
+      const NewWindowInfo& new_window_info = it->second;
+      if (new_window_info.changed || !guest_web_contents()->HasOpener())
+        NavigateGuest(new_window_info.url.spec());
+    } else {
+      NOTREACHED();
+    }
+
+    // Once a new guest is attached to the DOM of the embedder page, then the
+    // lifetime of the new guest is no longer managed by the opener guest.
+    GetOpener()->pending_new_windows_.erase(this);
+  }
 }
 
 void WebViewGuest::DidStopLoading() {
@@ -475,36 +493,6 @@ void WebViewGuest::CloseContents(WebContents* source) {
   DispatchEvent(new GuestViewBase::Event(webview::kEventClose, args.Pass()));
 }
 
-void WebViewGuest::DidAttach(const base::DictionaryValue& extra_params) {
-  std::string src;
-  if (extra_params.GetString("src", &src) && !src.empty())
-    NavigateGuest(src);
-
-  if (GetOpener()) {
-    // We need to do a navigation here if the target URL has changed between
-    // the time the WebContents was created and the time it was attached.
-    // We also need to do an initial navigation if a RenderView was never
-    // created for the new window in cases where there is no referrer.
-    PendingWindowMap::iterator it =
-        GetOpener()->pending_new_windows_.find(this);
-    if (it != GetOpener()->pending_new_windows_.end()) {
-      const NewWindowInfo& new_window_info = it->second;
-      NavigateGuest(new_window_info.url.spec());
-    } else {
-      NOTREACHED();
-    }
-
-    // Once a new guest is attached to the DOM of the embedder page, then the
-    // lifetime of the new guest is no longer managed by the opener guest.
-    GetOpener()->pending_new_windows_.erase(this);
-  }
-
-  if (pending_reload_on_attachment_) {
-    pending_reload_on_attachment_ = false;
-    guest_web_contents()->GetController().Reload(false);
-  }
-}
-
 void WebViewGuest::FindReply(WebContents* source,
                              int request_id,
                              int number_of_matches,
@@ -845,6 +833,8 @@ WebViewGuest::SetPermissionResult WebViewGuest::SetPermission(
 
 void WebViewGuest::SetUserAgentOverride(
     const std::string& user_agent_override) {
+  if (!attached())
+    return;
   is_overriding_user_agent_ = !user_agent_override.empty();
   if (is_overriding_user_agent_) {
     content::RecordAction(UserMetricsAction("WebView.Guest.OverrideUA"));
@@ -1052,18 +1042,14 @@ void WebViewGuest::RenderProcessGone(base::TerminationStatus status) {
 }
 
 void WebViewGuest::UserAgentOverrideSet(const std::string& user_agent) {
+  if (!attached())
+    return;
   content::NavigationController& controller =
       guest_web_contents()->GetController();
   content::NavigationEntry* entry = controller.GetVisibleEntry();
   if (!entry)
     return;
   entry->SetIsOverridingUserAgent(!user_agent.empty());
-  if (!attached()) {
-    // We cannot reload now because all resource loads are suspended until
-    // attachment.
-    pending_reload_on_attachment_ = true;
-    return;
-  }
   guest_web_contents()->GetController().Reload(false);
 }
 
@@ -1206,6 +1192,13 @@ void WebViewGuest::RequestPointerLockPermission(
       false /* allowed_by_default */);
 }
 
+void WebViewGuest::WillAttachToEmbedder() {
+  // We must install the mapping from guests to WebViews prior to resuming
+  // suspended resource loads so that the WebRequest API will catch resource
+  // requests.
+  AddWebViewToExtensionRendererState();
+}
+
 content::JavaScriptDialogManager*
     WebViewGuest::GetJavaScriptDialogManager() {
   return &javascript_dialog_helper_;
@@ -1472,8 +1465,9 @@ content::WebContents* WebViewGuest::OpenURLFromTab(
         opener->pending_new_windows_.find(this);
     if (it == opener->pending_new_windows_.end())
       return NULL;
-    const NewWindowInfo& old_target_url = it->second;
-    NewWindowInfo new_window_info(params.url, old_target_url.name);
+    const NewWindowInfo& info = it->second;
+    NewWindowInfo new_window_info(params.url, info.name);
+    new_window_info.changed = new_window_info.url != info.url;
     it->second = new_window_info;
     return NULL;
   }
diff --git a/chrome/browser/guest_view/web_view/web_view_guest.h b/chrome/browser/guest_view/web_view/web_view_guest.h
index 32defb5..5baa498 100644
--- a/chrome/browser/guest_view/web_view/web_view_guest.h
+++ b/chrome/browser/guest_view/web_view/web_view_guest.h
@@ -88,12 +88,12 @@ class WebViewGuest : public GuestView<WebViewGuest>,
   void SetZoom(double zoom_factor);
 
   // GuestViewBase implementation.
-  virtual void Attach(content::WebContents* embedder_web_contents,
-                      const base::DictionaryValue& args) OVERRIDE;
+  virtual void DidAttachToEmbedder() OVERRIDE;
   virtual void DidStopLoading() OVERRIDE;
   virtual void EmbedderDestroyed() OVERRIDE;
   virtual void GuestDestroyed() OVERRIDE;
   virtual bool IsDragAndDropEnabled() const OVERRIDE;
+  virtual void WillAttachToEmbedder() OVERRIDE;
   virtual void WillDestroy() OVERRIDE;
 
   // WebContentsDelegate implementation.
@@ -151,14 +151,12 @@ class WebViewGuest : public GuestView<WebViewGuest>,
                                   content::WebContents* new_contents) OVERRIDE;
 
   // BrowserPluginGuestDelegate implementation.
-  virtual void DidAttach(const base::DictionaryValue& extra_params) OVERRIDE;
   virtual void SizeChanged(const gfx::Size& old_size, const gfx::Size& new_size)
       OVERRIDE;
   virtual void RequestPointerLockPermission(
       bool user_gesture,
       bool last_unlocked_by_target,
       const base::Callback<void(bool)>& callback) OVERRIDE;
-
   // NotificationObserver implementation.
   virtual void Observe(int type,
                        const content::NotificationSource& source,
@@ -455,10 +453,6 @@ class WebViewGuest : public GuestView<WebViewGuest>,
   // True if the user agent is overridden.
   bool is_overriding_user_agent_;
 
-  // Indicates that the page needs to be reloaded once it has been attached to
-  // an embedder.
-  bool pending_reload_on_attachment_;
-
   // Main frame ID of last committed page.
   int64 main_frame_id_;
 
@@ -497,9 +491,11 @@ class WebViewGuest : public GuestView<WebViewGuest>,
   struct NewWindowInfo {
     GURL url;
     std::string name;
+    bool changed;
     NewWindowInfo(const GURL& url, const std::string& name) :
         url(url),
-        name(name) {}
+        name(name),
+        changed(false) {}
   };
 
   typedef std::map<WebViewGuest*, NewWindowInfo> PendingWindowMap;
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 25c8fac..f871ffb 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -736,6 +736,7 @@
         'browser/guest_view/guest_view_constants.h',
         'browser/guest_view/guest_view_base.cc',
         'browser/guest_view/guest_view_base.h',
+        'browser/guest_view/guest_view_manager_factory.h',
         'browser/guest_view/guest_view_manager.cc',
         'browser/guest_view/guest_view_manager.h',
         'browser/guest_view/guest_view.h',
diff --git a/content/browser/browser_plugin/browser_plugin_embedder.cc b/content/browser/browser_plugin/browser_plugin_embedder.cc
index 42f8f95..30b4789 100644
--- a/content/browser/browser_plugin/browser_plugin_embedder.cc
+++ b/content/browser/browser_plugin/browser_plugin_embedder.cc
@@ -136,18 +136,8 @@ void BrowserPluginEmbedder::OnGuestCallback(
                 : NULL;
   }
 
-  if (guest) {
-    // There is an implicit order expectation here:
-    // 1. The content embedder is made aware of the attachment.
-    // 2. BrowserPluginGuest::Attach is called.
-    // 3. The content embedder issues queued events if any that happened
-    //    prior to attachment.
-    GetContentClient()->browser()->GuestWebContentsAttached(
-        guest->GetWebContents(),
-        GetWebContents(),
-        *extra_params);
+  if (guest)
     guest->Attach(GetWebContents(), params, *extra_params);
-  }
 }
 
 void BrowserPluginEmbedder::OnAttach(
diff --git a/content/browser/browser_plugin/browser_plugin_guest.cc b/content/browser/browser_plugin/browser_plugin_guest.cc
index 6f590e9..b3bd05a 100644
--- a/content/browser/browser_plugin/browser_plugin_guest.cc
+++ b/content/browser/browser_plugin/browser_plugin_guest.cc
@@ -226,11 +226,6 @@ void BrowserPluginGuest::Initialize(
 
   // Inform the embedder of the guest's attachment.
   SendMessageToEmbedder(new BrowserPluginMsg_Attach_ACK(instance_id_));
-
-  if (delegate_) {
-    delegate_->DidAttach(extra_params);
-    has_render_view_ = true;
-  }
 }
 
 BrowserPluginGuest::~BrowserPluginGuest() {
@@ -487,6 +482,9 @@ void BrowserPluginGuest::Attach(
   if (attached())
     return;
 
+  if (delegate_)
+    delegate_->WillAttach(embedder_web_contents, extra_params);
+
   // If a RenderView has already been created for this new window, then we need
   // to initialize the browser-side state now so that the RenderFrameHostManager
   // does not create a new RenderView on navigation.
@@ -502,6 +500,9 @@ void BrowserPluginGuest::Attach(
 
   SendQueuedMessages();
 
+  if (delegate_)
+    delegate_->DidAttach();
+
   RecordAction(base::UserMetricsAction("BrowserPlugin.Guest.Attached"));
 }
 
diff --git a/content/browser/browser_plugin/browser_plugin_guest.h b/content/browser/browser_plugin/browser_plugin_guest.h
index 9c82e15..d1bd4ba 100644
--- a/content/browser/browser_plugin/browser_plugin_guest.h
+++ b/content/browser/browser_plugin/browser_plugin_guest.h
@@ -363,7 +363,9 @@ class CONTENT_EXPORT BrowserPluginGuest : public WebContentsObserver {
 
   // Indicates that this BrowserPluginGuest has associated renderer-side state.
   // This is used to determine whether or not to create a new RenderView when
-  // this guest is attached.
+  // this guest is attached. A BrowserPluginGuest would have renderer-side state
+  // prior to attachment if it is created via a call to window.open and
+  // maintains a JavaScript reference to its opener.
   bool has_render_view_;
 
   // Last seen size of guest contents (by OnUpdateRect).
diff --git a/content/public/browser/browser_plugin_guest_delegate.h b/content/public/browser/browser_plugin_guest_delegate.h
index 24dc3a8..7abd737 100644
--- a/content/public/browser/browser_plugin_guest_delegate.h
+++ b/content/public/browser/browser_plugin_guest_delegate.h
@@ -27,8 +27,13 @@ class CONTENT_EXPORT BrowserPluginGuestDelegate {
  public:
   virtual ~BrowserPluginGuestDelegate() {}
 
+  // Notification that the embedder will begin attachment. This is called
+  // prior to resuming resource loads.
+  virtual void WillAttach(content::WebContents* embedder_web_contents,
+                          const base::DictionaryValue& extra_params) {}
+
   // Notification that the embedder has completed attachment.
-  virtual void DidAttach(const base::DictionaryValue& extra_params) {}
+  virtual void DidAttach() {}
 
   // Notifies that the content size of the guest has changed in autosize mode.
   virtual void SizeChanged(const gfx::Size& old_size,
diff --git a/content/public/browser/content_browser_client.h b/content/public/browser/content_browser_client.h
index 95a92fe..d355596 100644
--- a/content/public/browser/content_browser_client.h
+++ b/content/public/browser/content_browser_client.h
@@ -159,16 +159,6 @@ class CONTENT_EXPORT ContentBrowserClient {
       BrowserPluginGuestDelegate** guest_delegate,
       scoped_ptr<base::DictionaryValue> extra_params) {}
 
-  // Notifies that a guest WebContents has been attached to a BrowserPlugin.
-  // A guest is attached to a BrowserPlugin when the guest has acquired an
-  // embedder WebContents. This happens on initial navigation or when a new
-  // window is attached to a BrowserPlugin. |extra_params| are params sent
-  // from javascript.
-  virtual void GuestWebContentsAttached(
-      WebContents* guest_web_contents,
-      WebContents* embedder_web_contents,
-      const base::DictionaryValue& extra_params) {}
-
   // Notifies that a render process will be created. This is called before
   // the content layer adds its own BrowserMessageFilters, so that the
   // embedder's IPC filters have priority.