Remove chrome::NOTIFICATION_WEB_DIALOG_SHOWN.

This is a preliminary work for moving web dialog stuffs to ui/web_dialogs.

BUG=125841
TEST=rowser_tests --gtest_filter="CertificateViewerUITest*.*" passes


Review URL: https://chromiumcodereview.appspot.com/10332231

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

chrome/browser/ui/webui/certificate_viewer_webui.cc

@@ -18,6 +18,7 @@
 #include "chrome/browser/ui/constrained_window.h"
 #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
 #include "chrome/browser/ui/webui/constrained_web_dialog_ui.h"
+#include "chrome/browser/ui/webui/web_dialog_observer.h"
 #include "chrome/common/net/x509_certificate_model.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/web_contents.h"
@@ -38,16 +39,19 @@ const int kDefaultHeight = 600;
 // Shows a certificate using the WebUI certificate viewer.
 void ShowCertificateViewer(gfx::NativeWindow parent,
                            net::X509Certificate* cert) {
-  CertificateViewerDialog::ShowDialog(parent, cert);
+  CertificateViewerDialog* dialog = new CertificateViewerDialog(cert);
+  dialog->Show(parent);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // CertificateViewerDialog
 
-void CertificateViewerDialog::ShowDialog(gfx::NativeWindow parent,
-                                         net::X509Certificate* cert) {
-  CertificateViewerDialog* dialog = new CertificateViewerDialog(cert);
-  dialog->Show(parent);
+void CertificateViewerDialog::AddObserver(WebDialogObserver* observer) {
+  observers_.AddObserver(observer);
+}
+
+void CertificateViewerDialog::RemoveObserver(WebDialogObserver* observer) {
+  observers_.RemoveObserver(observer);
 }
 
 CertificateViewerDialog::CertificateViewerDialog(net::X509Certificate* cert)
@@ -199,6 +203,14 @@ std::string CertificateViewerDialog::GetDialogArgs() const {
   return data;
 }
 
+void CertificateViewerDialog::OnDialogShown(
+    content::WebUI* webui,
+    content::RenderViewHost* render_view_host) {
+  FOR_EACH_OBSERVER(WebDialogObserver,
+                    observers_,
+                    OnDialogShown(webui, render_view_host));
+}
+
 void CertificateViewerDialog::OnDialogClosed(const std::string& json_retval) {
   delete this;
 }

chrome/browser/ui/webui/certificate_viewer_webui.h

@@ -10,12 +10,15 @@
 #include <vector>
 
 #include "base/compiler_specific.h"
+#include "base/observer_list.h"
 #include "base/values.h"
 #include "chrome/browser/ui/webui/web_dialog_delegate.h"
 #include "content/public/browser/web_ui_message_handler.h"
 #include "net/base/x509_certificate.h"
 #include "ui/gfx/native_widget_types.h"
 
+class WebDialogObserver;
+
 // Displays the native or WebUI certificate viewer dialog for the given
 // certificate.
 void ShowCertificateViewer(gfx::NativeWindow parent,
@@ -27,20 +30,22 @@ void ShowCertificateViewer(gfx::NativeWindow parent,
 // or "View" from the Certificate Manager.
 class CertificateViewerDialog : private WebDialogDelegate {
  public:
-  // Shows the certificate viewer dialog for the passed in certificate.
-  static void ShowDialog(gfx::NativeWindow parent,
-                         net::X509Certificate* cert);
-  virtual ~CertificateViewerDialog();
-
- private:
   // Construct a certificate viewer for the passed in certificate. A reference
   // to the certificate pointer is added for the lifetime of the certificate
   // viewer.
   explicit CertificateViewerDialog(net::X509Certificate* cert);
+  virtual ~CertificateViewerDialog();
 
   // Show the dialog using the given parent window.
   void Show(gfx::NativeWindow parent);
 
+  // Add WebDialogObserver for this dialog.
+  void AddObserver(WebDialogObserver* observer);
+
+  // Remove WebDialogObserver for this dialog.
+  void RemoveObserver(WebDialogObserver* observer);
+
+ private:
   // Overridden from WebDialogDelegate:
   virtual ui::ModalType GetDialogModalType() const OVERRIDE;
   virtual string16 GetDialogTitle() const OVERRIDE;
@@ -49,6 +54,9 @@ class CertificateViewerDialog : private WebDialogDelegate {
       std::vector<content::WebUIMessageHandler*>* handlers) const OVERRIDE;
   virtual void GetDialogSize(gfx::Size* size) const OVERRIDE;
   virtual std::string GetDialogArgs() const OVERRIDE;
+  virtual void OnDialogShown(
+      content::WebUI* webui,
+      content::RenderViewHost* render_view_host) OVERRIDE;
   virtual void OnDialogClosed(const std::string& json_retval) OVERRIDE;
   virtual void OnCloseContents(
       content::WebContents* source, bool* out_close_dialog) OVERRIDE;
@@ -63,6 +71,8 @@ class CertificateViewerDialog : private WebDialogDelegate {
   // The title of the certificate viewer dialog, Certificate Viewer: CN.
   string16 title_;
 
+  ObserverList<WebDialogObserver> observers_;
+
   DISALLOW_COPY_AND_ASSIGN(CertificateViewerDialog);
 };
 

chrome/browser/ui/webui/constrained_web_dialog_ui.cc

@@ -14,7 +14,6 @@
 #include "base/values.h"
 #include "chrome/browser/ui/webui/web_dialog_delegate.h"
 #include "chrome/browser/ui/webui/web_dialog_ui.h"
-#include "chrome/common/chrome_notification_types.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
@@ -55,10 +54,7 @@ void ConstrainedWebDialogUI::RenderViewCreated(
       base::Bind(&ConstrainedWebDialogUI::OnDialogCloseMessage,
                  base::Unretained(this)));
 
-  content::NotificationService::current()->Notify(
-      chrome::NOTIFICATION_WEB_DIALOG_SHOWN,
-      content::Source<content::WebUI>(web_ui()),
-      content::Details<RenderViewHost>(render_view_host));
+  dialog_delegate->OnDialogShown(web_ui(), render_view_host);
 }
 
 void ConstrainedWebDialogUI::OnDialogCloseMessage(const ListValue* args) {

chrome/browser/ui/webui/web_dialog_delegate.h

@@ -16,8 +16,10 @@
 class GURL;
 
 namespace content {
-class WebUIMessageHandler;
+class RenderViewHost;
 class WebContents;
+class WebUI;
+class WebUIMessageHandler;
 struct ContextMenuParams;
 struct OpenURLParams;
 }
@@ -63,6 +65,12 @@ class WebDialogDelegate {
   // changed.
   virtual void OnLoadingStateChanged(content::WebContents* source) {}
 
+  // A callback to notify the delegate that a web dialog has been shown.
+  // |webui| is the WebUI with which the dialog is associated.
+  // |render_view_host| is the RenderViewHost for the shown dialog.
+  virtual void OnDialogShown(content::WebUI* webui,
+                             content::RenderViewHost* render_view_host) {}
+
   // A callback to notify the delegate that the dialog closed.
   // IMPORTANT: Implementations should delete |this| here (unless they've
   // arranged for the delegate to be deleted in some other way, e.g. by

chrome/browser/ui/webui/web_dialog_observer.h

+// Copyright (c) 2012 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_UI_WEBUI_WEB_DIALOG_OBSERVER_H_
+#define CHROME_BROWSER_UI_WEBUI_WEB_DIALOG_OBSERVER_H_
+#pragma once
+
+namespace content {
+class RenderViewHost;
+class WebUI;
+}
+
+// Implement this class to receive notifications.
+class WebDialogObserver {
+ public:
+  // Invoked when a web dialog has been shown.
+  // |webui| is the WebUI with which the dialog is associated.
+  // |render_view_host| is the RenderViewHost for the shown dialog.
+  virtual void OnDialogShown(content::WebUI* webui,
+                             content::RenderViewHost* render_view_host) = 0;
+
+ protected:
+  virtual ~WebDialogObserver() {}
+};
+
+#endif  // CHROME_BROWSER_UI_WEBUI_WEB_DIALOG_OBSERVER_H_

chrome/browser/ui/webui/web_dialog_ui.cc

@@ -10,7 +10,6 @@
 #include "base/property_bag.h"
 #include "base/values.h"
 #include "chrome/browser/ui/webui/web_dialog_delegate.h"
-#include "chrome/common/chrome_notification_types.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
@@ -74,10 +73,8 @@ void WebDialogUI::RenderViewCreated(RenderViewHost* render_view_host) {
     web_ui()->AddMessageHandler(*it);
   }
 
-  content::NotificationService::current()->Notify(
-      chrome::NOTIFICATION_WEB_DIALOG_SHOWN,
-      content::Source<content::WebUI>(web_ui()),
-      content::Details<RenderViewHost>(render_view_host));
+  if (delegate)
+    (*delegate)->OnDialogShown(web_ui(), render_view_host);
 }
 
 void WebDialogUI::OnDialogClosed(const ListValue* args) {

chrome/chrome_browser.gypi

@@ -3943,6 +3943,7 @@
         'browser/ui/webui/web_dialog_controller.cc',
         'browser/ui/webui/web_dialog_controller.h',
         'browser/ui/webui/web_dialog_delegate.h',
+        'browser/ui/webui/web_dialog_observer.h',
         'browser/ui/webui/web_dialog_web_contents_delegate.cc',
         'browser/ui/webui/web_dialog_web_contents_delegate.h',
         'browser/ui/webui/web_dialog_ui.cc',

chrome/common/chrome_notification_types.h

@@ -79,11 +79,6 @@ enum NotificationType {
   // traversal. The source is the browser, there are no details.
   NOTIFICATION_FOCUS_RETURNED_TO_BROWSER,
 
-  // Sent after an WebDialog dialog has been shown. The source is the
-  // dialog. The details is a Details<RenderViewHost> with a pointer to the RVH
-  // for the shown dialog.
-  NOTIFICATION_WEB_DIALOG_SHOWN,
-
   // A new tab is created from an existing tab to serve as a target of a
   // navigation that is about to happen. The source will be a Source<Profile>
   // corresponding to the profile in which the new tab will live.  Details in

chrome/test/base/test_web_dialog_observer.cc

@@ -4,7 +4,6 @@
 
 #include "chrome/test/base/test_web_dialog_observer.h"
 
-#include "chrome/common/chrome_notification_types.h"
 #include "content/test/js_injection_ready_observer.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/notification_service.h"
@@ -23,8 +22,6 @@ TestWebDialogObserver::TestWebDialogObserver(
       web_ui_(NULL),
       done_(false),
       running_(false) {
-  registrar_.Add(this, chrome::NOTIFICATION_WEB_DIALOG_SHOWN,
-                 content::NotificationService::AllSources());
 }
 
 TestWebDialogObserver::~TestWebDialogObserver() {
@@ -35,24 +32,6 @@ void TestWebDialogObserver::Observe(
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   switch (type) {
-    case chrome::NOTIFICATION_WEB_DIALOG_SHOWN:
-      if (js_injection_ready_observer_) {
-        js_injection_ready_observer_->OnJsInjectionReady(
-            content::Details<content::RenderViewHost>(details).ptr());
-      }
-      web_ui_ = content::Source<content::WebUI>(source).ptr();
-      registrar_.Remove(this, chrome::NOTIFICATION_WEB_DIALOG_SHOWN,
-                        content::NotificationService::AllSources());
-      // Wait for navigation on the new WebUI instance to complete. This depends
-      // on receiving the notification of the WebDialog being shown before the
-      // NavigationController finishes loading. The WebDialog notification is
-      // issued from web_dialog_ui.cc on RenderView creation which results from
-      // the call to render_manager_.Navigate in the method
-      // WebContents::NavigateToEntry. The new RenderView is later told to
-      // navigate in this method, ensuring that this is not a race condition.
-      registrar_.Add(this, content::NOTIFICATION_LOAD_STOP,
-                     content::Source<NavigationController>(
-                         &web_ui_->GetWebContents()->GetController()));
       break;
     case content::NOTIFICATION_LOAD_STOP:
       DCHECK(web_ui_);
@@ -71,6 +50,25 @@ void TestWebDialogObserver::Observe(
   };
 }
 
+void TestWebDialogObserver::OnDialogShown(
+      content::WebUI* webui,
+      content::RenderViewHost* render_view_host) {
+  if (js_injection_ready_observer_) {
+    js_injection_ready_observer_->OnJsInjectionReady(render_view_host);
+  }
+  web_ui_ = webui;
+  // Wait for navigation on the new WebUI instance to complete. This depends
+  // on receiving the notification of the WebDialog being shown before the
+  // NavigationController finishes loading. The WebDialog notification is
+  // issued from web_dialog_ui.cc on RenderView creation which results from
+  // the call to render_manager_.Navigate in the method
+  // WebContents::NavigateToEntry. The new RenderView is later told to
+  // navigate in this method, ensuring that this is not a race condition.
+  registrar_.Add(this, content::NOTIFICATION_LOAD_STOP,
+                 content::Source<NavigationController>(
+                     &web_ui_->GetWebContents()->GetController()));
+}
+
 content::WebUI* TestWebDialogObserver::GetWebUI() {
   if (!done_) {
     EXPECT_FALSE(running_);

chrome/test/base/test_web_dialog_observer.h

@@ -8,19 +8,22 @@
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
+#include "chrome/browser/ui/webui/web_dialog_observer.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 
 class JsInjectionReadyObserver;
 
 namespace content {
+class RenderViewHost;
 class WebUI;
 }
 
 // For browser_tests, which run on the UI thread, run a second message
 // MessageLoop to detect WebDialog creation and quit when the constructed
 // WebUI instance is captured and ready.
-class TestWebDialogObserver : public content::NotificationObserver {
+class TestWebDialogObserver : public content::NotificationObserver,
+                              public WebDialogObserver {
  public:
   // Create and register a new TestWebDialogObserver. If
   // |js_injection_ready_observer| is non-NULL, notify it as soon as the RVH is
@@ -29,6 +32,11 @@ class TestWebDialogObserver : public content::NotificationObserver {
       JsInjectionReadyObserver* js_injection_ready_observer);
   virtual ~TestWebDialogObserver();
 
+  // Overridden from WebDialogObserver:
+  virtual void OnDialogShown(
+      content::WebUI* webui,
+      content::RenderViewHost* render_view_host) OVERRIDE;
+
   // Waits for an WebDialog to be created. The WebUI instance is captured
   // and the method returns it when the navigation on the dialog is complete.
   content::WebUI* GetWebUI();

chrome/test/data/webui/certificate_viewer_ui_test-inl.h

@@ -5,6 +5,7 @@
 #include "chrome/browser/certificate_viewer.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/webui/certificate_viewer_webui.h"
 #include "chrome/browser/ui/webui/web_ui_browsertest.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/test_web_dialog_observer.h"
@@ -34,7 +35,11 @@ void CertificateViewerUITest::ShowCertificateViewer() {
   ASSERT_TRUE(browser()->window());
 
   TestWebDialogObserver dialog_observer(this);
-  ::ShowCertificateViewer(browser()->window()->GetNativeHandle(), google_cert);
+  CertificateViewerDialog* dialog = new CertificateViewerDialog(
+      google_cert);
+  dialog->AddObserver(&dialog_observer);
+  dialog->Show(browser()->window()->GetNativeHandle());
+  dialog->RemoveObserver(&dialog_observer);
   content::WebUI* webui = dialog_observer.GetWebUI();
   webui->GetWebContents()->GetRenderViewHost()->SetWebUIProperty(
       "expectedUrl", chrome::kChromeUICertificateViewerURL);