Make TabContents own its infobar delegates....

chrome/browser/tab_contents/tab_contents.cc

 // Copyright (c) 2010 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/tab_contents/tab_contents.h"
 
 #include <cmath>
 
 #include "app/l10n_util.h"
 #include "app/resource_bundle.h"
@@ skipping 459 matching lines @@
       Source<TabContents>(this),
       NotificationService::NoDetails());
 
   // Notify any lasting InfobarDelegates that have not yet been removed that
   // whatever infobar they were handling in this TabContents has closed,
   // because the TabContents is going away entirely.
   // This must happen after the TAB_CONTENTS_DESTROYED notification as the
   // notification may trigger infobars calls that access their delegate. (and
   // some implementations of InfoBarDelegate do delete themselves on
   // InfoBarClosed()).
-  for (int i = 0; i < infobar_delegate_count(); ++i) {
-    InfoBarDelegate* delegate = GetInfoBarDelegateAt(i);
-    delegate->InfoBarClosed();
-  }
+  for (InfoBarDelegates::reverse_iterator i(infobar_delegates_.rbegin());
+       i != infobar_delegates_.rend(); ++i)
+    (*i)->InfoBarClosed();
   infobar_delegates_.clear();
 
   // TODO(brettw) this should be moved to the view.
 #if defined(OS_WIN)
   // If we still have a window handle, destroy it. GetNativeView can return
   // NULL if this contents was part of a window that closed.
   if (GetNativeView())
     ::DestroyWindow(GetNativeView());
 #endif
 
