For JavaScript dialogs, remove As[Type] members, expand delegates to handle those cases.

BUG=84604
TEST=no visible change

Review URL: http://codereview.chromium.org/7283022

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

chrome/browser/extensions/extension_host.cc

@@ -478,14 +478,6 @@ gfx::NativeWindow ExtensionHost::GetDialogRootWindow() {
   return NULL;
 }
 
-TabContents* ExtensionHost::AsTabContents() {
-  return NULL;
-}
-
-ExtensionHost* ExtensionHost::AsExtensionHost() {
-  return this;
-}
-
 void ExtensionHost::OnDialogClosed(IPC::Message* reply_msg,
                                    bool success,
                                    const string16& user_input) {

chrome/browser/extensions/extension_host.h

@@ -192,8 +192,6 @@ class ExtensionHost : public RenderViewHostDelegate,
                               bool success,
                               const string16& user_input) OVERRIDE;
   virtual gfx::NativeWindow GetDialogRootWindow() OVERRIDE;
-  virtual TabContents* AsTabContents() OVERRIDE;
-  virtual ExtensionHost* AsExtensionHost() OVERRIDE;
 
  protected:
   // Internal functions used to support the CreateNewWidget() method. If a

chrome/browser/tab_contents/background_contents.cc

@@ -150,14 +150,6 @@ gfx::NativeWindow BackgroundContents::GetDialogRootWindow() {
   return NULL;
 }
 
-TabContents* BackgroundContents::AsTabContents() {
-  return NULL;
-}
-
-ExtensionHost* BackgroundContents::AsExtensionHost() {
-  return NULL;
-}
-
 void BackgroundContents::UpdateInspectorSetting(const std::string& key,
                                          const std::string& value) {
   Profile* profile = render_view_host_->process()->profile();

chrome/browser/tab_contents/background_contents.h

@@ -129,8 +129,6 @@ class BackgroundContents : public RenderViewHostDelegate,
                               bool success,
                               const string16& user_input) OVERRIDE;
   virtual gfx::NativeWindow GetDialogRootWindow() OVERRIDE;
-  virtual TabContents* AsTabContents() OVERRIDE;
-  virtual ExtensionHost* AsExtensionHost() OVERRIDE;
 
   virtual void UpdateInspectorSetting(const std::string& key,
                                       const std::string& value);

chrome/browser/ui/app_modal_dialogs/app_modal_dialog.cc

@@ -6,14 +6,15 @@
 
 #include "chrome/browser/ui/app_modal_dialogs/app_modal_dialog_queue.h"
 #include "chrome/browser/ui/app_modal_dialogs/native_app_modal_dialog.h"
+#include "content/browser/javascript_dialogs.h"
 #include "content/browser/tab_contents/tab_contents.h"
 #include "content/common/notification_service.h"
 #include "content/common/notification_type.h"
 
