Change notification handling to prevent multiple 'Do you mean...?' infobars f...

chrome/browser/alternate_nav_url_fetcher.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/alternate_nav_url_fetcher.h"
 
 #include "app/l10n_util.h"
 #include "app/resource_bundle.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/intranet_redirect_detector.h"
@@ skipping 17 matching lines @@
       infobar_contents_(NULL) {
   registrar_.Add(this, NotificationType::NAV_ENTRY_PENDING,
                  NotificationService::AllSources());
 }
 
 void AlternateNavURLFetcher::Observe(NotificationType type,
                                      const NotificationSource& source,
                                      const NotificationDetails& details) {
   switch (type.value) {
     case NotificationType::NAV_ENTRY_PENDING:
-      controller_ = Source<NavigationController>(source).ptr();
-      DCHECK(controller_->pending_entry());
+      // If we've already received a notification for the same controller, we
+      // should delete ourselves as that indicates that the page is being
+      // re-loaded so this instance is now stale.
+      // http://crbug.com/43378
+      if (!infobar_contents_ &&
+          controller_ == Source<NavigationController>(source).ptr()) {
+        delete this;
+      } else if (!controller_) {

[Evan Stade, 2010/06/09 19:46:11]

can the hypothetical else be reached (where controller_ is non null, but doesn't
match source)? what would that indicated?

[csilv, 2010/06/09 20:09:22]

Yes, that can happen if some other tab sends a NAV_ENTRY_PENDING notification
since we don't stop listening for it.  In that case we just ignore the
notification which is the desired behavior.

+        controller_ = Source<NavigationController>(source).ptr();
+        DCHECK(controller_->pending_entry());
 
-      // Unregister for this notification now that we're pending, and start
-      // listening for the corresponding commit. We also need to listen for the
-      // tab close command since that means the load will never commit!
-      registrar_.Remove(this, NotificationType::NAV_ENTRY_PENDING,
-                        NotificationService::AllSources());
-      registrar_.Add(this, NotificationType::NAV_ENTRY_COMMITTED,
-                     Source<NavigationController>(controller_));
-      registrar_.Add(this, NotificationType::TAB_CLOSED,
-                     Source<NavigationController>(controller_));
+        // Start listening for the commit notification. We also need to listen
+        // for the tab close command since that means the load will never
+        // commit!
+        registrar_.Add(this, NotificationType::NAV_ENTRY_COMMITTED,
+                       Source<NavigationController>(controller_));
+        registrar_.Add(this, NotificationType::TAB_CLOSED,
+                       Source<NavigationController>(controller_));
 
-      DCHECK_EQ(NOT_STARTED, state_);
-      state_ = IN_PROGRESS;
-      fetcher_.reset(new URLFetcher(GURL(alternate_nav_url_), URLFetcher::HEAD,
-                                    this));
-      fetcher_->set_request_context(
-          controller_->profile()->GetRequestContext());
-      fetcher_->Start();
+        DCHECK_EQ(NOT_STARTED, state_);
+        state_ = IN_PROGRESS;
+        fetcher_.reset(new URLFetcher(GURL(alternate_nav_url_),
+                                      URLFetcher::HEAD, this));
+        fetcher_->set_request_context(
+            controller_->profile()->GetRequestContext());
+        fetcher_->Start();
+      }
       break;
 
     case NotificationType::NAV_ENTRY_COMMITTED:
       // The page was navigated, we can show the infobar now if necessary.
       registrar_.Remove(this, NotificationType::NAV_ENTRY_COMMITTED,
                         Source<NavigationController>(controller_));
       navigated_to_entry_ = true;
       ShowInfobarIfPossible();
       break;
 
@@ skipping 100 matching lines @@
     if (state_ == FAILED)
       delete this;
     return;
   }
 
   infobar_contents_ = controller_->tab_contents();
   StoreActiveEntryUniqueID(infobar_contents_);
   // We will be deleted when the InfoBar is destroyed. (See InfoBarClosed).
   infobar_contents_->AddInfoBar(this);
 }