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),
+      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,
                                         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.
   InfoBarService* infobar_service =
       InfoBarService::FromWebContents(web_contents());
   if (!infobar_service)
@@ skipping 34 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;
+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();
     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)

[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
   // 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 44 matching lines @@
 
 void HungPluginTabHelper::ShowBar(int child_id, PluginState* state) {
   InfoBarService* infobar_service =
       InfoBarService::FromWebContents(web_contents());
   if (!infobar_service)
     return;
 
   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) {
   InfoBarService* infobar_service =
       InfoBarService::FromWebContents(web_contents());
   if (infobar_service && state->infobar) {
     infobar_service->RemoveInfoBar(state->infobar);
     state->infobar = NULL;
   }
 }