Remove NOTIFICATION_WEB_CONTENTS_VISIBILITY_CHANGED from web contents modal dialogs.

BUG=170921
TEST=everything still works

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

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

3 files changed, 18 insertions, 48 deletions

diff --git a/components/web_modal/web_contents_modal_dialog_manager.cc b/components/web_modal/web_contents_modal_dialog_manager.cc
index 4d8aca7..f454445 100644
--- a/components/web_modal/web_contents_modal_dialog_manager.cc
+++ b/components/web_modal/web_contents_modal_dialog_manager.cc
@@ -7,9 +7,6 @@
 #include "components/web_modal/web_contents_modal_dialog_manager_delegate.h"
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/navigation_entry.h"
-#include "content/public/browser/notification_details.h"
-#include "content/public/browser/notification_source.h"
-#include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_view.h"
@@ -84,21 +81,6 @@ void WebContentsModalDialogManager::WillClose(
   BlockWebContentsInteraction(!child_dialogs_.empty());
 }
 
-void WebContentsModalDialogManager::Observe(
-    int type,
-    const content::NotificationSource& source,
-    const content::NotificationDetails& details) {
-  DCHECK(type == content::NOTIFICATION_WEB_CONTENTS_VISIBILITY_CHANGED);
-  if (child_dialogs_.empty())
-    return;
-
-  bool visible = *content::Details<bool>(details).ptr();
-  if (visible)
-    native_manager_->ShowDialog(child_dialogs_.front().dialog);
-  else
-    native_manager_->HideDialog(child_dialogs_.front().dialog);
-}
-
 WebContentsModalDialogManager::WebContentsModalDialogManager(
     content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents),
