Infobar Componentization Proof of Concept

chrome/browser/infobars/infobar_manager.h

Index: chrome/browser/infobars/infobar_manager.h
diff --git a/chrome/browser/infobars/infobar_service.h b/chrome/browser/infobars/infobar_manager.h
similarity index 61%
copy from chrome/browser/infobars/infobar_service.h
copy to chrome/browser/infobars/infobar_manager.h
index f5fd8b10b1e1742bbd557ad0daacfc339d6183d3..95fad2cd0b581a46bbd68a50262f998336db47ff 100644
--- a/chrome/browser/infobars/infobar_service.h
+++ b/chrome/browser/infobars/infobar_manager.h
@@ -1,23 +1,25 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright 2014 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.
 
-#ifndef CHROME_BROWSER_INFOBARS_INFOBAR_SERVICE_H_
-#define CHROME_BROWSER_INFOBARS_INFOBAR_SERVICE_H_
+#ifndef CHROME_BROWSER_INFOBARS_INFOBAR_MANAGER_H_
+#define CHROME_BROWSER_INFOBARS_INFOBAR_MANAGER_H_
 
 #include <vector>
 
 #include "base/memory/scoped_ptr.h"
-#include "content/public/browser/web_contents_observer.h"
-#include "content/public/browser/web_contents_user_data.h"
+#include "base/observer_list.h"
+#include "chrome/browser/infobars/infobar_delegate.h"
 
 class InfoBar;
 
 // Provides access to creating, removing and enumerating info bars
 // attached to a tab.
-class InfoBarService : public content::WebContentsObserver,
-                       public content::WebContentsUserData<InfoBarService> {
+class InfoBarManager {

[droger, 2014/03/18 15:59:53]

InfoBarManager has the core logic.
It will move in components/infobar/core/browser/

  public:
+  InfoBarManager();
+  ~InfoBarManager();
+
   // Adds the specified |infobar|, which already owns a delegate.
   //
   // If infobars are disabled for this tab or the tab already has an infobar
@@ -26,7 +28,7 @@ class InfoBarService : public content::WebContentsObserver,
   // immediately without being added.
   //
   // Returns the infobar if it was successfully added.
-  virtual InfoBar* AddInfoBar(scoped_ptr<InfoBar> infobar);
+  virtual InfoBar* AddInfoBar(scoped_ptr<InfoBar> infobar, int entry_id);
 
   // Removes the specified |infobar|.  This in turn may close immediately or
   // animate closed; at the end the infobar will delete itself.
@@ -46,7 +48,8 @@ class InfoBarService : public content::WebContentsObserver,
   //
   // NOTE: This does not perform any EqualsDelegate() checks like AddInfoBar().
   InfoBar* ReplaceInfoBar(InfoBar* old_infobar,
-                          scoped_ptr<InfoBar> new_infobar);
+                          scoped_ptr<InfoBar> new_infobar,
+                          int entry_id);
 
   // Returns the number of infobars for this tab.
   size_t infobar_count() const { return infobars_.size(); }
@@ -57,14 +60,25 @@ class InfoBarService : public content::WebContentsObserver,
   // Warning: Does not sanity check |index|.
   InfoBar* infobar_at(size_t index) { return infobars_[index]; }
 
-  // Retrieve the WebContents for the tab this service is associated with.
-  content::WebContents* web_contents() {
-    return content::WebContentsObserver::web_contents();
-  }
+  void RemoveAllInfoBars(bool animate);
 
- private:
-  friend class content::WebContentsUserData<InfoBarService>;
+  void OnNavigation(const InfoBarDelegate::NavigationDetails& details);
+
+  void CleanUp();

[Peter Kasting, 2014/03/18 18:29:17]

Nit: Again, add comments here (and, frankly, above as well) about when and why
this is called.

+
+  class Observer {

[droger, 2014/03/18 15:59:53]

Observer class, to replace the notifications.

[Peter Kasting, 2014/03/18 18:29:17]

Nit: Classes should be declared at the top of the containing class.

+   public:
+    virtual void OnInfoBarAdded(InfoBar* infobar) = 0;
+    virtual void OnInfoBarReplaced(InfoBar* old_infobar,
+                                   InfoBar* new_infobar) = 0;
+    virtual void OnInfoBarRemoved(InfoBar* infobar, bool animate) = 0;
+  };
+
+  void AddObserver(Observer* obs) { observer_list_.AddObserver(obs); }

[Peter Kasting, 2014/03/18 18:29:17]

Nit: Don't inline non-unix_hacker()-style functions.  (2 places)

 
+  void RemoveObserver(Observer* obs) { observer_list_.RemoveObserver(obs); }
+
+ private:
   // InfoBars associated with this InfoBarService.  We own these pointers.
   // However, this is not a ScopedVector, because we don't delete the infobars
   // directly once they've been added to this; instead, when we're done with an
@@ -72,28 +86,13 @@ class InfoBarService : public content::WebContentsObserver,
   // RemoveInfoBarInternal().
   typedef std::vector<InfoBar*> InfoBars;
 
-  explicit InfoBarService(content::WebContents* web_contents);
-  virtual ~InfoBarService();
-
-  // content::WebContentsObserver:
-  virtual void RenderProcessGone(base::TerminationStatus status) OVERRIDE;
-  virtual void NavigationEntryCommitted(
-      const content::LoadCommittedDetails& load_details) OVERRIDE;
-  virtual void WebContentsDestroyed(
-      content::WebContents* web_contents) OVERRIDE;
-  virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
-
   void RemoveInfoBarInternal(InfoBar* infobar, bool animate);
-  void RemoveAllInfoBars(bool animate);
-
-  // Message handlers.
-  void OnDidBlockDisplayingInsecureContent();
-  void OnDidBlockRunningInsecureContent();
 
   InfoBars infobars_;
   bool infobars_enabled_;
+  ObserverList<Observer> observer_list_;
 
-  DISALLOW_COPY_AND_ASSIGN(InfoBarService);
+  DISALLOW_COPY_AND_ASSIGN(InfoBarManager);
 };
 
-#endif  // CHROME_BROWSER_INFOBARS_INFOBAR_SERVICE_H_
+#endif  // CHROME_BROWSER_INFOBARS_INFOBAR_MANAGER_H_