InfoBarService inherits from InfoBarManager

chrome/browser/infobars/infobar_delegate.cc

Index: chrome/browser/infobars/infobar_delegate.cc
diff --git a/chrome/browser/infobars/infobar_delegate.cc b/chrome/browser/infobars/infobar_delegate.cc
index 5a6970758935c1eeae2e58a2472febcb53205db5..bdc0e36065a307121311270731b3324e3d18fc17 100644
--- a/chrome/browser/infobars/infobar_delegate.cc
+++ b/chrome/browser/infobars/infobar_delegate.cc
@@ -14,6 +14,17 @@
 #include "content/public/browser/web_contents.h"
 #include "ui/base/resource/resource_bundle.h"
 
+namespace {
+content::WebContents* WebContentsFromInfoBar(InfoBar* infobar) {

[droger, 2014/04/08 15:00:11]

The plan would be to make this helper function public so that any infobar
delegate that needs it can use it instead of InfoBarDelegate::web_contents().

[Peter Kasting, 2014/04/08 20:39:56]

I didn't go this route for two reasons:
(1) Callsite verbosity -- web_contents() is a lot simpler than
WebContentsFromInfoBar(this).
(2) Type-checking -- ensuring web_contents() is available due to an inheritance
hierarchy means that the only delegates that can call it are those that inherit
from the appropriate base class.  If this is public static, then any infobar
could theoretically call it, which we don't want.

[droger, 2014/04/08 20:54:46]

About (2), I don't think we still need to have different base classes for the
infobar delegates anymore (i.e. ContentConfirmInfoBar). The only point of these
base class was to add the web_contents() method, but this is no longer needed
with this design.

[Peter Kasting, 2014/04/08 21:41:35]

Right, I was suggesting that the base class route was preferable to the
no-base-class route.

I might be wrong.  This way might be better.  What really worries me is the
assumption here that infobar->owner() can be safely static_casted to
InfoBarService*.  Nothing in our current design really enforces this.  I was
hoping by use of inheritance to try and make it more mandatory that this holds. 
Maybe that doesn't actually improve, though.

In any case, it seems like you can't declare this in infobar_delegate.h if you
want that file to live in a component.  So where were you planning to move the
declaration?

+  if (!infobar || !infobar->owner())
+    return NULL;
+
+  InfoBarService* infobar_service =
+      static_cast<InfoBarService*>(infobar->owner());
+  return infobar_service->web_contents();
+}
+}  // namespace
+
 using content::NavigationEntry;
 
 // InfoBarDelegate ------------------------------------------------------------
@@ -108,8 +119,7 @@ gfx::Image InfoBarDelegate::GetIcon() const {
 }
 
 content::WebContents* InfoBarDelegate::web_contents() {
-  return (infobar_ && infobar_->owner()) ?
-      infobar_->owner()->web_contents() : NULL;
+  return WebContentsFromInfoBar(infobar_);
 }
 
 InfoBarDelegate::InfoBarDelegate() : contents_unique_id_(0) {