Change HungPluginTabHelper to listen for infobar changes through Observer style.

chrome/browser/ui/hung_plugin_tab_helper.cc

Index: chrome/browser/ui/hung_plugin_tab_helper.cc
diff --git a/chrome/browser/ui/hung_plugin_tab_helper.cc b/chrome/browser/ui/hung_plugin_tab_helper.cc
index 374932018845c63b1e30d2a4e0e28338e40730ba..a522d65e28ea8aef2751cc941e4bf7e4d5d17957 100644
--- a/chrome/browser/ui/hung_plugin_tab_helper.cc
+++ b/chrome/browser/ui/hung_plugin_tab_helper.cc
@@ -10,7 +10,6 @@
 #include "base/process/process.h"
 #include "base/rand_util.h"
 #include "build/build_config.h"
-#include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/common/chrome_version_info.h"
 #include "components/infobars/core/confirm_infobar_delegate.h"
@@ -18,8 +17,6 @@
 #include "content/public/browser/browser_child_process_host_iterator.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/child_process_data.h"
-#include "content/public/browser/notification_details.h"
-#include "content/public/browser/notification_service.h"
 #include "content/public/browser/plugin_service.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/common/process_type.h"
@@ -261,12 +258,12 @@ HungPluginTabHelper::PluginState::~PluginState() {
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(HungPluginTabHelper);
 
 HungPluginTabHelper::HungPluginTabHelper(content::WebContents* contents)
-    : content::WebContentsObserver(contents) {
-  registrar_.Add(this, chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_REMOVED,
-                 content::NotificationService::AllSources());
+    : content::WebContentsObserver(contents),
+      number_of_infobars_(0) {
 }
 
 HungPluginTabHelper::~HungPluginTabHelper() {
+  DCHECK_EQ(0, number_of_infobars_);

[Peter Kasting, 2014/05/31 03:44:59]

One of the following is true:

(1) This DCHECK doesn't always hold.
(2) We don't actually need to observe the "manager shutting down" notification,
because we will have gone to zero infobars and removed ourselves as an observer
before then.

If (2) were not true, then we could get "manager shutting down" while some
infobars are still open.  But then we wouldn't reset |number_of_infobars_| to
zero, so we'd fail this DCHECK.

Based on the code in InfoBarManager::ShutDown(), I would think (2) is true,
unless somehow ShowBar() can ever be called in response to the manager closing
all existing infobars.

 }
 
 void HungPluginTabHelper::PluginCrashed(const base::FilePath& plugin_path,
@@ -321,13 +318,16 @@ void HungPluginTabHelper::PluginHungStatusChanged(
   ShowBar(plugin_child_id, state.get());
 }
 
-void HungPluginTabHelper::Observe(
-    int type,
-    const content::NotificationSource& source,
-    const content::NotificationDetails& details) {
-  DCHECK_EQ(chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_REMOVED, type);
-  infobars::InfoBar* infobar =
-      content::Details<infobars::InfoBar::RemovedDetails>(details)->first;
+void HungPluginTabHelper::OnInfoBarRemoved(infobars::InfoBar* infobar,
+                                           bool animate) {
+  number_of_infobars_--;

[Peter Kasting, 2014/05/31 03:44:59]

Nit: Prefer predecrement to postdecrement

[tfarina, 2014/06/03 03:47:13]

Done.

+
+  if (number_of_infobars_ == 0) {
+    InfoBarService* infobar_service =
+        InfoBarService::FromWebContents(web_contents());

[Peter Kasting, 2014/05/31 03:44:59]

Nit: Just inline this into the next statement:

    InfoBarService::FromWebContents(web_contents())->RemoveObserver(this);

This will allow removing the {} too.

[tfarina, 2014/06/03 03:47:13]

Done.

+    infobar_service->RemoveObserver(this);
+  }
+
   for (PluginStateMap::iterator i = hung_plugins_.begin();
        i != hung_plugins_.end(); ++i) {
     PluginState* state = i->second.get();
@@ -348,6 +348,14 @@ void HungPluginTabHelper::Observe(
   }
 }
 
+void HungPluginTabHelper::OnManagerShuttingDown(
+    infobars::InfoBarManager* manager) {
+  InfoBarService* infobar_service =
+      InfoBarService::FromWebContents(web_contents());
+  if (infobar_service)

[Peter Kasting, 2014/05/31 03:44:59]

How can this be NULL if we're getting this notification?

+    infobar_service->RemoveObserver(this);
+}
+
 void HungPluginTabHelper::KillPlugin(int child_id) {
 #if defined(OS_WIN)
   // Dump renderers that are sending or receiving pepper messages, in order to
@@ -412,6 +420,12 @@ void HungPluginTabHelper::ShowBar(int child_id, PluginState* state) {
   DCHECK(!state->infobar);
   state->infobar = HungPluginInfoBarDelegate::Create(infobar_service, this,
                                                      child_id, state->name);
+
+  if (state->infobar)
+    number_of_infobars_++;

[Peter Kasting, 2014/05/31 03:44:59]

Nit: Preincrement

[tfarina, 2014/06/03 03:47:13]

Done.

+
+  if (number_of_infobars_ == 1)

[Peter Kasting, 2014/05/31 03:44:59]

This needs to be placed inside the (state->infobar) conditional check. 
Otherwise, if we add one infobar, then fail to create the second, we'll try to
AddObserver() again.

[tfarina, 2014/06/03 03:47:13]

Done.

+    infobar_service->AddObserver(this);
 }
 
 void HungPluginTabHelper::CloseBar(PluginState* state) {