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..93529d161fff4e8ca9d82d8bd640575727b09964 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() {

[tfarina, 2014/06/07 05:26:21]

Peter, I got the backtrace for this.

It is this (beautiful) thing:

Breakpoint 1, HungPluginTabHelper::~HungPluginTabHelper (this=0x3390eeb12bd0) at
../../chrome/browser/ui/hung_plugin_tab_helper.cc:265
265	HungPluginTabHelper::~HungPluginTabHelper() {
(gdb) bt
#0  HungPluginTabHelper::~HungPluginTabHelper (this=0x3390eeb12bd0) at
../../chrome/browser/ui/hung_plugin_tab_helper.cc:265
#1  0x0000555557acd58c in non-virtual thunk to
HungPluginTabHelper::~HungPluginTabHelper() ()
    at ../../chrome/browser/ui/hung_plugin_tab_helper.cc:267
#2  0x00007ffff5eeb837 in linked_ptr<base::SupportsUserData::Data>::depart
(this=0x3390eeaf4eb8) at ../../base/memory/linked_ptr.h:146
#3  0x00007ffff5eea965 in linked_ptr<base::SupportsUserData::Data>::~linked_ptr
(this=0x3390eeaf4eb8) at ../../base/memory/linked_ptr.h:84
#4  0x00007ffff5eeb41c in std::pair<void const* const,
linked_ptr<base::SupportsUserData::Data> >::~pair (this=0x3390eeaf4eb0)
    at
/usr/lib/gcc/x86_64-linux-gnu/4.7/../../../../include/c++/4.7/bits/stl_pair.h:88
#5  0x00007ffff5eeb3ec in std::_Rb_tree_node<std::pair<void const* const,
linked_ptr<base::SupportsUserData::Data> > >::~_Rb_tree_node (
    this=0x3390eeaf4e90) at
/usr/lib/gcc/x86_64-linux-gnu/4.7/../../../../include/c++/4.7/bits/stl_tree.h:130
troy<std::_Rb_tree_node<std::pair<void const* const,
linked_ptr<base::SupportsUserData::Data> > > > (this=0x7fffffff8200,
__p=0x3390eeaf4e90)
    at
/usr/lib/gcc/x86_64-linux-gnu/4.7/../../../../include/c++/4.7/ext/new_allocator.h:114
#7  0x00007ffff5eeb32c in std::_Rb_tree<void const*, std::pair<void const*
const, linked_ptr<base::SupportsUserData::Data> >,
std::_Select1st<std::pair<void const* const,
linked_ptr<base::SupportsUserData::Data> > >, std::less<void const*>,
std::allocator<std::pair<void const* const,
linked_ptr<base::SupportsUserData::Data> > > >::_M_destroy_node
(this=0x7fffffff8200, __p=0x3390eeaf4e90)
    at
/usr/lib/gcc/x86_64-linux-gnu/4.7/../../../../include/c++/4.7/bits/stl_tree.h:419
#8  0x00007ffff5eed2db in std::_Rb_tree<void const*, std::pair<void const*
const, linked_ptr<base::SupportsUserData::Data> >,
std::_Select1st<std::pair<void const* const,
linked_ptr<base::SupportsUserData::Data> > >, std::less<void const*>,
std::allocator<std::pair<void const* const,
linked_ptr<base::SupportsUserData::Data> > > >::_M_erase (this=0x7fffffff8200,
__x=0x3390eeaf4e90)
    at
/usr/lib/gcc/x86_64-linux-gnu/4.7/../../../../include/c++/4.7/bits/stl_tree.h:1084
#9  0x00007ffff5eed2be in std::_Rb_tree<void const*, std::pair<void const*
const, linked_ptr<base::SupportsUserData::Data> >,
std::_Select1st<std::pair<void const* const,
linked_ptr<base::SupportsUserData::Data> > >, std::less<void const*>,
std::allocator<std::pair<void const* const,
linked_ptr<base::SupportsUserData::Data> > > >::_M_erase (this=0x7fffffff8200,
__x=0x3390ee68e100)
    at
/usr/lib/gcc/x86_64-linux-gnu/4.7/../../../../include/c++/4.7/bits/stl_tree.h:1082
#10 0x00007ffff5eed265 in std::_Rb_tree<void const*, std::pair<void const*
const, linked_ptr<base::SupportsUserData::Data> >,
std::_Select1st<std::pair<void const* const,
linked_ptr<base::SupportsUserData::Data> > >, std::less<void const*>,
std::allocator<std::pair<void const* const,
linked_ptr<base::SupportsUserData::Data> > > >::~_Rb_tree (this=0x7fffffff8200)
    at
/usr/lib/gcc/x86_64-linux-gnu/4.7/../../../../include/c++/4.7/bits/stl_tree.h:646
#11 0x00007ffff5eed235 in std::__cxx1998::map<void const*,
linked_ptr<base::SupportsUserData::Data>, std::less<void const*>,
std::allocator<std::pair<void const* const,
linked_ptr<base::SupportsUserData::Data> > > >::~map (this=0x7fffffff8200)
    at
/usr/lib/gcc/x86_64-linux-gnu/4.7/../../../../include/c++/4.7/bits/stl_map.h:90
#12 0x00007ffff5eeaae1 in std::__debug::map<void const*,
linked_ptr<base::SupportsUserData::Data>, std::less<void const*>,
std::allocator<std::pair<void const* const,
linked_ptr<base::SupportsUserData::Data> > > >::~map (this=0x7fffffff8200)
    at
/usr/lib/gcc/x86_64-linux-gnu/4.7/../../../../include/c++/4.7/debug/map.h:107
#13 0x00007ffff5eea30e in base::SupportsUserData::~SupportsUserData
(this=0x3390eda13230) at ../../base/supports_user_data.cc:43
---Type <return> to continue, or q <return> to quit---
#14 0x00007fffe93ec065 in content::WebContents::~WebContents
(this=0x3390eda13220) at ../../content/public/browser/web_contents.h:149
#15 0x00007fffe93ade4c in content::WebContentsImpl::~WebContentsImpl
(this=0x3390eda13220)
    at ../../content/browser/web_contents/web_contents_impl.cc:426
#16 0x00007fffe93ae1a9 in content::WebContentsImpl::~WebContentsImpl
(this=0x3390eda13220)
    at ../../content/browser/web_contents/web_contents_impl.cc:358
[...]

+  DCHECK_EQ(0, number_of_infobars_);

[Peter Kasting, 2014/06/03 18:41:45]

Are you sure we're guaranteed that the InfoBarManager is destroyed before the
HungPluginTabHelper?  Because if the order could go either way, this DCHECK
isn't right.

It would probably help me to know what the callstacks are that lead to these
objects' destruction, so it's clear that one must be destroyed later than the
other.

We should also have a comment here describing why this DCHECK holds.

 }
 
 void HungPluginTabHelper::PluginCrashed(const base::FilePath& plugin_path,
@@ -321,13 +318,13 @@ 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/06/03 18:41:45]

This is not safe.  You get called on every kind of infobar removal, and you need
to be sure that you're being called about a hung plugin infobar before you act.

In other words, all this code needs to move inside the conditional below where
you find that this infobar is one of the infobars you're tracking.

[tfarina, 2014/06/07 05:26:21]

Done.

+

[Peter Kasting, 2014/06/03 18:41:45]

Nit: I'd avoid this newline (2 places)

[tfarina, 2014/06/07 05:26:21]

Done.

+  if (number_of_infobars_ == 0)
+    InfoBarService::FromWebContents(web_contents())->RemoveObserver(this);
+
   for (PluginStateMap::iterator i = hung_plugins_.begin();
        i != hung_plugins_.end(); ++i) {
     PluginState* state = i->second.get();
@@ -412,6 +409,13 @@ 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_;
+
+    if (number_of_infobars_ == 1)
+      infobar_service->AddObserver(this);
+  }
 }
 
 void HungPluginTabHelper::CloseBar(PluginState* state) {