@@ -106,9 +88,6 @@ WebContentsModalDialogManager::WebContentsModalDialogManager(
       native_manager_(CreateNativeManager(this)),
       closing_all_dialogs_(false) {
   DCHECK(native_manager_);
-  registrar_.Add(this,
-                 content::NOTIFICATION_WEB_CONTENTS_VISIBILITY_CHANGED,
-                 content::Source<content::WebContents>(web_contents));
 }
 
 WebContentsModalDialogManager::DialogState::DialogState(
@@ -169,6 +148,16 @@ void WebContentsModalDialogManager::DidGetIgnoredUIEvent() {
     native_manager_->FocusDialog(child_dialogs_.front().dialog);
 }
 
+void WebContentsModalDialogManager::WasShown() {
+  if (!child_dialogs_.empty())
+    native_manager_->ShowDialog(child_dialogs_.front().dialog);
+}
+
+void WebContentsModalDialogManager::WasHidden() {
+  if (!child_dialogs_.empty())
+    native_manager_->HideDialog(child_dialogs_.front().dialog);
+}
+
 void WebContentsModalDialogManager::WebContentsDestroyed(WebContents* tab) {
   // First cleanly close all child dialogs.
   // TODO(mpcomplete): handle case if MaybeCloseChildWindows() already asked
diff --git a/components/web_modal/web_contents_modal_dialog_manager.h b/components/web_modal/web_contents_modal_dialog_manager.h
index 58d5520..253de18 100644
--- a/components/web_modal/web_contents_modal_dialog_manager.h
+++ b/components/web_modal/web_contents_modal_dialog_manager.h
@@ -9,8 +9,6 @@
 
 #include "base/memory/scoped_ptr.h"
 #include "components/web_modal/native_web_contents_modal_dialog_manager.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_user_data.h"
 #include "ui/gfx/native_widget_types.h"
@@ -23,8 +21,7 @@ class WebContentsModalDialogManagerDelegate;
 class WebContentsModalDialogManager
     : public NativeWebContentsModalDialogManagerDelegate,
       public content::WebContentsObserver,
-      public content::WebContentsUserData<WebContentsModalDialogManager>,
-      public content::NotificationObserver {
+      public content::WebContentsUserData<WebContentsModalDialogManager> {
  public:
   virtual ~WebContentsModalDialogManager();
 
@@ -54,11 +51,6 @@ class WebContentsModalDialogManager
   // Called when a WebContentsModalDialogs we own is about to be closed.
   virtual void WillClose(NativeWebContentsModalDialog dialog) OVERRIDE;
 
-  // content::NotificationObserver overrides
-  virtual void Observe(int type,
-                       const content::NotificationSource& source,
-                       const content::NotificationDetails& details) OVERRIDE;
-
   // For testing.
   class TestApi {
    public:
@@ -70,6 +62,8 @@ class WebContentsModalDialogManager
     void ResetNativeManager(NativeWebContentsModalDialogManager* delegate) {
       manager_->native_manager_.reset(delegate);
     }
+    void WebContentsWasShown() { manager_->WasShown(); }
+    void WebContentsWasHidden() { manager_->WasHidden(); }
 
    private:
     WebContentsModalDialogManager* manager_;
@@ -107,6 +101,8 @@ class WebContentsModalDialogManager
       const content::LoadCommittedDetails& details,
       const content::FrameNavigateParams& params) OVERRIDE;
   virtual void DidGetIgnoredUIEvent() OVERRIDE;
+  virtual void WasShown() OVERRIDE;
+  virtual void WasHidden() OVERRIDE;
   virtual void WebContentsDestroyed(content::WebContents* tab) OVERRIDE;
   virtual void DidAttachInterstitialPage() OVERRIDE;
 
@@ -122,9 +118,6 @@ class WebContentsModalDialogManager
   // True while closing the dialogs on WebContents close.
   bool closing_all_dialogs_;
 
-  // A scoped container for notification registries.
-  content::NotificationRegistrar registrar_;
-
   DISALLOW_COPY_AND_ASSIGN(WebContentsModalDialogManager);
 };
 
diff --git a/components/web_modal/web_contents_modal_dialog_manager_unittest.cc b/components/web_modal/web_contents_modal_dialog_manager_unittest.cc
index bb86691..2da9124 100644
--- a/components/web_modal/web_contents_modal_dialog_manager_unittest.cc
+++ b/components/web_modal/web_contents_modal_dialog_manager_unittest.cc
@@ -8,10 +8,6 @@
 #include "components/web_modal/native_web_contents_modal_dialog_manager.h"
 #include "components/web_modal/web_contents_modal_dialog_manager.h"
 #include "components/web_modal/web_contents_modal_dialog_manager_delegate.h"
-#include "content/public/browser/notification_details.h"
-#include "content/public/browser/notification_service.h"
-#include "content/public/browser/notification_source.h"
-#include "content/public/browser/notification_types.h"
 #include "content/public/test/test_renderer_host.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -211,11 +207,9 @@ TEST_F(WebContentsModalDialogManagerTest, ShowDialogs) {
             native_manager->GetDialogState(dialog3));
 }
 
-// Test that the dialog is shown/hidden on
-// NOTIFICATION_WEB_CONTENTS_VISIBILITY_CHANGED.
+// Test that the dialog is shown/hidden when the WebContents is shown/hidden.
 TEST_F(WebContentsModalDialogManagerTest, VisibilityObservation) {
   const NativeWebContentsModalDialog dialog1 = MakeFakeDialog();
-  bool web_contents_visible = true;
 
   manager->ShowDialog(dialog1);
 
@@ -224,20 +218,14 @@ TEST_F(WebContentsModalDialogManagerTest, VisibilityObservation) {
   EXPECT_EQ(TestNativeWebContentsModalDialogManager::SHOWN,
             native_manager->GetDialogState(dialog1));
 
-  web_contents_visible = false;
-  manager->Observe(content::NOTIFICATION_WEB_CONTENTS_VISIBILITY_CHANGED,
-                   content::NotificationService::AllSources(),
-                   content::Details<bool>(&web_contents_visible));
+  test_api->WebContentsWasHidden();
 
   EXPECT_TRUE(manager->IsDialogActive());
   EXPECT_TRUE(delegate->web_contents_blocked());
   EXPECT_EQ(TestNativeWebContentsModalDialogManager::HIDDEN,
             native_manager->GetDialogState(dialog1));
 
-  web_contents_visible = true;
-  manager->Observe(content::NOTIFICATION_WEB_CONTENTS_VISIBILITY_CHANGED,
-                   content::NotificationService::AllSources(),
-                   content::Details<bool>(&web_contents_visible));
+  test_api->WebContentsWasShown();
 
   EXPECT_TRUE(manager->IsDialogActive());
   EXPECT_TRUE(delegate->web_contents_blocked());