Simplifying JS dialog interfaces.

-got rid of JavaScriptDialogDelegate since the embedder really only needs to know one callback to give the results to content
  -for telling it that the dialog is shown, it seemed that chrome's decision to activate the tab would be in chrome
  -used WebContents as the token for resetting dialogs if their tab went away
  -used currying to hide the implementation detail of reply_msg from the embedder
-made content handle NULL JS dialog creator interface instead of having a stub one in the public directory

BUG=098716
Review URL: https://chromiumcodereview.appspot.com/9159033

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

5 files changed, 40 insertions, 107 deletions

diff --git a/content/browser/javascript_dialogs.h b/content/browser/javascript_dialogs.h
index 31e13c42..74b02d9 100644
--- a/content/browser/javascript_dialogs.h
+++ b/content/browser/javascript_dialogs.h
@@ -6,48 +6,25 @@
 #define CONTENT_BROWSER_JAVASCRIPT_DIALOGS_H_
 #pragma once
 
+#include "base/callback.h"
 #include "base/string16.h"
-#include "content/browser/renderer_host/render_view_host.h"
 #include "content/common/content_export.h"
 #include "ui/base/javascript_message_type.h"
 #include "ui/gfx/native_widget_types.h"
 
-namespace IPC {
-class Message;
-}
-
 namespace content {
 
-class CONTENT_EXPORT DialogDelegate {
- public:
-  // Returns the root native window with which to associate the dialog.
-  virtual gfx::NativeWindow GetDialogRootWindow() const = 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 CONTENT_EXPORT JavaScriptDialogDelegate : public DialogDelegate {
- public:
-  // This callback is invoked when the dialog is closed.
-  virtual void OnDialogClosed(RenderViewHost* rvh,
-                              IPC::Message* reply_msg,
-                              bool success,
-                              const string16& user_input) = 0;
-
- protected:
-  virtual ~JavaScriptDialogDelegate() {}
-};
+class WebContents;
 
 // An interface consisting of methods that can be called to produce JavaScript
 // dialogs.
 class JavaScriptDialogCreator {
  public:
+
+  typedef base::Callback<void(bool /* success */,
+                              const string16& /* user_input */)>
+                                  DialogClosedCallback;
+
   enum TitleType {
     DIALOG_TITLE_NONE,
     DIALOG_TITLE_PLAIN_STRING,
@@ -57,23 +34,23 @@ class JavaScriptDialogCreator {
   // Displays a JavaScript dialog. |did_suppress_message| will not be nil; if
   // |true| is returned in it, the caller will handle faking the reply.
   virtual void RunJavaScriptDialog(
-      JavaScriptDialogDelegate* delegate,
+      WebContents* web_contents,
       TitleType title_type,
       const string16& title,
       ui::JavascriptMessageType javascript_message_type,
       const string16& message_text,
       const string16& default_prompt_text,
-      IPC::Message* reply_message,
+      const DialogClosedCallback& callback,
       bool* did_suppress_message) = 0;
 
   // Displays a dialog asking the user if they want to leave a page.
-  virtual void RunBeforeUnloadDialog(JavaScriptDialogDelegate* delegate,
+  virtual void RunBeforeUnloadDialog(WebContents* web_contents,
                                      const string16& message_text,
-                                     IPC::Message* reply_message) = 0;
+                                     const DialogClosedCallback& callback) = 0;
 
   // Cancels all pending dialogs and resets any saved JavaScript dialog state
-  // for the delegate.
-  virtual void ResetJavaScriptState(JavaScriptDialogDelegate* delegate) = 0;
+  // for the given WebContents.
+  virtual void ResetJavaScriptState(WebContents* web_contents) = 0;
 
  protected:
   virtual ~JavaScriptDialogCreator() {}
diff --git a/content/browser/tab_contents/tab_contents.cc b/content/browser/tab_contents/tab_contents.cc
index de5ee52..1f83d53 100644
--- a/content/browser/tab_contents/tab_contents.cc
+++ b/content/browser/tab_contents/tab_contents.cc
@@ -20,6 +20,7 @@
 #include "content/browser/host_zoom_map_impl.h"
 #include "content/browser/in_process_webkit/session_storage_namespace.h"
 #include "content/browser/intents/web_intents_dispatcher_impl.h"
+#include "content/browser/javascript_dialogs.h"
 #include "content/browser/load_from_memory_cache_details.h"
 #include "content/browser/load_notification_details.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
@@ -2032,7 +2033,8 @@ void TabContents::RunJavaScriptMessage(
       rvh->is_swapped_out() ||
       ShowingInterstitialPage() ||
       !delegate_ ||
-      delegate_->ShouldSuppressDialogs();
+      delegate_->ShouldSuppressDialogs() ||
+      !delegate_->GetJavaScriptDialogCreator();
 
   if (!suppress_this_message) {
     content::JavaScriptDialogCreator::TitleType title_type;
@@ -2049,14 +2051,12 @@ void TabContents::RunJavaScriptMessage(
     }
 
     dialog_creator_ = delegate_->GetJavaScriptDialogCreator();
-    dialog_creator_->RunJavaScriptDialog(this,
-                                         title_type,
-                                         title,
-                                         javascript_message_type,
-                                         message,
-                                         default_prompt,
-                                         reply_msg,
-                                         &suppress_this_message);
+    dialog_creator_->RunJavaScriptDialog(
+        this, title_type, title, javascript_message_type, message,
+        default_prompt,
+        base::Bind(&TabContents::OnDialogClosed, base::Unretained(this), rvh,
+                   reply_msg),
+        &suppress_this_message);
   }
 
   if (suppress_this_message) {
@@ -2077,7 +2077,8 @@ void TabContents::RunBeforeUnloadConfirm(RenderViewHost* rvh,
   bool suppress_this_message =
       rvh->is_swapped_out() ||
       !delegate_ ||
-      delegate_->ShouldSuppressDialogs();
+      delegate_->ShouldSuppressDialogs() ||
+      !delegate_->GetJavaScriptDialogCreator();
   if (suppress_this_message) {
     // The reply must be sent to the RVH that sent the request.
     rvh->JavaScriptDialogClosed(reply_msg, true, string16());
@@ -2086,9 +2087,10 @@ void TabContents::RunBeforeUnloadConfirm(RenderViewHost* rvh,
 
   is_showing_before_unload_dialog_ = true;
   dialog_creator_ = delegate_->GetJavaScriptDialogCreator();
-  dialog_creator_->RunBeforeUnloadDialog(this,
-                                         message,
-                                         reply_msg);
+  dialog_creator_->RunBeforeUnloadDialog(
+      this, message,
+      base::Bind(&TabContents::OnDialogClosed, base::Unretained(this), rvh,
+                 reply_msg));
 }
 
 WebPreferences TabContents::GetWebkitPrefs() {
@@ -2285,14 +2287,6 @@ void TabContents::OnDialogClosed(RenderViewHost* rvh,
   rvh->JavaScriptDialogClosed(reply_msg, success, user_input);
 }
 
-gfx::NativeWindow TabContents::GetDialogRootWindow() const {
-  return view_->GetTopLevelNativeWindow();
-}
-
-void TabContents::OnDialogShown() {
-  Activate();
-}
-
 void TabContents::SetEncoding(const std::string& encoding) {
   encoding_ = content::GetContentClient()->browser()->
       GetCanonicalEncodingNameByAliasName(encoding);
diff --git a/content/browser/tab_contents/tab_contents.h b/content/browser/tab_contents/tab_contents.h
index 263883c..2ec9aba 100644
--- a/content/browser/tab_contents/tab_contents.h
+++ b/content/browser/tab_contents/tab_contents.h
@@ -15,7 +15,6 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/observer_list.h"
 #include "base/property_bag.h"
-#include "content/browser/javascript_dialogs.h"
 #include "content/browser/renderer_host/java/java_bridge_dispatcher_host_manager.h"
 #include "content/browser/tab_contents/navigation_controller_impl.h"
 #include "content/browser/tab_contents/render_view_host_manager.h"
@@ -40,8 +39,9 @@ struct ViewHostMsg_DidFailProvisionalLoadWithError_Params;
 namespace content {
 class DownloadItem;
 class SiteInstance;
-class WebContentsObserver;
+class JavaScriptDialogCreator;
 class WebContentsDelegate;
+class WebContentsObserver;
 class WebContentsView;
 }
 
@@ -52,8 +52,7 @@ struct WebIntentData;
 class CONTENT_EXPORT TabContents
     : public NON_EXPORTED_BASE(content::WebContents),
       public content::RenderViewHostDelegate,
-      public RenderViewHostManager::Delegate,
-      public content::JavaScriptDialogDelegate {
+      public RenderViewHostManager::Delegate {
  public:
   // See WebContents::Create for a description of these parameters.
   TabContents(content::BrowserContext* browser_context,
@@ -335,14 +334,6 @@ class CONTENT_EXPORT TabContents
   virtual void SetFocusToLocationBar(bool select_all) OVERRIDE;
   virtual void CreateViewAndSetSizeForRVH(RenderViewHost* rvh) OVERRIDE;
 
-  // Overridden from JavaScriptDialogDelegate:
-  virtual void OnDialogClosed(RenderViewHost* rvh,
-                              IPC::Message* reply_msg,
-                              bool success,
-                              const string16& user_input) OVERRIDE;
-  virtual gfx::NativeWindow GetDialogRootWindow() const OVERRIDE;
-  virtual void OnDialogShown() OVERRIDE;
-
  protected:
   friend class content::WebContentsObserver;
 
@@ -379,6 +370,12 @@ class CONTENT_EXPORT TabContents
   // TODO(brettw) TestTabContents shouldn't exist!
   friend class TestTabContents;
 
+  // Callback function when showing JS dialogs.
+  void OnDialogClosed(RenderViewHost* rvh,
+                      IPC::Message* reply_msg,
+                      bool success,
+                      const string16& user_input);
+
   // Message handlers.
   void OnRegisterIntentService(const string16& action,
                                const string16& type,
diff --git a/content/public/browser/web_contents_delegate.cc b/content/public/browser/web_contents_delegate.cc
index 4edadc2..c2f0023 100644
--- a/content/public/browser/web_contents_delegate.cc
+++ b/content/public/browser/web_contents_delegate.cc
@@ -123,42 +123,8 @@ bool WebContentsDelegate::ShouldCreateWebContents(
   return true;
 }
 
-// A stubbed-out version of JavaScriptDialogCreator that doesn't do anything.
-class JavaScriptDialogCreatorStub : public JavaScriptDialogCreator {
- public:
-  static JavaScriptDialogCreatorStub* GetInstance() {
-    return Singleton<JavaScriptDialogCreatorStub>::get();
-  }
-
-  virtual void RunJavaScriptDialog(
-      JavaScriptDialogDelegate* delegate,
-      TitleType title_type,
-      const string16& title,
-      ui::JavascriptMessageType javascript_message_type,
-      const string16& message_text,
-      const string16& default_prompt_text,
-      IPC::Message* reply_message,
-      bool* did_suppress_message) OVERRIDE {
-    *did_suppress_message = true;
-  }
-
-  virtual void RunBeforeUnloadDialog(
-      JavaScriptDialogDelegate* delegate,
-      const string16& message_text,
-      IPC::Message* reply_message) OVERRIDE {
-    // TODO(creis): Pass in the RVH that sent the request.
-    delegate->OnDialogClosed(NULL, reply_message, true, string16());
-  }
-
-  virtual void ResetJavaScriptState(
-      JavaScriptDialogDelegate* delegate) OVERRIDE {
-  }
- private:
-  friend struct DefaultSingletonTraits<JavaScriptDialogCreatorStub>;
-};
-
 JavaScriptDialogCreator* WebContentsDelegate::GetJavaScriptDialogCreator() {
-  return JavaScriptDialogCreatorStub::GetInstance();
+  return NULL;
 }
 
 bool WebContentsDelegate::IsFullscreenForTab(const WebContents* tab) const {
diff --git a/content/public/browser/web_contents_delegate.h b/content/public/browser/web_contents_delegate.h
index d19ee91..9739429 100644
--- a/content/public/browser/web_contents_delegate.h
+++ b/content/public/browser/web_contents_delegate.h
@@ -312,9 +312,8 @@ class CONTENT_EXPORT WebContentsDelegate {
   // been committed.
   virtual void DidNavigateToPendingEntry(WebContents* tab) {}
 
-  // Returns a pointer to a service to create JavaScript dialogs. The default
-  // pointer returned is to a stub service that marks all dialogs as suppressed
-  // and displays nothing.
+  // Returns a pointer to a service to create JavaScript dialogs. May return
+  // NULL in which case dialogs aren't shown.
   virtual JavaScriptDialogCreator* GetJavaScriptDialogCreator();
 
   // Called when a file selection is to be done.