-AppModalDialog::AppModalDialog(TabContents* tab_contents,
+AppModalDialog::AppModalDialog(content::DialogDelegate* delegate,
                                const string16& title)
-    : skip_this_dialog_(false),
+    : valid_(true),
-      tab_contents_(tab_contents),
+      delegate_(delegate),
       native_dialog_(NULL),
       title_(title) {
 }
@@ -22,8 +23,8 @@ AppModalDialog::~AppModalDialog() {
 }
 
 void AppModalDialog::ShowModalDialog() {
-  if (tab_contents_)
+  if (delegate_)
-    tab_contents_->Activate();
+    delegate_->OnDialogShown();
 
   CreateAndShowDialog();
 
@@ -39,13 +40,21 @@ void AppModalDialog::CreateAndShowDialog() {
 }
 
 bool AppModalDialog::IsValid() {
-  return !skip_this_dialog_;
+  return valid_;
+}
+
+void AppModalDialog::Invalidate() {
+  valid_ = false;
 }
 
 bool AppModalDialog::IsJavaScriptModalDialog() {
   return false;
 }
 
+content::DialogDelegate* AppModalDialog::delegate() const {
+  return delegate_;
+}
+
 void AppModalDialog::ActivateModalDialog() {
   DCHECK(native_dialog_);
   native_dialog_->ActivateAppModalDialog();

chrome/browser/ui/app_modal_dialogs/app_modal_dialog.h

@@ -13,7 +13,10 @@
 #include "build/build_config.h"
 
 class NativeAppModalDialog;
-class TabContents;
+
+namespace content {
+class DialogDelegate;
+}
 
 // A controller+model base class for modal dialogs.
 class AppModalDialog {
@@ -21,7 +24,7 @@ class AppModalDialog {
   // A union of data necessary to determine the type of message box to
   // show. |tab_contents| parameter is optional, if provided that tab will be
   // activated before the modal dialog is displayed.
-  AppModalDialog(TabContents* tab_contents, const string16& title);
+  AppModalDialog(content::DialogDelegate* delegate, const string16& title);
   virtual ~AppModalDialog();
 
   // Called by the AppModalDialogQueue to show this dialog.
@@ -42,35 +45,41 @@ class AppModalDialog {
 
   NativeAppModalDialog* native_dialog() const { return native_dialog_; }
 
-  // Methods overridable by AppModalDialog subclasses:
-
   // Creates an implementation of NativeAppModalDialog and shows it.
   // When the native dialog is closed, the implementation of
   // NativeAppModalDialog should call OnAccept or OnCancel to notify the
   // renderer of the user's action. The NativeAppModalDialog is also
   // expected to delete the AppModalDialog associated with it.
-  virtual void CreateAndShowDialog();
+  void CreateAndShowDialog();
 
   // Returns true if the dialog is still valid. As dialogs are created they are
   // added to the AppModalDialogQueue. When the current modal dialog finishes
   // and it's time to show the next dialog in the queue IsValid is invoked.
   // If IsValid returns false the dialog is deleted and not shown.
-  virtual bool IsValid();
+  bool IsValid();
+
+  // Methods overridable by AppModalDialog subclasses:
+
+  // Invalidates the dialog, therefore causing it to not be shown when its turn
+  // to be shown comes around.
+  virtual void Invalidate();
 
   // Used only for testing. Returns whether the dialog is a JavaScript modal
   // dialog.
   virtual bool IsJavaScriptModalDialog();
 
+  virtual content::DialogDelegate* delegate() const;
+
  protected:
   // Overridden by subclasses to create the feature-specific native dialog box.
   virtual NativeAppModalDialog* CreateNativeDialog() = 0;
 
-  // True if the dialog should no longer be shown, e.g. because the underlying
+  // False if the dialog should no longer be shown, e.g. because the underlying
   // tab navigated away while the dialog was queued.
-  bool skip_this_dialog_;
+  bool valid_;
 
-  // Parent tab contents.
+  // The owner of this dialog.
-  TabContents* tab_contents_;
+  content::DialogDelegate* delegate_;
 
   // The toolkit-specific implementation of the app modal dialog box.
   NativeAppModalDialog* native_dialog_;

chrome/browser/ui/app_modal_dialogs/app_modal_dialog_queue.cc

@@ -11,7 +11,7 @@ void AppModalDialogQueue::AddDialog(AppModalDialog* dialog) {
     ShowModalDialog(dialog);
     return;
   }
-  app_modal_dialog_queue_.push(dialog);
+  app_modal_dialog_queue_.push_back(dialog);
 }
 
 void AppModalDialogQueue::ShowNextDialog() {
@@ -60,7 +60,7 @@ void AppModalDialogQueue::ShowModalDialog(AppModalDialog* dialog) {
 AppModalDialog* AppModalDialogQueue::GetNextDialog() {
   while (!app_modal_dialog_queue_.empty()) {
     AppModalDialog* dialog = app_modal_dialog_queue_.front();
-    app_modal_dialog_queue_.pop();
+    app_modal_dialog_queue_.pop_front();
     if (dialog->IsValid())
       return dialog;
     delete dialog;

chrome/browser/ui/app_modal_dialogs/app_modal_dialog_queue.h

@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -6,7 +6,7 @@
 #define CHROME_BROWSER_UI_APP_MODAL_DIALOGS_APP_MODAL_DIALOG_QUEUE_H_
 #pragma once
 
-#include <queue>
+#include <deque>
 
 #include "chrome/browser/ui/app_modal_dialogs/app_modal_dialog.h"
 
@@ -56,6 +56,16 @@ class AppModalDialogQueue {
     return active_dialog_;
   }
 
+  // Iterators to walk the queue.
+  typedef std::deque<AppModalDialog*>::iterator iterator;
+  iterator begin() {
+    return app_modal_dialog_queue_.begin();
+  }
+
+  iterator end() {
+    return app_modal_dialog_queue_.end();
+  }
+
  private:
   friend struct DefaultSingletonTraits<AppModalDialogQueue>;
 
@@ -73,7 +83,7 @@ class AppModalDialogQueue {
 
   // Contains all app modal dialogs which are waiting to be shown, with the
   // currently modal dialog at the front of the queue.
-  std::queue<AppModalDialog*> app_modal_dialog_queue_;
+  std::deque<AppModalDialog*> app_modal_dialog_queue_;
 
   // The currently active app-modal dialog box's delegate. NULL if there is no
   // active app-modal dialog box.

chrome/browser/ui/app_modal_dialogs/js_modal_dialog.cc

@@ -4,14 +4,8 @@
 
 #include "chrome/browser/ui/app_modal_dialogs/js_modal_dialog.h"
 
-#include "base/string_util.h"
-#include "base/utf_string_conversions.h"
 #include "chrome/browser/browser_shutdown.h"
-#include "chrome/browser/extensions/extension_host.h"
 #include "chrome/browser/ui/app_modal_dialogs/native_app_modal_dialog.h"
-#include "content/browser/tab_contents/tab_contents.h"
-#include "content/common/notification_service.h"
-#include "content/common/notification_type.h"
 #include "ui/base/text/text_elider.h"
 
 namespace {
@@ -62,10 +56,8 @@ JavaScriptAppModalDialog::JavaScriptAppModalDialog(
     bool display_suppress_checkbox,
     bool is_before_unload_dialog,
     IPC::Message* reply_msg)
-    : AppModalDialog(delegate->AsTabContents(), title),
+    : AppModalDialog(delegate, title),
-      delegate_(delegate),
       extra_data_(extra_data),
-      extension_host_(delegate->AsExtensionHost()),
       dialog_flags_(dialog_flags),
       display_suppress_checkbox_(display_suppress_checkbox),
       is_before_unload_dialog_(is_before_unload_dialog),
@@ -73,9 +65,6 @@ JavaScriptAppModalDialog::JavaScriptAppModalDialog(
       use_override_prompt_text_(false) {
   EnforceMaxTextSize(message_text, &message_text_);
   EnforceMaxPromptSize(default_prompt_text, &default_prompt_text_);
-
-  DCHECK((tab_contents_ != NULL) != (extension_host_ != NULL));
-  InitNotifications();
 }
 
 JavaScriptAppModalDialog::~JavaScriptAppModalDialog() {
@@ -91,42 +80,18 @@ bool JavaScriptAppModalDialog::IsJavaScriptModalDialog() {
   return true;
 }
 
-void JavaScriptAppModalDialog::Observe(NotificationType type,
+void JavaScriptAppModalDialog::Invalidate() {
-                                       const NotificationSource& source,
+  if (!valid_)
-                                       const NotificationDetails& details) {
-  if (skip_this_dialog_)
     return;
 
-  if (NotificationType::EXTENSION_HOST_DESTROYED == type &&
+  valid_ = false;
-      Details<ExtensionHost>(extension_host_) != details)
-    return;
-
-  // If we reach here, we know the notification is relevant to us, either
-  // because we're only observing applicable sources or because we passed the
-  // check above. Both of those indicate that we should ignore this dialog.
-  // Also clear the delegate, since it's now invalid.
-  skip_this_dialog_ = true;
   delegate_ = NULL;
   if (native_dialog_)
     CloseModalDialog();
 }
 
-void JavaScriptAppModalDialog::InitNotifications() {
+content::JavaScriptDialogDelegate* JavaScriptAppModalDialog::delegate() const {
-  // Make sure we get relevant navigation notifications so we know when our
+  return static_cast<content::JavaScriptDialogDelegate*>(delegate_);
-  // parent contents will disappear or navigate to a different page.
-  if (tab_contents_) {
-    registrar_.Add(this, NotificationType::NAV_ENTRY_COMMITTED,
-                   Source<NavigationController>(&tab_contents_->controller()));
-    registrar_.Add(this, NotificationType::TAB_CONTENTS_DESTROYED,
-                   Source<TabContents>(tab_contents_));
-  } else if (extension_host_) {
-    // EXTENSION_HOST_DESTROYED uses the Profile as its source, but we care
-    // about the ExtensionHost (which is passed in the details).
-    registrar_.Add(this, NotificationType::EXTENSION_HOST_DESTROYED,
-                   NotificationService::AllSources());
-  } else {
-    NOTREACHED();
-  }
 }
 
 void JavaScriptAppModalDialog::OnCancel(bool suppress_js_messages) {
@@ -170,15 +135,15 @@ void JavaScriptAppModalDialog::SetOverridePromptText(
 void JavaScriptAppModalDialog::NotifyDelegate(bool success,
                                               const string16& user_input,
                                               bool suppress_js_messages) {
-  if (skip_this_dialog_)
+  if (!valid_)
     return;
 
-  delegate_->OnDialogClosed(reply_msg_, success, user_input);
+  delegate()->OnDialogClosed(reply_msg_, success, user_input);
 
   extra_data_->last_javascript_message_dismissal_ = base::TimeTicks::Now();
   extra_data_->suppress_javascript_messages_ = suppress_js_messages;
 
   // On Views, we can end up coming through this code path twice :(.
   // See crbug.com/63732.
-  skip_this_dialog_ = true;
+  valid_ = false;
 }

chrome/browser/ui/app_modal_dialogs/js_modal_dialog.h

@@ -8,16 +8,15 @@
 
 #include <string>
 
+#include "base/compiler_specific.h"
 #include "base/time.h"
 #include "build/build_config.h"
 #include "chrome/browser/ui/app_modal_dialogs/app_modal_dialog.h"
 #include "content/browser/javascript_dialogs.h"
-#include "content/common/notification_observer.h"
-#include "content/common/notification_registrar.h"
 
-class ExtensionHost;
+namespace content {
-class NativeAppModalDialog;
+class JavaScriptDialogDelegate;
-class TabContents;
+}
 
 namespace IPC {
 class Message;
@@ -37,8 +36,7 @@ class ChromeJavaScriptDialogExtraData {
 
 // A controller + model class for JavaScript alert, confirm, prompt, and
 // onbeforeunload dialog boxes.
-class JavaScriptAppModalDialog : public AppModalDialog,
+class JavaScriptAppModalDialog : public AppModalDialog {
-                                 public NotificationObserver {
  public:
   JavaScriptAppModalDialog(content::JavaScriptDialogDelegate* delegate,
                            ChromeJavaScriptDialogExtraData* extra_data,
@@ -52,10 +50,10 @@ class JavaScriptAppModalDialog : public AppModalDialog,
   virtual ~JavaScriptAppModalDialog();
 
   // Overridden from AppModalDialog:
-  virtual NativeAppModalDialog* CreateNativeDialog();
+  virtual NativeAppModalDialog* CreateNativeDialog() OVERRIDE;
-  virtual bool IsJavaScriptModalDialog();
+  virtual bool IsJavaScriptModalDialog() OVERRIDE;
-
+  virtual void Invalidate() OVERRIDE;
-  content::JavaScriptDialogDelegate* delegate() const { return delegate_; }
+  virtual content::JavaScriptDialogDelegate* delegate() const OVERRIDE;
 
   // Callbacks from NativeDialog when the user accepts or cancels the dialog.
   void OnCancel(bool suppress_js_messages);
@@ -77,24 +75,10 @@ class JavaScriptAppModalDialog : public AppModalDialog,
   bool is_before_unload_dialog() const { return is_before_unload_dialog_; }
 
  private:
-  // Overridden from NotificationObserver:
-  virtual void Observe(NotificationType type,
-                       const NotificationSource& source,
-                       const NotificationDetails& details);
-
-  // Initializes for notifications to listen.
-  void InitNotifications();
-
   // Notifies the delegate with the result of the dialog.
   void NotifyDelegate(bool success, const string16& prompt_text,
                       bool suppress_js_messages);
 
-  NotificationRegistrar registrar_;
-
-  // An implementation of the client interface to provide supporting methods
-  // and receive results.
-  content::JavaScriptDialogDelegate* delegate_;
-
   // The extra Chrome-only data associated with the delegate_.
   ChromeJavaScriptDialogExtraData* extra_data_;
 

chrome/browser/ui/app_modal_dialogs/message_box_handler.cc

@@ -49,6 +49,8 @@ class ChromeJavaScriptDialogCreator : public content::JavaScriptDialogCreator {
                     const string16& title,
                     bool is_alert);
 
+  void CancelPendingDialogs(content::DialogDelegate* delegate);
+
   // Mapping between the JavaScriptDialogDelegates and their extra data. The key
   // is a void* because the pointer is just a cookie and is never dereferenced.
   typedef std::map<void*, ChromeJavaScriptDialogExtraData>
@@ -138,6 +140,7 @@ void ChromeJavaScriptDialogCreator::RunBeforeUnloadDialog(
 
 void ChromeJavaScriptDialogCreator::ResetJavaScriptState(
     content::JavaScriptDialogDelegate* delegate) {
+  CancelPendingDialogs(delegate);
   javascript_dialog_extra_data_.erase(delegate);
 }
 
@@ -168,6 +171,16 @@ string16 ChromeJavaScriptDialogCreator::GetTitle(TitleType title_type,
   return string16();
 }
 
+void ChromeJavaScriptDialogCreator::CancelPendingDialogs(
+    content::DialogDelegate* delegate) {
+  AppModalDialogQueue* queue = AppModalDialogQueue::GetInstance();
+  for (AppModalDialogQueue::iterator i = queue->begin();
+       i != queue->end(); ++i) {
+    if ((*i)->delegate() == delegate)
+      (*i)->Invalidate();
+  }
+}
+
 //------------------------------------------------------------------------------
 
 content::JavaScriptDialogCreator* GetJavaScriptDialogCreatorInstance() {

content/browser/javascript_dialogs.h

@@ -20,33 +20,31 @@ class Message;
 
 namespace content {
 
+class DialogDelegate {
+ public:
+  // Returns the root native window with which to associate the dialog.
+  virtual gfx::NativeWindow GetDialogRootWindow() = 0;
+
+  // Called right before the dialog is shown.
+  virtual void OnDialogShown() {}
+
+ protected:
+  virtual ~DialogDelegate() {}
+};
+
 // A class that invokes a JavaScript dialog must implement this interface to
 // allow the dialog implementation to get needed information and return results.
-class JavaScriptDialogDelegate {
+class JavaScriptDialogDelegate : public DialogDelegate {
  public:
   // This callback is invoked when the dialog is closed.
   virtual void OnDialogClosed(IPC::Message* reply_msg,
                               bool success,
                               const string16& user_input) = 0;
 
-  // Returns the root native window with which to associate the dialog.
-  virtual gfx::NativeWindow GetDialogRootWindow() = 0;
-
-  // Returns the TabContents implementing this delegate, or NULL if there is
-  // none. TODO(avi): This breaks encapsulation and in general sucks; figure out
-  // a better way of doing this.
-  virtual TabContents* AsTabContents() = 0;
-
-  // Returns the ExtensionHost implementing this delegate, or NULL if there is
-  // none. TODO(avi): This is even suckier than AsTabContents above as it breaks
-  // layering; figure out a better way of doing this. http://crbug.com/84604
-  virtual ExtensionHost* AsExtensionHost() = 0;
-
  protected:
   virtual ~JavaScriptDialogDelegate() {}
 };
 
-
 // An interface consisting of methods that can be called to produce JavaScript
 // dialogs.
 class JavaScriptDialogCreator {
@@ -73,7 +71,8 @@ class JavaScriptDialogCreator {
                                      const string16& message_text,
                                      IPC::Message* reply_message) = 0;
 
-  // Resets any saved JavaScript dialog state for the delegate.
+  // Cancels all pending dialogs and resets any saved JavaScript dialog state
+  // for the delegate.
   virtual void ResetJavaScriptState(JavaScriptDialogDelegate* delegate) = 0;
 
  protected:

content/browser/tab_contents/tab_contents.cc

@@ -1834,12 +1834,8 @@ gfx::NativeWindow TabContents::GetDialogRootWindow() {
   return view_->GetTopLevelNativeWindow();
 }
 
-TabContents* TabContents::AsTabContents() {
+void TabContents::OnDialogShown() {
-  return this;
+  Activate();
-}
-
-ExtensionHost* TabContents::AsExtensionHost() {
-  return NULL;
 }
 
 void TabContents::set_encoding(const std::string& encoding) {

content/browser/tab_contents/tab_contents.h

@@ -444,8 +444,7 @@ class TabContents : public PageNavigator,
                               bool success,
                               const string16& user_input) OVERRIDE;
   virtual gfx::NativeWindow GetDialogRootWindow() OVERRIDE;
-  virtual TabContents* AsTabContents() OVERRIDE;
+  virtual void OnDialogShown() OVERRIDE;
-  virtual ExtensionHost* AsExtensionHost() OVERRIDE;
 
   // The BookmarkDragDelegate is used to forward bookmark drag and drop events
   // to extensions.