@@ skipping 565 matching lines @@
     return true;
   return false;
 }
 
 void TabContents::SetFocusToLocationBar(bool select_all) {
   if (delegate())
     delegate()->SetFocusToLocationBar(select_all);
 }
 
 void TabContents::AddInfoBar(InfoBarDelegate* delegate) {
+  DCHECK(delegate != NULL);
   if (delegate_ && !delegate_->infobars_enabled()) {
     delegate->InfoBarClosed();
     return;
   }
 
   // Look through the existing InfoBarDelegates we have for a match. If we've
   // already got one that matches, then we don't add the new one.
-  for (int i = 0; i < infobar_delegate_count(); ++i) {
-    if (GetInfoBarDelegateAt(i)->EqualsDelegate(delegate)) {
-      // Tell the new infobar to close so that it can clean itself up.
-      delegate->InfoBarClosed();
-      return;
-    }
+  InfoBarDelegates::iterator i(std::find_if(infobar_delegates_.begin(),
+    infobar_delegates_.end(),
+    std::bind2nd(std::mem_fun(&InfoBarDelegate::EqualsDelegate), delegate)));
+  if (i != infobar_delegates_.end()) {
+    // Tell the new infobar to close so that it can clean itself up.
+    delegate->InfoBarClosed();
+    return;
   }
 
   infobar_delegates_.push_back(delegate);
   NotificationService::current()->Notify(
       NotificationType::TAB_CONTENTS_INFOBAR_ADDED,
-      Source<TabContents>(this),
-      Details<InfoBarDelegate>(delegate));
+      Source<TabContents>(this), Details<InfoBarDelegate>(delegate));
 
   // Add ourselves as an observer for navigations the first time a delegate is
   // added. We use this notification to expire InfoBars that need to expire on
   // page transitions.
   if (infobar_delegates_.size() == 1) {
     registrar_.Add(this, NotificationType::NAV_ENTRY_COMMITTED,
                    Source<NavigationController>(&controller_));
   }
 }
 
 void TabContents::RemoveInfoBar(InfoBarDelegate* delegate) {
+  DCHECK(delegate != NULL);
   if (delegate_ && !delegate_->infobars_enabled()) {
+    delegate->InfoBarClosed();
     return;
   }
 
-  std::vector<InfoBarDelegate*>::iterator it =
-      find(infobar_delegates_.begin(), infobar_delegates_.end(), delegate);
-  if (it != infobar_delegates_.end()) {
-    InfoBarDelegate* delegate = *it;
-    NotificationService::current()->Notify(
-        NotificationType::TAB_CONTENTS_INFOBAR_REMOVED,
-        Source<TabContents>(this),
-        Details<InfoBarDelegate>(delegate));
+  InfoBarDelegates::iterator i(std::find(infobar_delegates_.begin(),
+                                         infobar_delegates_.end(), delegate));
+  DCHECK(i != infobar_delegates_.end());
+  NotificationService::current()->Notify(
+      NotificationType::TAB_CONTENTS_INFOBAR_REMOVED,
+      Source<TabContents>(this), Details<InfoBarDelegate>(delegate));
 
-    // Just to be safe, make sure the delegate was not removed by an observer.
-    it = find(infobar_delegates_.begin(), infobar_delegates_.end(), delegate);
-    if (it != infobar_delegates_.end()) {
-      infobar_delegates_.erase(it);
-      // Remove ourselves as an observer if we are tracking no more InfoBars.
-      if (infobar_delegates_.empty()) {
-        registrar_.Remove(this, NotificationType::NAV_ENTRY_COMMITTED,
-                          Source<NavigationController>(&controller_));
-      }
-    } else {
-      // If you hit this NOTREACHED, please comment in bug
-      // http://crbug.com/50428 how you got there.
-      NOTREACHED();
-    }
+  // Just to be safe, make sure the delegate was not removed by an observer.  If
+  // you hit this DCHECK, please comment in http://crbug.com/50428 how you got
+  // there.
+  i = std::find(infobar_delegates_.begin(), infobar_delegates_.end(), delegate);
+  DCHECK(i != infobar_delegates_.end());
+  infobar_delegates_.erase(i);
+  delegate->InfoBarClosed();
+
+  // Remove ourselves as an observer if we are tracking no more InfoBars.
+  if (infobar_delegates_.empty()) {
+    registrar_.Remove(this, NotificationType::NAV_ENTRY_COMMITTED,
+                      Source<NavigationController>(&controller_));
   }
 }
 
 void TabContents::ReplaceInfoBar(InfoBarDelegate* old_delegate,
                                  InfoBarDelegate* new_delegate) {
+  DCHECK(old_delegate != NULL);
+  DCHECK(new_delegate != NULL);
   if (delegate_ && !delegate_->infobars_enabled()) {
     new_delegate->InfoBarClosed();
     return;
   }
 
-  std::vector<InfoBarDelegate*>::iterator it =
-      find(infobar_delegates_.begin(), infobar_delegates_.end(), old_delegate);
-  DCHECK(it != infobar_delegates_.end());
+  InfoBarDelegates::iterator i(std::find(infobar_delegates_.begin(),
+      infobar_delegates_.end(), old_delegate));
+  DCHECK(i != infobar_delegates_.end());
 
   // Notify the container about the change of plans.
-  scoped_ptr<std::pair<InfoBarDelegate*, InfoBarDelegate*> > details(
-    new std::pair<InfoBarDelegate*, InfoBarDelegate*>(
-        old_delegate, new_delegate));
+  typedef std::pair<InfoBarDelegate*, InfoBarDelegate*> Delegates;
+  scoped_ptr<Delegates> details(new Delegates(old_delegate, new_delegate));
   NotificationService::current()->Notify(
       NotificationType::TAB_CONTENTS_INFOBAR_REPLACED,
-      Source<TabContents>(this),
-      Details<std::pair<InfoBarDelegate*, InfoBarDelegate*> >(details.get()));
+      Source<TabContents>(this), Details<Delegates>(details.get()));
 
-  // Just to be safe, make sure the delegate was not removed by an observer.
-  it = find(infobar_delegates_.begin(), infobar_delegates_.end(), old_delegate);
-  if (it != infobar_delegates_.end()) {
-    // Remove the old one.
-    infobar_delegates_.erase(it);
-  } else {
-    // If you hit this NOTREACHED, please comment in bug
-    // http://crbug.com/50428 how you got there.
-    NOTREACHED();
-  }
+  // Just to be safe, make sure the delegate was not removed by an observer.  If
+  // you hit this DCHECK, please comment in http://crbug.com/50428 how you got
+  // there.
+  i = std::find(infobar_delegates_.begin(), infobar_delegates_.end(),
+                old_delegate);
+  DCHECK(i != infobar_delegates_.end());
+  // Remove the old one.
+  infobar_delegates_.erase(i);
+  old_delegate->InfoBarClosed();
 
   // Add the new one.
-  DCHECK(find(infobar_delegates_.begin(),
-              infobar_delegates_.end(), new_delegate) ==
-         infobar_delegates_.end());
+  DCHECK(std::find(infobar_delegates_.begin(), infobar_delegates_.end(),
+                   new_delegate) == infobar_delegates_.end());
   infobar_delegates_.push_back(new_delegate);
 }
 
 bool TabContents::ShouldShowBookmarkBar() {
   if (showing_interstitial_page())
     return false;
 
   // Do not show bookmarks bar if bookmarks aren't enabled.
   if (!browser_defaults::bookmarks_enabled)
     return false;
 
   // See GetDOMUIForCurrentState() comment for more info. This case is very
   // similar, but for non-first loads, we want to use the committed entry. This
   // is so the bookmarks bar disappears at the same time the page does.
   if (controller_.GetLastCommittedEntry()) {
     // Not the first load, always use the committed DOM UI.
-    if (render_manager_.dom_ui())
-      return render_manager_.dom_ui()->force_bookmark_bar_visible();
-    return false;  // Default.
+    return (render_manager_.dom_ui() == NULL) ?

[tfarina, 2010/11/10 13:11:57]

This change to ternary operator just make it *less* readable. I really don't
like the change to ternary operator when it does not fit into one line. The same
thing below. Also if the check is not wrong, why change it at all? It's just
increasing the size of the diff.

+        false : render_manager_.dom_ui()->force_bookmark_bar_visible();
   }
 
   // When it's the first load, we know either the pending one or the committed
   // one will have the DOM UI in it (see GetDOMUIForCurrentState), and only one
   // of them will be valid, so we can just check both.
   if (render_manager_.pending_dom_ui())
     return render_manager_.pending_dom_ui()->force_bookmark_bar_visible();
-  if (render_manager_.dom_ui())
-    return render_manager_.dom_ui()->force_bookmark_bar_visible();
-  return false;  // Default.
+  return (render_manager_.dom_ui() == NULL) ?
+      false : render_manager_.dom_ui()->force_bookmark_bar_visible();
 }
 
 void TabContents::ToolbarSizeChanged(bool is_animating) {
   TabContentsDelegate* d = delegate();
   if (d)
     d->ToolbarSizeChanged(this, is_animating);
 }
 
 bool TabContents::CanDownload(int request_id) {
   TabContentsDelegate* d = delegate();
   if (d)
     return d->CanDownload(request_id);
   return true;
 }
 
 void TabContents::OnStartDownload(DownloadItem* download) {
   DCHECK(download);
 
   // Download in a constrained popup is shown in the tab that opened it.
   TabContents* tab_contents = delegate()->GetConstrainingContents(this);
 
   if (tab_contents && tab_contents->delegate())
     tab_contents->delegate()->OnStartDownload(download, this);
 }
 
 void TabContents::WillClose(ConstrainedWindow* window) {
-  ConstrainedWindowList::iterator it =
-      find(child_windows_.begin(), child_windows_.end(), window);
-  bool removed_topmost_window = it == child_windows_.begin();
-  if (it != child_windows_.end())
-    child_windows_.erase(it);
-  if (child_windows_.size() > 0) {
-    if (removed_topmost_window) {
+  ConstrainedWindowList::iterator i(
+      std::find(child_windows_.begin(), child_windows_.end(), window));
+  bool removed_topmost_window = (i == child_windows_.begin());
+  if (i != child_windows_.end())
+    child_windows_.erase(i);
+  if (child_windows_.empty()) {
+    BlockTabContent(false);
+  } else {
+    if (removed_topmost_window)
       child_windows_[0]->ShowConstrainedWindow();
-    }
     BlockTabContent(true);
-  } else {
-    BlockTabContent(false);
   }
 }
 
 void TabContents::WillCloseBlockedContentContainer(
     BlockedContentContainer* container) {
   DCHECK(blocked_contents_ == container);
   blocked_contents_ = NULL;
   PopupNotificationVisibilityChanged(false);
 }
 
@@ skipping 310 matching lines @@
 }
 
 void TabContents::ExpireInfoBars(
     const NavigationController::LoadCommittedDetails& details) {
   // Only hide InfoBars when the user has done something that makes the main
   // frame load. We don't want various automatic or subframe navigations making
   // it disappear.
   if (!details.is_user_initiated_main_frame_load())
     return;
 
-  for (int i = infobar_delegate_count() - 1; i >= 0; --i) {
-    InfoBarDelegate* delegate = GetInfoBarDelegateAt(i);
-    if (!delegate) {
-      // If you hit this NOTREACHED, please comment in bug
-      // http://crbug.com/50428 how you got there.
-      NOTREACHED();
-      continue;
-    }
-
-    if (delegate->ShouldExpire(details))
-      RemoveInfoBar(delegate);
+  for (InfoBarDelegates::reverse_iterator i(infobar_delegates_.rbegin());
+       i != infobar_delegates_.rend(); ++i) {
+    if ((*i)->ShouldExpire(details))
+      RemoveInfoBar(*i);
   }
 }
 
 DOMUI* TabContents::GetDOMUIForCurrentState() {
   // When there is a pending navigation entry, we want to use the pending DOMUI
   // that goes along with it to control the basic flags. For example, we want to
   // show the pending URL in the URL bar, so we want the display_url flag to
   // be from the pending entry.
   //
   // The confusion comes because there are multiple possibilities for the
@@ skipping 830 matching lines @@
   if (rvh != render_view_host()) {
     // The pending page's RenderViewHost is gone.
     return;
   }
 
   SetIsLoading(false, NULL);
   NotifyDisconnected();
   SetIsCrashed(true);
 
   // Remove all infobars.
-  for (int i = infobar_delegate_count() - 1; i >=0 ; --i)
-    RemoveInfoBar(GetInfoBarDelegateAt(i));
+  while (!infobar_delegates_.empty())

[tfarina, 2010/11/10 13:11:57]

What is wrong with the for loop? I guess this is superfluous.

+    RemoveInfoBar(infobar_delegates_.front());
 
   // Tell the view that we've crashed so it can prepare the sad tab page.
   // Only do this if we're not in browser shutdown, so that TabContents
   // objects that are not in a browser (e.g., HTML dialogs) and thus are
   // visible do not flash a sad tab page.
   if (browser_shutdown::GetShutdownType() == browser_shutdown::NOT_VALID)
     view_->OnTabCrashed();
 
   // Hide any visible hung renderer warning for this web contents' process.
   hung_renderer_dialog::HideForTabContents(this);
@@ skipping 898 matching lines @@
   AddInfoBar(new SavePasswordInfoBarDelegate(this, form_to_save));
 }
 
 Profile* TabContents::GetProfileForPasswordManager() {
   return profile();
 }
 
 bool TabContents::DidLastPageLoadEncounterSSLErrors() {
   return controller().ssl_manager()->ProcessedSSLErrorFromRequest();
 }