Fix window.open()/window.close() regression by disabling window.close() until a message comes back

from the Browser thread saying that it's OK to allow javascript close calls.

ISSUE=http://crbug.com/4007
Review URL: http://codereview.chromium.org/12691

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

10 files changed, 60 insertions, 7 deletions

diff --git a/chrome/browser/render_view_host.cc b/chrome/browser/render_view_host.cc
index cb31cda..38bbe9e 100644
--- a/chrome/browser/render_view_host.cc
+++ b/chrome/browser/render_view_host.cc
@@ -1033,6 +1033,10 @@ void RenderViewHost::DisassociateFromPopupCount() {
   Send(new ViewMsg_DisassociateFromPopupCount(routing_id_));
 }
 
+void RenderViewHost::PopupNotificationVisibilityChanged(bool visible) {
+  Send(new ViewMsg_PopupNotificationVisiblityChanged(routing_id_, visible));
+}
+
 void RenderViewHost::OnMsgGoToEntryAtOffset(int offset) {
   delegate_->GoToEntryAtOffset(offset);
 }
diff --git a/chrome/browser/render_view_host.h b/chrome/browser/render_view_host.h
index 3638f9b..b7d1ae2 100644
--- a/chrome/browser/render_view_host.h
+++ b/chrome/browser/render_view_host.h
@@ -392,6 +392,10 @@ class RenderViewHost : public RenderWidgetHost {
   // as a popup.
   void DisassociateFromPopupCount();
 
+  // Notifies the Renderer that we've either displayed or hidden the popup
+  // notification.
+  void PopupNotificationVisibilityChanged(bool visible);
+
   // Called by the AutofillManager when the list of suggestions is ready.
   void AutofillSuggestionsReturned(const std::vector<std::wstring>& suggestions,
                                    int64 node_id,
diff --git a/chrome/browser/tab_contents.cc b/chrome/browser/tab_contents.cc
index ebd8f2b..ffd17cb 100644
--- a/chrome/browser/tab_contents.cc
+++ b/chrome/browser/tab_contents.cc
@@ -292,6 +292,8 @@ void TabContents::AddNewContents(TabContents* new_contents,
 
     delegate_->AddNewContents(this, new_contents, disposition, initial_pos,
                               user_gesture);
+
+    PopupNotificationVisibilityChanged(ShowingBlockedPopupNotification());
   }
 }
 
@@ -311,6 +313,7 @@ void TabContents::AddConstrainedPopup(TabContents* new_contents,
   }
 
   blocked_popups_->AddTabContents(new_contents, initial_pos);
+  PopupNotificationVisibilityChanged(ShowingBlockedPopupNotification());
 }
 
 void TabContents::CloseAllSuppressedPopups() {
diff --git a/chrome/browser/tab_contents.h b/chrome/browser/tab_contents.h
index dad5f63..4ff6a44 100644
--- a/chrome/browser/tab_contents.h
+++ b/chrome/browser/tab_contents.h
@@ -337,6 +337,9 @@ class TabContents : public PageNavigator,
   // of unwanted popups.
   void CloseAllSuppressedPopups();
 
+  // Called when the blocked popup notification is shown or hidden.
+  virtual void PopupNotificationVisibilityChanged(bool visible) { }
+
   // Views and focus -----------------------------------------------------------
 
   // Returns the actual window that is focused when this TabContents is shown.
diff --git a/chrome/browser/views/blocked_popup_container.cc b/chrome/browser/views/blocked_popup_container.cc
index 22993ac..ef66737 100644
--- a/chrome/browser/views/blocked_popup_container.cc
+++ b/chrome/browser/views/blocked_popup_container.cc
@@ -306,6 +306,7 @@ std::wstring BlockedPopupContainer::GetDisplayStringForItem(int index) {
 
 void BlockedPopupContainer::CloseAllPopups() {
   CloseEachTabContents();
+  owner_->PopupNotificationVisibilityChanged(false);
   container_view_->UpdatePopupCountLabel();
   HideSelf();
 }
diff --git a/chrome/browser/web_contents.cc b/chrome/browser/web_contents.cc
index 0c0b34d..4afc290 100644
--- a/chrome/browser/web_contents.cc
+++ b/chrome/browser/web_contents.cc
@@ -486,6 +486,10 @@ void WebContents::SetDownloadShelfVisible(bool visible) {
   }
 }
 
+void WebContents::PopupNotificationVisibilityChanged(bool visible) {
+  render_view_host()->PopupNotificationVisibilityChanged(visible);
+}
+
 // Stupid view pass-throughs
 void WebContents::CreateView() {
   view_->CreateView();
@@ -847,11 +851,8 @@ void WebContents::UpdateThumbnail(const GURL& url,
 }
 
 void WebContents::Close(RenderViewHost* rvh) {
-  // Ignore this if it comes from a RenderViewHost that we aren't showing, and
-  // refuse to allow javascript to close this window if we have a blocked popup
-  // notification.
-  if (delegate() && rvh == render_view_host() &&
-      !ShowingBlockedPopupNotification())
+  // Ignore this if it comes from a RenderViewHost that we aren't showing.
+  if (delegate() && rvh == render_view_host())
     delegate()->CloseContents(this);
 }
 
diff --git a/chrome/browser/web_contents.h b/chrome/browser/web_contents.h
index 75c5c689..efe7a10 100644
--- a/chrome/browser/web_contents.h
+++ b/chrome/browser/web_contents.h
@@ -98,6 +98,7 @@ class WebContents : public TabContents,
   virtual void ShowContents();
   virtual void HideContents();
   virtual void SetDownloadShelfVisible(bool visible);
+  virtual void PopupNotificationVisibilityChanged(bool visible);
 
   // Retarded pass-throughs to the view.
   // TODO(brettw) fix this, tab contents shouldn't have these methods, probably
diff --git a/chrome/common/render_messages_internal.h b/chrome/common/render_messages_internal.h
index 0d4076c..8cc598a 100644
--- a/chrome/common/render_messages_internal.h
+++ b/chrome/common/render_messages_internal.h
@@ -471,6 +471,12 @@ IPC_BEGIN_MESSAGES(View, 1)
                       std::vector<std::wstring> /* suggestions */,
                       int /* index of default suggestion */)
 
+  // Sent by the Browser process to alert a window about whether a blocked
+  // popup notification is visible. The renderer assumes every new window is a
+  // blocked popup until notified otherwise.
+  IPC_MESSAGE_ROUTED1(ViewMsg_PopupNotificationVisiblityChanged,
+                      bool /* Whether it is visible */)
+
 IPC_END_MESSAGES(View)
 
 
diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index 48a0e36..e571ace 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -162,7 +162,8 @@ RenderView::RenderView()
     decrement_shared_popup_at_destruction_(false),
     greasemonkey_enabled_(false),
     waiting_for_create_window_ack_(false),
-    form_field_autofill_request_id_(0) {
+    form_field_autofill_request_id_(0),
+    popup_notification_visible_(false)  {
   resource_dispatcher_ = new ResourceDispatcher(this);
 #ifdef CHROME_PERSONALIZATION
   personalization_ = Personalization::CreateRendererPersonalization();
@@ -387,6 +388,9 @@ void RenderView::OnMessageReceived(const IPC::Message& message) {
                         OnDisassociateFromPopupCount)
     IPC_MESSAGE_HANDLER(ViewMsg_AutofillSuggestions,
                         OnReceivedAutofillSuggestions)
+    IPC_MESSAGE_HANDLER(ViewMsg_PopupNotificationVisiblityChanged,
+                        OnPopupNotificationVisiblityChanged)
+
     // Have the super handle all other messages.
     IPC_MESSAGE_UNHANDLED(RenderWidget::OnMessageReceived(message))
   IPC_END_MESSAGE_MAP()
@@ -1692,6 +1696,10 @@ void RenderView::OnReceivedAutofillSuggestions(
                                         default_suggestion_index);
 }
 
+void RenderView::OnPopupNotificationVisiblityChanged(bool visible) {
+  popup_notification_visible_ = visible;
+}
+
 void RenderView::ShowModalHTMLDialog(const GURL& url, int width, int height,
                                      const std::string& json_arguments,
                                      std::string* json_retval) {
@@ -1765,6 +1773,10 @@ WebView* RenderView::CreateWebView(WebView* webview, bool user_gesture) {
   if (shared_popup_counter_->data > kMaximumNumberOfUnacknowledgedPopups)
     return NULL;
 
+  // This window can't be closed from a window.close() call until we receive a
+  // message from the Browser process explicitly allowing it.
+  popup_notification_visible_ = true;
+
   int32 routing_id = MSG_ROUTING_NONE;
   HANDLE modal_dialog_event = NULL;
   bool result = RenderThread::current()->Send(
@@ -1896,6 +1908,11 @@ void RenderView::Show(WebWidget* webwidget, WindowOpenDisposition disposition) {
       WasOpenedByUserGestureHelper()));
 }
 
+void RenderView::CloseWidgetSoon(WebWidget* webwidget) {
+  if (popup_notification_visible_ == false)
+    RenderWidget::CloseWidgetSoon(webwidget);
+}
+
 void RenderView::RunModal(WebWidget* webwidget) {
   DCHECK(did_show_) << "should already have shown the view";
 
diff --git a/chrome/renderer/render_view.h b/chrome/renderer/render_view.h
index 7ee0f8f..0d6cda8 100644
--- a/chrome/renderer/render_view.h
+++ b/chrome/renderer/render_view.h
@@ -273,6 +273,7 @@ class RenderView : public RenderWidget, public WebViewDelegate,
   // WebWidgetDelegate
   // Most methods are handled by RenderWidget.
   virtual void Show(WebWidget* webwidget, WindowOpenDisposition disposition);
+  virtual void CloseWidgetSoon(WebWidget* webwidget);
   virtual void RunModal(WebWidget* webwidget);
 
   // Do not delete directly.  This class is reference counted.
@@ -471,6 +472,9 @@ class RenderView : public RenderWidget, public WebViewDelegate,
       const std::vector<std::wstring>& suggestions,
       int default_suggestions_index);
 
+  // Message that the popup notification has been shown or hidden.
+  void OnPopupNotificationVisiblityChanged(bool visible);
+
 #ifdef CHROME_PERSONALIZATION
   void OnPersonalizationEvent(std::string event_name, std::string event_args);
 #endif
@@ -695,7 +699,16 @@ class RenderView : public RenderWidget, public WebViewDelegate,
   // WebCore (via the window.history.go API).  We only have one such navigation
   // pending at a time.
   scoped_refptr<WebHistoryItem> history_navigation_item_;
-  
+
+  // We need to prevent windows from closing themselves with a window.close()
+  // call while a blocked popup notification is being displayed. We cannot
+  // synchronously querry the Browser process. We cannot wait for the Browser
+  // process to send a message to us saying that a blocked popup notification
+  // is being displayed. We instead assume that when we create a window off
+  // this RenderView, that it is going to be blocked until we get a message
+  // from the Browser process telling us otherwise.
+  bool popup_notification_visible_;
+
   DISALLOW_COPY_AND_ASSIGN(RenderView);
 };