Split InfoBarService core code into InfoBarManager

chrome/browser/ui/gtk/browser_window_gtk.cc

Index: chrome/browser/ui/gtk/browser_window_gtk.cc
diff --git a/chrome/browser/ui/gtk/browser_window_gtk.cc b/chrome/browser/ui/gtk/browser_window_gtk.cc
index 58288972d72b7aee719122f1291ba2f93ef6dd9d..15eb4be03ef1046344824ff54c4cc7735c7acd6a 100644
--- a/chrome/browser/ui/gtk/browser_window_gtk.cc
+++ b/chrome/browser/ui/gtk/browser_window_gtk.cc
@@ -802,8 +802,12 @@ void BrowserWindowGtk::OnActiveTabChanged(WebContents* old_contents,
 
   // Update various elements that are interested in knowing the current
   // WebContents.
-  infobar_container_->ChangeInfoBarService(
-      InfoBarService::FromWebContents(new_contents));
+  InfoBarManager* infobar_manager = NULL;

[blundell, 2014/03/31 08:29:22]

Optional thought for a follow-up CL: You use this code pattern several places in
this CL. It might be worth creating a static
InfoBarService::InfoBarManagerFromWebContents() method.

[Peter Kasting, 2014/03/31 20:05:34]

This comment made me glance at this code (which I didn't review originally).

Are you sure your new NULL-check is appropriate?  Can |infobar_service| ever
actually be NULL below?  I wouldn't expect that to normally be true... maybe
during shutdown?

In general, make sure you don't add any NULL-checks to the existing code unless
they're truly necessary, and when they are necessary, please leave a comment
about when the variable can be NULL.

[droger, 2014/04/01 10:48:34]

InfoBarService::FromWebContents() indeed returns NULL during shutdown
(InfoBarService removes itself from WebContents in WebContentsDestroyed).
I don't know if this code can be called during shutdown, but the existing code
was supporting the case where the InfoBarService was NULL (because
ChangeInfoBarService() has a NULL check inside).

I applied Colin's suggestion and factored this code in a new
InfoBarManagerFromWebContents() method, and added a comment explaining why it
can be NULL there.

+  InfoBarService* infobar_service =
+      InfoBarService::FromWebContents(new_contents);
+  if (infobar_service)
+    infobar_manager = infobar_service->infobar_manager();
+  infobar_container_->ChangeInfoBarManager(infobar_manager);
   contents_container_->SetTab(new_contents);
   UpdateDevToolsForContents(new_contents);
 
@@ -1285,7 +1289,7 @@ void BrowserWindowGtk::TabDetachedAt(WebContents* contents, int index) {
   // browser_->tab_strip_model()->GetActiveWebContents() will return NULL or
   // something else.
   if (index == browser_->tab_strip_model()->active_index()) {
-    infobar_container_->ChangeInfoBarService(NULL);
+    infobar_container_->ChangeInfoBarManager(NULL);
     UpdateDevToolsForContents(NULL);
   }
   contents_container_->DetachTab(contents);