Change HungPluginTabHelper to listen for infobar changes through Observer style.

chrome/browser/ui/hung_plugin_tab_helper.cc

 // 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.
 
 #include "chrome/browser/ui/hung_plugin_tab_helper.h"
 
 #include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/memory/scoped_ptr.h"
 #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"
 #include "components/infobars/core/infobar.h"
 #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"
 #include "content/public/common/result_codes.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "grit/locale_settings.h"
 #include "grit/theme_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 
@@ skipping 221 matching lines @@
 
 HungPluginTabHelper::PluginState::~PluginState() {
 }
 
 
 // HungPluginTabHelper --------------------------------------------------------
 
 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),
+      observer_installed_(false) {
 }
 
 HungPluginTabHelper::~HungPluginTabHelper() {
 }
 
 void HungPluginTabHelper::PluginCrashed(const base::FilePath& plugin_path,
                                         base::ProcessId plugin_pid) {
   // TODO(brettw) ideally this would take the child process ID. When we do this
   // for NaCl plugins, we'll want to know exactly which process it was since
   // the path won't be useful.
@@ skipping 37 matching lines @@
 
   base::string16 plugin_name =
       content::PluginService::GetInstance()->GetPluginDisplayNameByPath(
           plugin_path);
 
   linked_ptr<PluginState> state(new PluginState(plugin_path, plugin_name));
   hung_plugins_[plugin_child_id] = state;
   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;
-  for (PluginStateMap::iterator i = hung_plugins_.begin();
-       i != hung_plugins_.end(); ++i) {
-    PluginState* state = i->second.get();
-    if (state->infobar == infobar) {
-      state->infobar = NULL;
-
-      // Schedule the timer to re-show the infobar if the plugin continues to be
-      // hung.
-      state->timer.Start(FROM_HERE, state->next_reshow_delay,
-          base::Bind(&HungPluginTabHelper::OnReshowTimer,
-                     base::Unretained(this),
-                     i->first));
-
-      // Next time we do this, delay it twice as long to avoid being annoying.
-      state->next_reshow_delay *= 2;
-      return;
-    }
-  }
-}
-
 void HungPluginTabHelper::KillPlugin(int child_id) {
 #if defined(OS_WIN)
   // Dump renderers that are sending or receiving pepper messages, in order to
   // diagnose inter-process deadlocks.
   // Only do that on the Canary channel, for 20% of pepper plugin hangs.
   if (base::RandInt(0, 100) < 20) {
     chrome::VersionInfo::Channel channel = chrome::VersionInfo::GetChannel();
     if (channel == chrome::VersionInfo::CHANNEL_CANARY) {
       scoped_ptr<OwnedHandleVector> renderer_handles(new OwnedHandleVector);
       HANDLE current_process = ::GetCurrentProcess();
@@ skipping 41 matching lines @@
   DCHECK(!found->second->infobar);
   ShowBar(child_id, found->second.get());
 }
 
 void HungPluginTabHelper::ShowBar(int child_id, PluginState* state) {
   InfoBarService* infobar_service =
       InfoBarService::FromWebContents(web_contents());
   if (!infobar_service)
     return;
 
+  if (!observer_installed_) {
+    observer_installed_ = true;
+    infobar_service->AddObserver(this);
+  }
+
   DCHECK(!state->infobar);
   state->infobar = HungPluginInfoBarDelegate::Create(infobar_service, this,
                                                      child_id, state->name);
 }
 
 void HungPluginTabHelper::CloseBar(PluginState* state) {
   InfoBarService* infobar_service =
       InfoBarService::FromWebContents(web_contents());
   if (infobar_service && state->infobar) {
     infobar_service->RemoveInfoBar(state->infobar);
     state->infobar = NULL;
+
+    if (observer_installed_) {
+      observer_installed_ = false;
+      infobar_service->RemoveObserver(this);

[Peter Kasting, 2014/05/30 18:54:47]

This still doesn't seem right.

(1) What if the bar is closed by a different method than CloseBar(), e.g. the
user closing the infobar?  Then we won't get notified.  Seems like this code
should instead be in OnInfoBarRemoved().
(2) What if we're observing notificatinos for more than one infobar?  Then we
don't want to remove our observer yet.

Also, is it possible for the class to be destroyed with infobars open?  If so,
we need to RemoveObserver() in our destructor.  If not, the destructor should
DCHECK that we're no longer an observer.

[tfarina, 2014/05/30 21:14:03]

Not sure if that answers your question. But the biggest issue (imo) is that
InfoBarService is destroyed before HungPluginTabHelper, and possibly before many
other TabHelpers.

If HPTH was destroyed before, then we could deregister from it in the dtor.

+    }
   }
 }
+
+void HungPluginTabHelper::OnInfoBarRemoved(infobars::InfoBar* infobar,
+                                           bool animate) {
+  for (PluginStateMap::iterator i = hung_plugins_.begin();
+       i != hung_plugins_.end(); ++i) {
+    PluginState* state = i->second.get();
+    if (state->infobar == infobar) {
+      state->infobar = NULL;
+
+      // Schedule the timer to re-show the infobar if the plugin continues to be
+      // hung.
+      state->timer.Start(FROM_HERE,
+                         state->next_reshow_delay,
+                         base::Bind(&HungPluginTabHelper::OnReshowTimer,
+                                    base::Unretained(this),
+                                    i->first));
+
+      // Next time we do this, delay it twice as long to avoid being annoying.
+      state->next_reshow_delay *= 2;
+      return;
+    }
+  }
+}
+
+void HungPluginTabHelper::OnManagerShuttingDown(
+    infobars::InfoBarManager* manager) {
+  InfoBarService* infobar_service =
+      InfoBarService::FromWebContents(web_contents());
+  if (!infobar_service)
+    return;
+
+  if (observer_installed_) {
+    observer_installed_ = false;
+    infobar_service->RemoveObserver(this);
+  }
+}