Revert 197194 "<webview>: Fix redirect from about:blank"

> <webview>: Fix redirect from about:blank
> 
> Some sites replace default link behavior with the following snippet:
> 
> function open(href) {
>   var w = window.open('', '_blank');
>   w.opener = null;
>   w.document.write('<META HTTP-EQUIV="refresh" content="0; url=' +
>                   href + '">');
>   w.document.close();
> }
> 
> The subsequent navigation fails because it's delegated to the browser
> process, and the New Window API is not expecting browser-side navigation
> prior to attachment.
> 
> This patch delays all pre-attachment navigations until attachment.
> 
> BUG=233475, 222618
> Test=WebViewInteractiveTest.NewWindow
> 
> Review URL: https://chromiumcodereview.appspot.com/13837005

The WebViewInteractiveTest.NewWindow is failing on Mac bots.
TBR=fsamuel@chromium.org

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

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

4 files changed, 28 insertions, 99 deletions

diff --git a/content/browser/browser_plugin/browser_plugin_guest.cc b/content/browser/browser_plugin/browser_plugin_guest.cc
index fc04b7d..d45a7f0 100644
--- a/content/browser/browser_plugin/browser_plugin_guest.cc
+++ b/content/browser/browser_plugin/browser_plugin_guest.cc
@@ -124,9 +124,7 @@ class BrowserPluginGuest::EmbedderRenderViewHostObserver
 
 BrowserPluginGuest::BrowserPluginGuest(
     int instance_id,
-    WebContentsImpl* web_contents,
-    BrowserPluginGuest* opener,
-    bool has_render_view)
+    WebContentsImpl* web_contents)
     : WebContentsObserver(web_contents),
       weak_ptr_factory_(this),
       embedder_web_contents_(NULL),
