Keep infobar visible during plug-in installation and show installation state.

Instead of dismissing the infobar when the user accepts it, it's now dismissed when the last missing plug-in placeholder is gone (because we loaded the actual plug-in).

BUG=110701
TEST=none


Review URL: http://codereview.chromium.org/9269007

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

8 files changed, 172 insertions, 48 deletions

diff --git a/chrome/browser/plugin_installer.cc b/chrome/browser/plugin_installer.cc
index 0316412..e812282 100644
--- a/chrome/browser/plugin_installer.cc
+++ b/chrome/browser/plugin_installer.cc
@@ -33,6 +33,19 @@ void PluginInstaller::AddObserver(PluginInstallerObserver* observer) {
 
 void PluginInstaller::RemoveObserver(PluginInstallerObserver* observer) {
   observers_.RemoveObserver(observer);
+  if (observers_.size() == weak_observers_.size()) {
+    FOR_EACH_OBSERVER(WeakPluginInstallerObserver, weak_observers_,
+                      OnlyWeakObserversLeft());
+  }
+}
+
+void PluginInstaller::AddWeakObserver(WeakPluginInstallerObserver* observer) {
+  weak_observers_.AddObserver(observer);
+}
+
+void PluginInstaller::RemoveWeakObserver(
+    WeakPluginInstallerObserver* observer) {
+  weak_observers_.RemoveObserver(observer);
 }
 
 void PluginInstaller::StartInstalling(
@@ -50,6 +63,12 @@ void PluginInstaller::StartInstalling(
       base::Bind(&PluginInstaller::DownloadError, base::Unretained(this)));
 }
 
+void PluginInstaller::DidOpenDownloadURL() {
+  DCHECK(state_ == kStateIdle);
+  DCHECK(url_for_display_);
+  FOR_EACH_OBSERVER(PluginInstallerObserver, observers_, DidFinishDownload());
+}
+
 void PluginInstaller::DidFinishDownload(const FilePath& downloaded_file) {
   DCHECK(state_ == kStateDownloading);
   state_ = kStateIdle;
@@ -59,5 +78,7 @@ void PluginInstaller::DidFinishDownload(const FilePath& downloaded_file) {
 }
 
 void PluginInstaller::DownloadError(const std::string& msg) {
+  DCHECK(state_ == kStateDownloading);
+  state_ = kStateIdle;
   FOR_EACH_OBSERVER(PluginInstallerObserver, observers_, DownloadError(msg));
 }
diff --git a/chrome/browser/plugin_installer.h b/chrome/browser/plugin_installer.h
index 514d919..6f76967 100644
--- a/chrome/browser/plugin_installer.h
+++ b/chrome/browser/plugin_installer.h
@@ -12,6 +12,7 @@
 
 class FilePath;
 class PluginInstallerObserver;
+class WeakPluginInstallerObserver;
 
 namespace net {
 class URLRequestContextGetter;
@@ -34,6 +35,9 @@ class PluginInstaller {
   void AddObserver(PluginInstallerObserver* observer);
   void RemoveObserver(PluginInstallerObserver* observer);
 
+  void AddWeakObserver(WeakPluginInstallerObserver* observer);
+  void RemoveWeakObserver(WeakPluginInstallerObserver* observer);
+
   State state() const { return state_; }
 
   // Unique identifier for the plug-in. Should be kept in sync with the
@@ -55,12 +59,17 @@ class PluginInstaller {
 
   void StartInstalling(net::URLRequestContextGetter* request_context);
 
+  // Called when the browser opened the download URL in a new tab, to notify
+  // observers.
+  void DidOpenDownloadURL();
+
  private:
   void DidFinishDownload(const FilePath& downloaded_file);
   void DownloadError(const std::string& msg);
 
   State state_;
   ObserverList<PluginInstallerObserver> observers_;
+  ObserverList<WeakPluginInstallerObserver> weak_observers_;
 
   std::string identifier_;
   GURL plugin_url_;
diff --git a/chrome/browser/plugin_installer_infobar_delegate.cc b/chrome/browser/plugin_installer_infobar_delegate.cc
index 5945760..4716e40 100644
--- a/chrome/browser/plugin_installer_infobar_delegate.cc
+++ b/chrome/browser/plugin_installer_infobar_delegate.cc
@@ -4,8 +4,10 @@
 
 #include "chrome/browser/plugin_installer_infobar_delegate.h"
 
+#include "base/utf_string_conversions.h"
 #include "chrome/browser/google/google_util.h"
 #include "chrome/browser/infobars/infobar_tab_helper.h"
+#include "chrome/browser/plugin_installer.h"
 #include "content/browser/renderer_host/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "grit/generated_resources.h"
@@ -18,33 +20,48 @@ using content::OpenURLParams;
 using content::Referrer;
 
 PluginInstallerInfoBarDelegate::PluginInstallerInfoBarDelegate(
-    PluginInstaller* installer,
     InfoBarTabHelper* infobar_helper,
-    const string16& plugin_name,
-    const GURL& learn_more_url,
-    const base::Closure& callback)
+    PluginInstaller* installer,
+    const base::Closure& callback,
+    const string16& message)
     : ConfirmInfoBarDelegate(infobar_helper),
-      PluginInstallerObserver(installer),
-      plugin_name_(plugin_name),
-      learn_more_url_(learn_more_url),
-      callback_(callback) {
+      WeakPluginInstallerObserver(installer),
+      callback_(callback),
+      message_(message) {
 }
 
 PluginInstallerInfoBarDelegate::~PluginInstallerInfoBarDelegate() {
 }
 
+InfoBarDelegate* PluginInstallerInfoBarDelegate::Create(
+    InfoBarTabHelper* infobar_helper,
+    PluginInstaller* installer,
+    const base::Closure& callback) {
+  string16 message;
+  switch (installer->state()) {
+    case PluginInstaller::kStateIdle:
+      message = l10n_util::GetStringFUTF16(
+          IDS_PLUGININSTALLER_INSTALLPLUGIN_PROMPT, installer->name());
+      break;
+    case PluginInstaller::kStateDownloading:
+      message = l10n_util::GetStringUTF16(IDS_PLUGIN_DOWNLOADING);
+      break;
+  }
+  return new PluginInstallerInfoBarDelegate(
+      infobar_helper, installer, callback, message);
+}
+
 gfx::Image* PluginInstallerInfoBarDelegate::GetIcon() const {
   return &ResourceBundle::GetSharedInstance().GetNativeImageNamed(
       IDR_INFOBAR_PLUGIN_INSTALL);
 }
 
 string16 PluginInstallerInfoBarDelegate::GetMessageText() const {
-  return l10n_util::GetStringFUTF16(IDS_PLUGININSTALLER_INSTALLPLUGIN_PROMPT,
-                                    plugin_name_);
+  return message_;
 }
 
 int PluginInstallerInfoBarDelegate::GetButtons() const {
-  return BUTTON_OK;
+  return callback_.is_null() ? BUTTON_NONE : BUTTON_OK;
 }
 
 string16 PluginInstallerInfoBarDelegate::GetButtonLabel(
@@ -55,7 +72,7 @@ string16 PluginInstallerInfoBarDelegate::GetButtonLabel(
 
 bool PluginInstallerInfoBarDelegate::Accept() {
   callback_.Run();
-  return true;
+  return false;
 }
 
 string16 PluginInstallerInfoBarDelegate::GetLinkText() const {
@@ -64,7 +81,7 @@ string16 PluginInstallerInfoBarDelegate::GetLinkText() const {
 
 bool PluginInstallerInfoBarDelegate::LinkClicked(
     WindowOpenDisposition disposition) {
-  GURL url(learn_more_url_);
+  GURL url(installer()->help_url());
   if (url.is_empty()) {
     url = google_util::AppendGoogleLocaleParam(GURL(
       "https://www.google.com/support/chrome/bin/answer.py?answer=142064"));
@@ -79,6 +96,33 @@ bool PluginInstallerInfoBarDelegate::LinkClicked(
 }
 
 void PluginInstallerInfoBarDelegate::DidStartDownload() {
+  ReplaceWithInfoBar(l10n_util::GetStringUTF16(IDS_PLUGIN_DOWNLOADING));
+}
+
+void PluginInstallerInfoBarDelegate::DidFinishDownload() {
+  ReplaceWithInfoBar(l10n_util::GetStringUTF16(IDS_PLUGIN_INSTALLING));
+}
+
+void PluginInstallerInfoBarDelegate::DownloadError(const std::string& message) {
+  ReplaceWithInfoBar(
+      l10n_util::GetStringUTF16(IDS_PLUGIN_DOWNLOAD_ERROR_SHORT));
+}
+
+void PluginInstallerInfoBarDelegate::OnlyWeakObserversLeft() {
   if (owner())
     owner()->RemoveInfoBar(this);
 }
+
+void PluginInstallerInfoBarDelegate::ReplaceWithInfoBar(
+    const string16& message) {
+  // Return early if the message doesn't change. This is important in case the
+  // PluginInstaller is still iterating over its observers (otherwise we would
+  // keep replacing infobar delegates infinitely).
+  if (message_ == message)
+    return;
+  if (!owner())
+    return;
+  InfoBarDelegate* delegate = new PluginInstallerInfoBarDelegate(
+      owner(), installer(), base::Closure(), message);
+  owner()->ReplaceInfoBar(this, delegate);
+}
diff --git a/chrome/browser/plugin_installer_infobar_delegate.h b/chrome/browser/plugin_installer_infobar_delegate.h
index b43b623..5dad51e 100644
--- a/chrome/browser/plugin_installer_infobar_delegate.h
+++ b/chrome/browser/plugin_installer_infobar_delegate.h
@@ -9,24 +9,27 @@
 #include "base/callback.h"
 #include "chrome/browser/plugin_installer_observer.h"
 #include "chrome/browser/tab_contents/confirm_infobar_delegate.h"
+#include "chrome/browser/tab_contents/link_infobar_delegate.h"
 #include "googleurl/src/gurl.h"
 
 // The main purpose for this class is to popup/close the infobar when there is
 // a missing plugin.
 class PluginInstallerInfoBarDelegate : public ConfirmInfoBarDelegate,
-                                       public PluginInstallerObserver {
+                                       public WeakPluginInstallerObserver {
  public:
-  // Shows an infobar asking whether to install the plugin with the name
-  // |plugin_name|. When the user accepts, |callback| is called.
-  // If |installer| is not NULL, registers itself as its observer, to dismiss
-  // the infobar if it's accepted in another tab.
-  PluginInstallerInfoBarDelegate(PluginInstaller* installer,
-                                 InfoBarTabHelper* infobar_helper,
-                                 const string16& plugin_name,
-                                 const GURL& learn_more_url,
+  // Shows an infobar asking whether to install the plugin represented by
+  // |installer|. When the user accepts, |callback| is called.
+  // During installation of the plug-in, the infobar will change to reflect the
+  // installation state.
+  static InfoBarDelegate* Create(InfoBarTabHelper* infobar_helper,
+                                 PluginInstaller* installer,
                                  const base::Closure& callback);
 
  private:
+  PluginInstallerInfoBarDelegate(InfoBarTabHelper* infobar_helper,
+                                 PluginInstaller* installer,
+                                 const base::Closure& callback,
+                                 const string16& message);
   virtual ~PluginInstallerInfoBarDelegate();
 
   // ConfirmInfoBarDelegate:
@@ -40,11 +43,20 @@ class PluginInstallerInfoBarDelegate : public ConfirmInfoBarDelegate,
 
   // PluginInstallerObserver:
   virtual void DidStartDownload() OVERRIDE;
+  virtual void DidFinishDownload() OVERRIDE;
+  virtual void DownloadError(const std::string& message) OVERRIDE;
+
+  // WeakPluginInstallerObserver:
+  virtual void OnlyWeakObserversLeft() OVERRIDE;
+
+  // Replaces this infobar with one showing |message|. The new infobar will
+  // not have any buttons (and not call the callback).
+  void ReplaceWithInfoBar(const string16& message);
 
-  string16 plugin_name_;
-  GURL learn_more_url_;
   base::Closure callback_;
 
+  string16 message_;
+
   DISALLOW_COPY_AND_ASSIGN(PluginInstallerInfoBarDelegate);
 };
 
diff --git a/chrome/browser/plugin_installer_observer.cc b/chrome/browser/plugin_installer_observer.cc
index 72284c7..419f60e 100644
--- a/chrome/browser/plugin_installer_observer.cc
+++ b/chrome/browser/plugin_installer_observer.cc
@@ -8,13 +8,11 @@
 
 PluginInstallerObserver::PluginInstallerObserver(PluginInstaller* installer)
     : installer_(installer) {
-  if (installer)
-    installer->AddObserver(this);
+  installer->AddObserver(this);
 }
 
 PluginInstallerObserver::~PluginInstallerObserver() {
-  if (installer_)
-    installer_->RemoveObserver(this);
+  installer_->RemoveObserver(this);
 }
 
 void PluginInstallerObserver::DidStartDownload() {
@@ -25,3 +23,15 @@ void PluginInstallerObserver::DidFinishDownload() {
 
 void PluginInstallerObserver::DownloadError(const std::string& message) {
 }
+
+WeakPluginInstallerObserver::WeakPluginInstallerObserver(
+    PluginInstaller* installer) : PluginInstallerObserver(installer) {
+  installer->AddWeakObserver(this);
+}
+
+WeakPluginInstallerObserver::~WeakPluginInstallerObserver() {
+  installer()->RemoveWeakObserver(this);
+}
+
+void WeakPluginInstallerObserver::OnlyWeakObserversLeft() {
+}
diff --git a/chrome/browser/plugin_installer_observer.h b/chrome/browser/plugin_installer_observer.h
index 1d95afd..b9b228a 100644
--- a/chrome/browser/plugin_installer_observer.h
+++ b/chrome/browser/plugin_installer_observer.h
@@ -16,7 +16,7 @@ class PluginInstallerObserver {
   virtual ~PluginInstallerObserver();
 
  protected:
-  PluginInstaller* installer() { return installer_; }
+  PluginInstaller* installer() const { return installer_; }
 
  private:
   friend class PluginInstaller;
@@ -29,4 +29,18 @@ class PluginInstallerObserver {
   PluginInstaller* installer_;
 };
 
+// A WeakPluginInstallerObserver is like a weak pointer to the installer, in the
+// sense that if only weak observers are left, we don't need to show
+// installation UI anymore.
+class WeakPluginInstallerObserver : public PluginInstallerObserver {
+ public:
+  explicit WeakPluginInstallerObserver(PluginInstaller* installer);
+  virtual ~WeakPluginInstallerObserver();
+
+ private:
+  friend class PluginInstaller;
+
+  virtual void OnlyWeakObserversLeft();
+};
+
 #endif  // CHROME_BROWSER_PLUGIN_INSTALLER_OBSERVER_H_
diff --git a/chrome/browser/plugin_observer.cc b/chrome/browser/plugin_observer.cc
index eb85064..975b369 100644
--- a/chrome/browser/plugin_observer.cc
+++ b/chrome/browser/plugin_observer.cc
@@ -6,6 +6,7 @@
 
 #include "base/auto_reset.h"
 #include "base/bind.h"
+#include "base/stl_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/content_settings/host_content_settings_map.h"
@@ -300,7 +301,7 @@ bool OutdatedPluginInfoBarDelegate::LinkClicked(
 
 #if defined(ENABLE_PLUGIN_INSTALLATION)
 class ConfirmInstallDialogDelegate : public TabModalConfirmDialogDelegate,
-                                     public PluginInstallerObserver {
+                                     public WeakPluginInstallerObserver {
  public:
   ConfirmInstallDialogDelegate(WebContents* web_contents,
                                PluginInstaller* installer);
@@ -314,6 +315,7 @@ class ConfirmInstallDialogDelegate : public TabModalConfirmDialogDelegate,
 
   // PluginInstallerObserver methods:
   virtual void DidStartDownload() OVERRIDE;
+  virtual void OnlyWeakObserversLeft() OVERRIDE;
 
  private:
   net::URLRequestContextGetter* request_context_;
@@ -323,7 +325,7 @@ ConfirmInstallDialogDelegate::ConfirmInstallDialogDelegate(
     WebContents* web_contents,
     PluginInstaller* installer)
     : TabModalConfirmDialogDelegate(web_contents),
-      PluginInstallerObserver(installer),
+      WeakPluginInstallerObserver(installer),
       request_context_(web_contents->GetBrowserContext()->GetRequestContext()) {
 }
 
@@ -352,6 +354,10 @@ void ConfirmInstallDialogDelegate::OnCanceled() {
 void ConfirmInstallDialogDelegate::DidStartDownload() {
   Cancel();
 }
+
+void ConfirmInstallDialogDelegate::OnlyWeakObserversLeft() {
+  Cancel();
+}
 #endif  // defined(ENABLE_PLUGIN_INSTALLATION)
 
 }  // namespace
@@ -371,7 +377,6 @@ class PluginObserver::MissingPluginHost : public PluginInstallerObserver {
       case PluginInstaller::kStateIdle: {
         observer->Send(new ChromeViewMsg_FoundMissingPlugin(routing_id_,
                                                             installer->name()));
-        observer->ShowPluginInstallationInfoBar(installer);
         break;
       }
       case PluginInstaller::kStateDownloading: {
@@ -409,6 +414,7 @@ PluginObserver::PluginObserver(TabContentsWrapper* tab_contents)
 }
 
 PluginObserver::~PluginObserver() {
+  STLDeleteValues(&missing_plugins_);
 }
 
 bool PluginObserver::OnMessageReceived(const IPC::Message& message) {
@@ -418,6 +424,8 @@ bool PluginObserver::OnMessageReceived(const IPC::Message& message) {
 #if defined(ENABLE_PLUGIN_INSTALLATION)
     IPC_MESSAGE_HANDLER(ChromeViewHostMsg_FindMissingPlugin,
                         OnFindMissingPlugin)
+    IPC_MESSAGE_HANDLER(ChromeViewHostMsg_RemoveMissingPluginHost,
+                        OnRemoveMissingPluginHost)
 #endif
     IPC_MESSAGE_HANDLER(ChromeViewHostMsg_OpenAboutPlugins,
                         OnOpenAboutPlugins)
@@ -455,19 +463,14 @@ void PluginObserver::OnFindMissingPlugin(int placeholder_id,
 void PluginObserver::FoundMissingPlugin(int placeholder_id,
                                         const std::string& mime_type,
                                         PluginInstaller* installer) {
-  missing_plugins_.push_back(
-      new MissingPluginHost(this, placeholder_id, installer));
-}
-
-void PluginObserver::ShowPluginInstallationInfoBar(PluginInstaller* installer) {
+  missing_plugins_[placeholder_id] =
+      new MissingPluginHost(this, placeholder_id, installer);
   InfoBarTabHelper* infobar_helper = tab_contents_->infobar_tab_helper();
-  infobar_helper->AddInfoBar(new PluginInstallerInfoBarDelegate(
-      installer,
-      infobar_helper,
-      installer->name(),
-      installer->help_url(),
+  InfoBarDelegate* delegate = PluginInstallerInfoBarDelegate::Create(
+      infobar_helper, installer,
       base::Bind(&PluginObserver::InstallMissingPlugin,
-                 weak_ptr_factory_.GetWeakPtr(), installer)));
+                 weak_ptr_factory_.GetWeakPtr(), installer));
+  infobar_helper->AddInfoBar(delegate);
 }
 
 void PluginObserver::DidNotFindMissingPlugin(int placeholder_id) {
@@ -481,12 +484,24 @@ void PluginObserver::InstallMissingPlugin(PluginInstaller* installer) {
         content::Referrer(web_contents()->GetURL(),
                           WebKit::WebReferrerPolicyDefault),
         NEW_FOREGROUND_TAB, content::PAGE_TRANSITION_TYPED, false));
+    installer->DidOpenDownloadURL();
   } else {
     browser::ShowTabModalConfirmDialog(
         new ConfirmInstallDialogDelegate(web_contents(), installer),
         tab_contents_);
   }
 }
+
+void PluginObserver::OnRemoveMissingPluginHost(int placeholder_id) {
+  std::map<int, MissingPluginHost*>::iterator it =
+      missing_plugins_.find(placeholder_id);
+  if (it == missing_plugins_.end()) {
+    NOTREACHED();
+    return;
+  }
+  delete it->second;
+  missing_plugins_.erase(it);
+}
 #endif  // defined(ENABLE_PLUGIN_INSTALLATION)
 
 void PluginObserver::OnOpenAboutPlugins() {
diff --git a/chrome/browser/plugin_observer.h b/chrome/browser/plugin_observer.h
index e8b8fdd..58f3e11 100644
--- a/chrome/browser/plugin_observer.h
+++ b/chrome/browser/plugin_observer.h
@@ -10,7 +10,7 @@
 #include "content/public/browser/web_contents_observer.h"
 
 #if defined(ENABLE_PLUGIN_INSTALLATION)
-#include "base/memory/scoped_vector.h"
+#include <map>
 #endif
 
 class GURL;
@@ -30,9 +30,6 @@ class PluginObserver : public content::WebContentsObserver {
   // content::WebContentsObserver implementation.
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
 
-  // Shows the infobar that offers to install a missing plug-in.
-  void ShowPluginInstallationInfoBar(PluginInstaller* installer);
-
  private:
 #if defined(ENABLE_PLUGIN_INSTALLATION)
   class MissingPluginHost;
@@ -49,13 +46,15 @@ class PluginObserver : public content::WebContentsObserver {
   void InstallMissingPlugin(PluginInstaller* installer);
 #endif
   void OnOpenAboutPlugins();
+  void OnRemoveMissingPluginHost(int placeholder_id);
 
   base::WeakPtrFactory<PluginObserver> weak_ptr_factory_;
 
   TabContentsWrapper* tab_contents_;
 
 #if defined(ENABLE_PLUGIN_INSTALLATION)
-  ScopedVector<MissingPluginHost> missing_plugins_;
+  // Stores all MissingPluginHosts, keyed by their routing ID.ß
+  std::map<int, MissingPluginHost*> missing_plugins_;
 #endif
 
   DISALLOW_COPY_AND_ASSIGN(PluginObserver);