@@ -140,9 +138,8 @@ BrowserPluginGuest::BrowserPluginGuest(
       mouse_locked_(false),
       pending_lock_request_(false),
       embedder_visible_(true),
-      opener_(opener),
-      next_permission_request_id_(0),
-      has_render_view_(has_render_view) {
+      opener_(NULL),
+      next_permission_request_id_(0) {
   DCHECK(web_contents);
   web_contents->SetDelegate(this);
   GetWebContents()->GetBrowserPluginGuestManager()->AddGuest(instance_id_,
@@ -288,8 +285,6 @@ void BrowserPluginGuest::Initialize(
   if (!params.src.empty())
     OnNavigateGuest(instance_id_, params.src);
 
-  has_render_view_ = true;
-
   GetContentClient()->browser()->GuestWebContentsCreated(
       GetWebContents(), embedder_web_contents_);
 }
@@ -304,19 +299,7 @@ BrowserPluginGuest* BrowserPluginGuest::Create(
   RecordAction(UserMetricsAction("BrowserPlugin.Guest.Create"));
   if (factory_)
     return factory_->CreateBrowserPluginGuest(instance_id, web_contents);
-  return new BrowserPluginGuest(instance_id, web_contents, NULL, false);
-}
-
-// static
-BrowserPluginGuest* BrowserPluginGuest::CreateWithOpener(
-    int instance_id,
-    WebContentsImpl* web_contents,
-    BrowserPluginGuest* opener,
-    bool has_render_view) {
-  return new BrowserPluginGuest(instance_id,
-                                web_contents,
-                                opener,
-                                has_render_view);
+  return new BrowserPluginGuest(instance_id, web_contents);
 }
 
 RenderWidgetHostView* BrowserPluginGuest::GetEmbedderRenderWidgetHostView() {
@@ -402,29 +385,6 @@ bool BrowserPluginGuest::HandleContextMenu(
   return true;
 }
 
-WebContents* BrowserPluginGuest::OpenURLFromTab(WebContents* source,
-                                                const OpenURLParams& params) {
-  // If the guest wishes to navigate away prior to attachment then we save the
-  // navigation to perform upon attachment. Navigation initializes a lot of
-  // state that assumes an embedder exists, such as RenderWidgetHostViewGuest.
-  // Navigation also resumes resource loading which we don't want to allow
-  // until attachment.
-  if (!attached()) {
-    PendingWindowMap::iterator it = opener()->pending_new_windows_.find(this);
-    if (it == opener()->pending_new_windows_.end())
-      return NULL;
-    const TargetURL& old_target_url = it->second;
-    TargetURL new_target_url(params.url);
-    new_target_url.changed = new_target_url.url != old_target_url.url;
-    it->second = new_target_url;
-    return NULL;
-  }
-  // This can happen for cross-site redirects.
-  source->GetController().LoadURL(
-        params.url, params.referrer, params.transition, std::string());
-  return source;
-}
-
 void BrowserPluginGuest::WebContentsCreated(WebContents* source_contents,
                                             int64 source_frame_id,
                                             const string16& frame_name,
@@ -433,11 +393,12 @@ void BrowserPluginGuest::WebContentsCreated(WebContents* source_contents,
   WebContentsImpl* new_contents_impl =
       static_cast<WebContentsImpl*>(new_contents);
   BrowserPluginGuest* guest = new_contents_impl->GetBrowserPluginGuest();
+  guest->opener_ = this;
   guest->name_ = UTF16ToUTF8(frame_name);
   // Take ownership of the new guest until it is attached to the embedder's DOM
   // tree to avoid leaking a guest if this guest is destroyed before attaching
   // the new guest.
-  pending_new_windows_.insert(std::make_pair(guest, TargetURL(target_url)));
+  pending_new_windows_.insert(make_pair(guest, target_url.spec()));
 }
 
 void BrowserPluginGuest::RendererUnresponsive(WebContents* source) {
@@ -522,14 +483,14 @@ void BrowserPluginGuest::RequestNewWindowPermission(
   PendingWindowMap::iterator it = pending_new_windows_.find(guest);
   if (it == pending_new_windows_.end())
     return;
-  const TargetURL& target_url = it->second;
+  const std::string& target_url = it->second;
   base::DictionaryValue request_info;
   request_info.Set(browser_plugin::kInitialHeight,
                    base::Value::CreateIntegerValue(initial_bounds.height()));
   request_info.Set(browser_plugin::kInitialWidth,
                    base::Value::CreateIntegerValue(initial_bounds.width()));
   request_info.Set(browser_plugin::kTargetURL,
-                   base::Value::CreateStringValue(target_url.url.spec()));
+                   base::Value::CreateStringValue(target_url));
   request_info.Set(browser_plugin::kWindowID,
                    base::Value::CreateIntegerValue(guest->instance_id()));
   request_info.Set(browser_plugin::kWindowOpenDisposition,
@@ -783,31 +744,18 @@ bool BrowserPluginGuest::OnMessageReceived(const IPC::Message& message) {
 void BrowserPluginGuest::Attach(
     WebContentsImpl* embedder_web_contents,
     BrowserPluginHostMsg_CreateGuest_Params 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 RenderViewHostManager
-  // does not create a new RenderView on navigation.
-  if (has_render_view_) {
-    static_cast<RenderViewHostImpl*>(
-        GetWebContents()->GetRenderViewHost())->Init();
-
+  const std::string target_url = opener()->pending_new_windows_[this];
+  if (!GetWebContents()->opener()) {
+    // For guests that have a suppressed opener, we navigate now.
+    // Navigation triggers the creation of a RenderWidgetHostViewGuest so
+    // we don't need to create one manually.
+    params.src = target_url;
+  } else {
+    // Ensure that the newly attached guest gets a RenderWidgetHostViewGuest.
     WebContentsViewGuest* new_view =
         static_cast<WebContentsViewGuest*>(GetWebContents()->GetView());
     new_view->CreateViewForWidget(web_contents()->GetRenderViewHost());
   }
-
-  // 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 = opener()->pending_new_windows_.find(this);
-  if (it != opener()->pending_new_windows_.end()) {
-    const TargetURL& target_url = it->second;
-    if (target_url.changed || !has_render_view_)
-      params.src = it->second.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.
   opener()->pending_new_windows_.erase(this);
@@ -820,6 +768,14 @@ void BrowserPluginGuest::Attach(
 
   Initialize(embedder_web_contents, params);
 
+  // We initialize the RenderViewHost after a BrowserPlugin has been attached
+  // to it and is ready to receive pixels. Until a RenderViewHost is
+  // initialized, it will not allow any resize requests.
+  if (!GetWebContents()->GetRenderViewHost()->IsRenderViewLive()) {
+    static_cast<RenderViewHostImpl*>(
+        GetWebContents()->GetRenderViewHost())->Init();
+  }
+
   // Inform the embedder of the guest's information.
   // We pull the partition information from the site's URL, which is of the form
   // guest://site/{persist}?{partition_name}.
diff --git a/content/browser/browser_plugin/browser_plugin_guest.h b/content/browser/browser_plugin/browser_plugin_guest.h
index 7538550..c335c4b 100644
--- a/content/browser/browser_plugin/browser_plugin_guest.h
+++ b/content/browser/browser_plugin/browser_plugin_guest.h
@@ -90,12 +90,6 @@ class CONTENT_EXPORT BrowserPluginGuest : public NotificationObserver,
       int instance_id,
       WebContentsImpl* web_contents);
 
-  static BrowserPluginGuest* CreateWithOpener(
-      int instance_id,
-      WebContentsImpl* web_contents,
-      BrowserPluginGuest* opener,
-      bool has_render_view);
-
   // Destroys the guest WebContents and all its associated state, including
   // this BrowserPluginGuest, and its new unattached windows.
   void Destroy();
@@ -188,8 +182,6 @@ class CONTENT_EXPORT BrowserPluginGuest : public NotificationObserver,
                            const std::string& request_method,
                            const base::Callback<void(bool)>& callback) OVERRIDE;
   virtual bool HandleContextMenu(const ContextMenuParams& params) OVERRIDE;
-  virtual WebContents* OpenURLFromTab(WebContents* source,
-                                      const OpenURLParams& params) OVERRIDE;
   virtual void WebContentsCreated(WebContents* source_contents,
                                   int64 source_frame_id,
                                   const string16& frame_name,
@@ -261,9 +253,7 @@ class CONTENT_EXPORT BrowserPluginGuest : public NotificationObserver,
   friend class TestBrowserPluginGuest;
 
   BrowserPluginGuest(int instance_id,
-                     WebContentsImpl* web_contents,
-                     BrowserPluginGuest* opener,
-                     bool has_render_view);
+                     WebContentsImpl* web_contents);
 
   // Destroy unattached new windows that have been opened by this
   // BrowserPluginGuest.
@@ -448,18 +438,7 @@ class CONTENT_EXPORT BrowserPluginGuest : public NotificationObserver,
   gfx::Size max_auto_size_;
   gfx::Size min_auto_size_;
 
-  // Tracks the target URL of the new window and whether or not it has changed
-  // since the WebContents has been created and before the new window has been
-  // attached to a BrowserPlugin. Once the first navigation commits, we no
-  // longer track this URL.
-  struct TargetURL {
-    bool changed;
-    GURL url;
-    explicit TargetURL(const GURL& url) :
-        changed(false),
-        url(url) {}
-  };
-  typedef std::map<BrowserPluginGuest*, TargetURL> PendingWindowMap;
+  typedef std::map<BrowserPluginGuest*, std::string> PendingWindowMap;
   PendingWindowMap pending_new_windows_;
   BrowserPluginGuest* opener_;
   // A counter to generate a unique request id for a permission request.
@@ -477,11 +456,6 @@ class CONTENT_EXPORT BrowserPluginGuest : public NotificationObserver,
   typedef std::map<int, base::Callback<void(bool)> > DownloadRequestMap;
   DownloadRequestMap download_request_callback_map_;
 
-  // 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.
-  bool has_render_view_;
-
   DISALLOW_COPY_AND_ASSIGN(BrowserPluginGuest);
 };
 
diff --git a/content/browser/browser_plugin/test_browser_plugin_guest.cc b/content/browser/browser_plugin/test_browser_plugin_guest.cc
index 083ea5a..5b1ae21 100644
--- a/content/browser/browser_plugin/test_browser_plugin_guest.cc
+++ b/content/browser/browser_plugin/test_browser_plugin_guest.cc
@@ -17,7 +17,7 @@ class BrowserPluginGuest;
 TestBrowserPluginGuest::TestBrowserPluginGuest(
     int instance_id,
     WebContentsImpl* web_contents)
-    : BrowserPluginGuest(instance_id, web_contents, NULL, false),
+    : BrowserPluginGuest(instance_id, web_contents),
       update_rect_count_(0),
       damage_buffer_call_count_(0),
       exit_observed_(false),
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 0e678f6..7ba9ba4 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -1382,8 +1382,7 @@ void WebContentsImpl::CreateNewWindow(
     WebContentsImpl* new_contents_impl =
         static_cast<WebContentsImpl*>(new_contents);
     new_contents_impl->browser_plugin_guest_.reset(
-        BrowserPluginGuest::CreateWithOpener(instance_id, new_contents_impl,
-            GetBrowserPluginGuest(), !!new_contents_impl->opener()));
+        BrowserPluginGuest::Create(instance_id, new_contents_impl));
   }
   new_contents->Init(create_params);