Changes TabContentsDelegate::ShouldAddNavigationToHistory to be passed

the HistoryAddPageArgs so that they can be cached and used later
on. Refactors some other code that interacts with history so that it
can be called separately as well.

BUG=none
TEST=none

Review URL: http://codereview.chromium.org/3158029

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@57458 0039d316-1c4b-4281-b951-d872f2087c98

18 files changed, 217 insertions, 121 deletions

diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index e952b50..dad49e7 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -2963,7 +2963,9 @@ void Browser::ShowCollectedCookiesDialog(TabContents *tab_contents) {
   window()->ShowCollectedCookiesDialog(tab_contents);
 }
 
-bool Browser::ShouldAddNavigationsToHistory() const {
+bool Browser::ShouldAddNavigationToHistory(
+    const history::HistoryAddPageArgs& add_page_args,
+    NavigationType::Type navigation_type) {
   // Don't update history if running as app.
   return !IsApplication();
 }
diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h
index 27c6f0b..37885f5 100644
--- a/chrome/browser/browser.h
+++ b/chrome/browser/browser.h
@@ -745,7 +745,9 @@ class Browser : public TabStripModelDelegate,
   virtual void ShowRepostFormWarningDialog(TabContents* tab_contents);
   virtual void ShowContentSettingsWindow(ContentSettingsType content_type);
   virtual void ShowCollectedCookiesDialog(TabContents* tab_contents);
-  virtual bool ShouldAddNavigationsToHistory() const;
+  virtual bool ShouldAddNavigationToHistory(
+      const history::HistoryAddPageArgs& add_page_args,
+      NavigationType::Type navigation_type);
   virtual void OnDidGetApplicationInfo(TabContents* tab_contents,
                                        int32 page_id);
   virtual void ContentTypeChanged(TabContents* source);
diff --git a/chrome/browser/chromeos/login/eula_view.h b/chrome/browser/chromeos/login/eula_view.h
index 192f136..01b7c02 100644
--- a/chrome/browser/chromeos/login/eula_view.h
+++ b/chrome/browser/chromeos/login/eula_view.h
@@ -53,7 +53,11 @@ class EULATabContentsDelegate : public TabContentsDelegate {
   virtual bool IsPopup(TabContents* source) { return false; }
   virtual void URLStarredChanged(TabContents* source, bool starred) {}
   virtual void UpdateTargetURL(TabContents* source, const GURL& url) {}
-  virtual bool ShouldAddNavigationToHistory() const { return false; }
+  virtual bool ShouldAddNavigationToHistory(
+      const history::HistoryAddPageArgs& add_page_args,
+      NavigationType::Type navigation_type) {
+    return false;
+  }
   virtual void MoveContents(TabContents* source, const gfx::Rect& pos) {}
   virtual void ToolbarSizeChanged(TabContents* source, bool is_animating) {}
   virtual bool HandleContextMenu(const ContextMenuParams& params) {
diff --git a/chrome/browser/chromeos/login/web_page_screen.h b/chrome/browser/chromeos/login/web_page_screen.h
index 2cb4583..13c074a 100644
--- a/chrome/browser/chromeos/login/web_page_screen.h
+++ b/chrome/browser/chromeos/login/web_page_screen.h
@@ -43,7 +43,11 @@ class WebPageScreen : public TabContentsDelegate {
   virtual bool IsPopup(TabContents* source) { return false; }
   virtual void URLStarredChanged(TabContents* source, bool starred) {}
   virtual void UpdateTargetURL(TabContents* source, const GURL& url) {}
-  virtual bool ShouldAddNavigationToHistory() const { return false; }
+  virtual bool ShouldAddNavigationToHistory(
+      const history::HistoryAddPageArgs& add_page_args,
+      NavigationType::Type navigation_type) {
+    return false;
+  }
   virtual void MoveContents(TabContents* source, const gfx::Rect& pos) {}
   virtual void ToolbarSizeChanged(TabContents* source, bool is_animating) {}
   virtual bool HandleContextMenu(const ContextMenuParams& params);
diff --git a/chrome/browser/dom_ui/html_dialog_tab_contents_delegate.cc b/chrome/browser/dom_ui/html_dialog_tab_contents_delegate.cc
index be32f7f..92772c4 100644
--- a/chrome/browser/dom_ui/html_dialog_tab_contents_delegate.cc
+++ b/chrome/browser/dom_ui/html_dialog_tab_contents_delegate.cc
@@ -107,7 +107,9 @@ void HtmlDialogTabContentsDelegate::UpdateTargetURL(TabContents* source,
   // Ignored.
 }
 
-bool HtmlDialogTabContentsDelegate::ShouldAddNavigationToHistory() const {
+bool HtmlDialogTabContentsDelegate::ShouldAddNavigationToHistory(
+    const history::HistoryAddPageArgs& add_page_args,
+    NavigationType::Type navigation_type) {
   return false;
 }
 
diff --git a/chrome/browser/dom_ui/html_dialog_tab_contents_delegate.h b/chrome/browser/dom_ui/html_dialog_tab_contents_delegate.h
index 4b68a9f..b0ab266 100644
--- a/chrome/browser/dom_ui/html_dialog_tab_contents_delegate.h
+++ b/chrome/browser/dom_ui/html_dialog_tab_contents_delegate.h
@@ -54,7 +54,9 @@ class HtmlDialogTabContentsDelegate : public TabContentsDelegate {
   virtual bool IsPopup(const TabContents* source) const;
   virtual void URLStarredChanged(TabContents* source, bool starred);
   virtual void UpdateTargetURL(TabContents* source, const GURL& url);
-  virtual bool ShouldAddNavigationToHistory() const;
+  virtual bool ShouldAddNavigationToHistory(
+      const history::HistoryAddPageArgs& add_page_args,
+      NavigationType::Type navigation_type);
 
  protected:
   // Overridden only for testing.
diff --git a/chrome/browser/dom_ui/html_dialog_tab_contents_delegate_unittest.cc b/chrome/browser/dom_ui/html_dialog_tab_contents_delegate_unittest.cc
index 8e1b7ed..7f487fc 100644
--- a/chrome/browser/dom_ui/html_dialog_tab_contents_delegate_unittest.cc
+++ b/chrome/browser/dom_ui/html_dialog_tab_contents_delegate_unittest.cc
@@ -10,6 +10,7 @@
 #include "base/scoped_ptr.h"
 #include "chrome/browser/browser.h"
 #include "chrome/browser/browser_list.h"
+#include "chrome/browser/history/history_types.h"
 #include "chrome/browser/tab_contents/test_tab_contents.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/browser_with_test_window_test.h"
@@ -101,7 +102,12 @@ class HtmlDialogTabContentsDelegateTest : public BrowserWithTestWindowTest {
 TEST_F(HtmlDialogTabContentsDelegateTest, DoNothingMethodsTest) {
   // None of the following calls should do anything.
   EXPECT_TRUE(test_tab_contents_delegate_->IsPopup(NULL));
-  EXPECT_FALSE(test_tab_contents_delegate_->ShouldAddNavigationToHistory());
+  scoped_refptr<history::HistoryAddPageArgs> should_add_args(
+      new history::HistoryAddPageArgs(
+          GURL(), base::Time::Now(), 0, 0, GURL(), history::RedirectList(),
+          PageTransition::TYPED, history::SOURCE_SYNCED, false));
+  EXPECT_FALSE(test_tab_contents_delegate_->ShouldAddNavigationToHistory(
+                   *should_add_args, NavigationType::NEW_PAGE));
   test_tab_contents_delegate_->NavigationStateChanged(NULL, 0);
   test_tab_contents_delegate_->ActivateContents(NULL);
   test_tab_contents_delegate_->LoadingStateChanged(NULL);
diff --git a/chrome/browser/external_tab_container_win.cc b/chrome/browser/external_tab_container_win.cc
index ba7202b..d3a7684 100644
--- a/chrome/browser/external_tab_container_win.cc
+++ b/chrome/browser/external_tab_container_win.cc
@@ -16,6 +16,7 @@
 #include "chrome/browser/browser_window.h"
 #include "chrome/browser/debugger/devtools_manager.h"
 #include "chrome/browser/debugger/devtools_toggle_action.h"
+#include "chrome/browser/history/history_types.h"
 #include "chrome/browser/load_notification_details.h"
 #include "chrome/browser/page_info_window.h"
 #include "chrome/browser/profile.h"
@@ -283,10 +284,10 @@ ExternalTabContainer*
 // ExternalTabContainer, TabContentsDelegate implementation:
 
 void ExternalTabContainer::OpenURLFromTab(TabContents* source,
-                           const GURL& url,
-                           const GURL& referrer,
-                           WindowOpenDisposition disposition,
-                           PageTransition::Type transition) {
+                                          const GURL& url,
+                                          const GURL& referrer,
+                                          WindowOpenDisposition disposition,
+                                          PageTransition::Type transition) {
   if (pending()) {
     PendingTopLevelNavigation url_request;
     url_request.disposition = disposition;
@@ -323,7 +324,9 @@ void ExternalTabContainer::OpenURLFromTab(TabContents* source,
         NavigationController::LoadCommittedDetails details;
         details.did_replace_entry = false;
 
-        tab_contents_->UpdateHistoryForNavigation(url, details, params);
+        scoped_refptr<history::HistoryAddPageArgs> add_page_args(
+            tab_contents_->CreateHistoryAddPageArgs(url, details, params));
+        tab_contents_->UpdateHistoryForNavigation(add_page_args);
       }
       break;
     default:
diff --git a/chrome/browser/history/history.cc b/chrome/browser/history/history.cc
index eb27b97..bc9c8e5 100644
--- a/chrome/browser/history/history.cc
+++ b/chrome/browser/history/history.cc
@@ -316,36 +316,43 @@ void HistoryService::AddPage(const GURL& url,
                              const history::RedirectList& redirects,
                              history::VisitSource visit_source,
                              bool did_replace_entry) {
+  scoped_refptr<history::HistoryAddPageArgs> request(
+      new history::HistoryAddPageArgs(url, time, id_scope, page_id, referrer,
+                                      redirects, transition, visit_source,
+                                      did_replace_entry));
+  AddPage(*request);
+}
+
+void HistoryService::AddPage(const history::HistoryAddPageArgs& add_page_args) {
   DCHECK(thread_) << "History service being called after cleanup";
 
   // Filter out unwanted URLs. We don't add auto-subframe URLs. They are a
   // large part of history (think iframes for ads) and we never display them in
   // history UI. We will still add manual subframes, which are ones the user
   // has clicked on to get.
-  if (!CanAddURL(url))
+  if (!CanAddURL(add_page_args.url))
     return;
 
   // Add link & all redirects to visited link list.
   VisitedLinkMaster* visited_links;
   if (profile_ && (visited_links = profile_->GetVisitedLinkMaster())) {
-    visited_links->AddURL(url);
+    visited_links->AddURL(add_page_args.url);
 
-    if (!redirects.empty()) {
+    if (!add_page_args.redirects.empty()) {
       // We should not be asked to add a page in the middle of a redirect chain.
-      DCHECK(redirects[redirects.size() - 1] == url);
+      DCHECK_EQ(add_page_args.url,
+                add_page_args.redirects[add_page_args.redirects.size() - 1]);
 
       // We need the !redirects.empty() condition above since size_t is unsigned
       // and will wrap around when we subtract one from a 0 size.
-      for (size_t i = 0; i < redirects.size() - 1; i++)
-        visited_links->AddURL(redirects[i]);
+      for (size_t i = 0; i < add_page_args.redirects.size() - 1; i++)
+        visited_links->AddURL(add_page_args.redirects[i]);
     }
   }
 
-  scoped_refptr<history::HistoryAddPageArgs> request(
-      new history::HistoryAddPageArgs(url, time, id_scope, page_id, referrer,
-                                      redirects, transition, visit_source,
-                                      did_replace_entry));
-  ScheduleAndForget(PRIORITY_NORMAL, &HistoryBackend::AddPage, request);
+  ScheduleAndForget(PRIORITY_NORMAL, &HistoryBackend::AddPage,
+                    scoped_refptr<history::HistoryAddPageArgs>(
+                        add_page_args.Clone()));
 }
 
 void HistoryService::SetPageTitle(const GURL& url,
diff --git a/chrome/browser/history/history.h b/chrome/browser/history/history.h
index 5bc3b7f..560db1a 100644
--- a/chrome/browser/history/history.h
+++ b/chrome/browser/history/history.h
@@ -48,15 +48,14 @@ class TypedUrlDataTypeController;
 }
 
 namespace history {
-
 class InMemoryHistoryBackend;
 class InMemoryURLIndex;
+class HistoryAddPageArgs;
 class HistoryBackend;
 class HistoryDatabase;
 struct HistoryDetails;
 class HistoryQueryTest;
 class URLDatabase;
-
 }  // namespace history
 
 
@@ -206,6 +205,9 @@ class HistoryService : public CancelableRequestProvider,
             history::RedirectList(), visit_source, false);
   }
 
+  // All AddPage variants end up here.
+  void AddPage(const history::HistoryAddPageArgs& add_page_args);
+
   // Sets the title for the given page. The page should be in history. If it
   // is not, this operation is ignored. This call will not update the full
   // text index. The last title set when the page is indexed will be the
diff --git a/chrome/browser/history/history_marshaling.h b/chrome/browser/history/history_marshaling.h
index e746d20..1efd670 100644
--- a/chrome/browser/history/history_marshaling.h
+++ b/chrome/browser/history/history_marshaling.h
@@ -17,52 +17,6 @@
 
 namespace history {
 
-// Navigation -----------------------------------------------------------------
-
-// Marshalling structure for AddPage.
-class HistoryAddPageArgs
-    : public base::RefCountedThreadSafe<HistoryAddPageArgs> {
- public:
-  HistoryAddPageArgs(const GURL& arg_url,
-                     base::Time arg_time,
-                     const void* arg_id_scope,
-                     int32 arg_page_id,
-                     const GURL& arg_referrer,
-                     const history::RedirectList& arg_redirects,
-                     PageTransition::Type arg_transition,
-                     VisitSource arg_source,
-                     bool arg_did_replace_entry)
-      : url(arg_url),
-        time(arg_time),
-        id_scope(arg_id_scope),
-        page_id(arg_page_id),
-        referrer(arg_referrer),
-        redirects(arg_redirects),
-        transition(arg_transition),
-        visit_source(arg_source),
-        did_replace_entry(arg_did_replace_entry) {
-  }
-
-  GURL url;
-  base::Time time;
-
-  const void* id_scope;
-  int32 page_id;
-
-  GURL referrer;
-  history::RedirectList redirects;
-  PageTransition::Type transition;
-  VisitSource visit_source;
-  bool did_replace_entry;
-
- private:
-  friend class base::RefCountedThreadSafe<HistoryAddPageArgs>;
-
-  ~HistoryAddPageArgs() {}
-
-  DISALLOW_COPY_AND_ASSIGN(HistoryAddPageArgs);
-};
-
 // Querying -------------------------------------------------------------------
 
 typedef CancelableRequest1<HistoryService::QueryURLCallback,
diff --git a/chrome/browser/history/history_types.cc b/chrome/browser/history/history_types.cc
index 6454df6..4f67beb 100644
--- a/chrome/browser/history/history_types.cc
+++ b/chrome/browser/history/history_types.cc
@@ -238,4 +238,31 @@ void QueryResults::AdjustResultMap(size_t begin, size_t end, ptrdiff_t delta) {
   }
 }
 
+HistoryAddPageArgs::HistoryAddPageArgs(
+    const GURL& arg_url,
+    base::Time arg_time,
+    const void* arg_id_scope,
+    int32 arg_page_id,
+    const GURL& arg_referrer,
+    const history::RedirectList& arg_redirects,
+    PageTransition::Type arg_transition,
+    VisitSource arg_source,
+    bool arg_did_replace_entry)
+      : url(arg_url),
+        time(arg_time),
+        id_scope(arg_id_scope),
+        page_id(arg_page_id),
+        referrer(arg_referrer),
+        redirects(arg_redirects),
+        transition(arg_transition),
+        visit_source(arg_source),
+        did_replace_entry(arg_did_replace_entry) {
+}
+
+HistoryAddPageArgs* HistoryAddPageArgs::Clone() const {
+  return new HistoryAddPageArgs(
+      url, time, id_scope, page_id, referrer, redirects, transition,
+      visit_source, did_replace_entry);
+}
+
 }  // namespace history
diff --git a/chrome/browser/history/history_types.h b/chrome/browser/history/history_types.h
index 30e3fa4..6c10bfc 100644
--- a/chrome/browser/history/history_types.h
+++ b/chrome/browser/history/history_types.h
@@ -558,6 +558,46 @@ struct Images {
 
 typedef std::vector<MostVisitedURL> MostVisitedURLList;
 
+// Navigation -----------------------------------------------------------------
+
+// Marshalling structure for AddPage.
+class HistoryAddPageArgs
+    : public base::RefCountedThreadSafe<HistoryAddPageArgs> {
+ public:
+  HistoryAddPageArgs(const GURL& arg_url,
+                     base::Time arg_time,
+                     const void* arg_id_scope,
+                     int32 arg_page_id,
+                     const GURL& arg_referrer,
+                     const history::RedirectList& arg_redirects,
+                     PageTransition::Type arg_transition,
+                     VisitSource arg_source,
+                     bool arg_did_replace_entry);
+
+  // Returns a new HistoryAddPageArgs that is a copy of this (ref count is
+  // of course reset). Ownership of returned object passes to caller.
+  HistoryAddPageArgs* Clone() const;
+
+  GURL url;
+  base::Time time;
+
+  const void* id_scope;
+  int32 page_id;
+
+  GURL referrer;
+  history::RedirectList redirects;
+  PageTransition::Type transition;
+  VisitSource visit_source;
+  bool did_replace_entry;
+
+ private:
+  friend class base::RefCountedThreadSafe<HistoryAddPageArgs>;
+
+  ~HistoryAddPageArgs() {}
+
+  DISALLOW_COPY_AND_ASSIGN(HistoryAddPageArgs);
+};
+
 }  // namespace history
 
 #endif  // CHROME_BROWSER_HISTORY_HISTORY_TYPES_H_
diff --git a/chrome/browser/tab_contents/match_preview.cc b/chrome/browser/tab_contents/match_preview.cc
index 4a05856..0b742db 100644
--- a/chrome/browser/tab_contents/match_preview.cc
+++ b/chrome/browser/tab_contents/match_preview.cc
@@ -108,7 +108,11 @@ class MatchPreview::TabContentsDelegateImpl : public TabContentsDelegate {
   virtual void ShowContentSettingsWindow(ContentSettingsType content_type) {}
   virtual void ShowCollectedCookiesDialog(TabContents* tab_contents) {}
   virtual bool OnGoToEntryOffset(int offset) { return false; }
-  virtual bool ShouldAddNavigationsToHistory() const { return false; }
+  virtual bool ShouldAddNavigationToHistory(
+      const history::HistoryAddPageArgs& add_page_args,
+      NavigationType::Type navigation_type) {
+    return false;
+  }
   virtual void OnDidGetApplicationInfo(TabContents* tab_contents,
                                        int32 page_id) {}
   virtual gfx::NativeWindow GetFrameNativeWindow() {
diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index 4e6709a..d0bcf56 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -37,6 +37,7 @@
 #include "chrome/browser/download/download_request_limiter.h"
 #include "chrome/browser/external_protocol_handler.h"
 #include "chrome/browser/extensions/extensions_service.h"
+#include "chrome/browser/history/history_types.h"
 #include "chrome/browser/history/top_sites.h"
 #include "chrome/browser/favicon_service.h"
 #include "chrome/browser/find_bar_state.h"
@@ -1449,6 +1450,26 @@ TabContents* TabContents::CloneAndMakePhantom() {
   return new_contents;
 }
 
+void TabContents::UpdateHistoryForNavigation(
+    scoped_refptr<history::HistoryAddPageArgs> add_page_args) {
+  if (profile()->IsOffTheRecord())
+    return;
+
+  // Add to history service.
+  HistoryService* hs = profile()->GetHistoryService(Profile::IMPLICIT_ACCESS);
+  if (hs)
+    hs->AddPage(*add_page_args);
+}
+
+void TabContents::UpdateHistoryPageTitle(const NavigationEntry& entry) {
+  if (profile()->IsOffTheRecord())
+    return;
+
+  HistoryService* hs = profile()->GetHistoryService(Profile::IMPLICIT_ACCESS);
+  if (hs)
+    hs->SetPageTitle(entry.virtual_url(), entry.title());
+}
+
 // Notifies the RenderWidgetHost instance about the fact that the page is
 // loading, or done loading and calls the base implementation.
 void TabContents::SetIsLoading(bool is_loading,
@@ -1750,36 +1771,29 @@ void TabContents::UpdateMaxPageIDIfNecessary(SiteInstance* site_instance,
   }
 }
 
-void TabContents::UpdateHistoryForNavigation(
+scoped_refptr<history::HistoryAddPageArgs>
+TabContents::CreateHistoryAddPageArgs(
     const GURL& virtual_url,
     const NavigationController::LoadCommittedDetails& details,
     const ViewHostMsg_FrameNavigate_Params& params) {
-  if (profile()->IsOffTheRecord())
-    return;
-
-  // Add to history service.
-  HistoryService* hs = profile()->GetHistoryService(Profile::IMPLICIT_ACCESS);
-  if (hs) {
-    if (PageTransition::IsMainFrame(params.transition) &&
-        virtual_url != params.url) {
-      // Hack on the "virtual" URL so that it will appear in history. For some
-      // types of URLs, we will display a magic URL that is different from where
-      // the page is actually navigated. We want the user to see in history
-      // what they saw in the URL bar, so we add the virtual URL as a redirect.
-      // This only applies to the main frame, as the virtual URL doesn't apply
-      // to sub-frames.
-      std::vector<GURL> redirects = params.redirects;
-      if (!redirects.empty())
-        redirects.back() = virtual_url;
-      hs->AddPage(virtual_url, this, params.page_id, params.referrer,
-                  params.transition, redirects, history::SOURCE_BROWSED,
-                  details.did_replace_entry);
-    } else {
-      hs->AddPage(params.url, this, params.page_id, params.referrer,
-                  params.transition, params.redirects, history::SOURCE_BROWSED,
-                  details.did_replace_entry);
-    }
-  }
+  scoped_refptr<history::HistoryAddPageArgs> add_page_args(
+      new history::HistoryAddPageArgs(
+          params.url, base::Time::Now(), this, params.page_id, params.referrer,
+          params.redirects, params.transition, history::SOURCE_BROWSED,
+          details.did_replace_entry));
+  if (PageTransition::IsMainFrame(params.transition) &&
+      virtual_url != params.url) {
+    // Hack on the "virtual" URL so that it will appear in history. For some
+    // types of URLs, we will display a magic URL that is different from where
+    // the page is actually navigated. We want the user to see in history what
+    // they saw in the URL bar, so we add the virtual URL as a redirect.  This
+    // only applies to the main frame, as the virtual URL doesn't apply to
+    // sub-frames.
+    add_page_args->url = virtual_url;
+    if (!add_page_args->redirects.empty())
+      add_page_args->redirects.back() = virtual_url;
+  }
+  return add_page_args;
 }
 
 bool TabContents::UpdateTitleForEntry(NavigationEntry* entry,
@@ -1802,14 +1816,8 @@ bool TabContents::UpdateTitleForEntry(NavigationEntry* entry,
 
   entry->set_title(final_title);
 
-  // Update the history system for this page.
-  if (!profile()->IsOffTheRecord() && !received_page_title_) {
-    HistoryService* hs =
-        profile()->GetHistoryService(Profile::IMPLICIT_ACCESS);
-    if (hs)
-      hs->SetPageTitle(entry->virtual_url(), final_title);
-
-    // Don't allow the title to be saved again for explicitly set ones.
+  if (!received_page_title_) {
+    UpdateHistoryPageTitle(*entry);
     received_page_title_ = explicit_set;
   }
 
@@ -2414,13 +2422,18 @@ void TabContents::DidNavigate(RenderViewHost* rvh,
 
   // Update history. Note that this needs to happen after the entry is complete,
   // which WillNavigate[Main,Sub]Frame will do before this function is called.
-  if (params.should_update_history &&
-      (!delegate() || delegate()->ShouldAddNavigationsToHistory())) {
+  if (params.should_update_history) {
     // Most of the time, the displayURL matches the loaded URL, but for about:
-    // URLs, we use a data: URL as the real value.  We actually want to save
-    // the about: URL to the history db and keep the data: URL hidden. This is
-    // what the TabContents' URL getter does.
-    UpdateHistoryForNavigation(GetURL(), details, params);
+    // URLs, we use a data: URL as the real value.  We actually want to save the
+    // about: URL to the history db and keep the data: URL hidden. This is what
+    // the TabContents' URL getter does.
+    scoped_refptr<history::HistoryAddPageArgs> add_page_args(
+        CreateHistoryAddPageArgs(GetURL(), details, params));
+    if (!delegate() ||
+        delegate()->ShouldAddNavigationToHistory(*add_page_args,
+                                                 details.type)) {
+      UpdateHistoryForNavigation(add_page_args);
+    }
   }
 
   if (!did_navigate)
@@ -2468,7 +2481,7 @@ void TabContents::UpdateTitle(RenderViewHost* rvh,
 
   DCHECK(rvh == render_view_host());
   NavigationEntry* entry = controller_.GetEntryWithPageID(GetSiteInstance(),
-                                                            page_id);
+                                                          page_id);
   if (!entry || !UpdateTitleForEntry(entry, title))
     return;
 
diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h
index 25b32eb..113bc72 100644
--- a/chrome/browser/tab_contents/tab_contents.h
+++ b/chrome/browser/tab_contents/tab_contents.h
@@ -46,6 +46,10 @@ namespace gfx {
 class Rect;
 }
 
+namespace history {
+class HistoryAddPageArgs;
+}
+
 namespace printing {
 class PrintViewManager;
 }
@@ -696,6 +700,15 @@ class TabContents : public PageNavigator,
   virtual Profile* GetProfileForPasswordManager();
   virtual bool DidLastPageLoadEncounterSSLErrors();
 
+  // Updates history with the specified navigation. This is called by
+  // OnMsgNavigate to update history state.
+  void UpdateHistoryForNavigation(
+      scoped_refptr<history::HistoryAddPageArgs> add_page_args);
+
+  // Sends the page title to the history service. This is called when we receive
+  // the page title and we know we want to update history.
+  void UpdateHistoryPageTitle(const NavigationEntry& entry);
+
  private:
   friend class NavigationController;
   // Used to access the child_windows_ (ConstrainedWindowList) for testing
@@ -722,7 +735,7 @@ class TabContents : public PageNavigator,
   // TODO(brettw) TestTabContents shouldn't exist!
   friend class TestTabContents;
 
-  // Used to access the UpdateHistoryForNavigation member function.
+  // Used to access the CreateHistoryAddPageArgs member function.
   friend class ExternalTabContainer;
 
   // Changes the IsLoading state and notifies delegate as needed
@@ -788,9 +801,10 @@ class TabContents : public PageNavigator,
   void UpdateMaxPageIDIfNecessary(SiteInstance* site_instance,
                                   RenderViewHost* rvh);
 
-  // Called by OnMsgNavigate to update history state. Overridden by subclasses
-  // that don't want to be added to history.
-  virtual void UpdateHistoryForNavigation(const GURL& virtual_url,
+  // Returns the history::HistoryAddPageArgs to use for adding a page to
+  // history.
+  scoped_refptr<history::HistoryAddPageArgs> CreateHistoryAddPageArgs(
+      const GURL& virtual_url,
       const NavigationController::LoadCommittedDetails& details,
       const ViewHostMsg_FrameNavigate_Params& params);
 
diff --git a/chrome/browser/tab_contents/tab_contents_delegate.cc b/chrome/browser/tab_contents/tab_contents_delegate.cc
index aca5fc6..2735a2c 100644
--- a/chrome/browser/tab_contents/tab_contents_delegate.cc
+++ b/chrome/browser/tab_contents/tab_contents_delegate.cc
@@ -134,7 +134,9 @@ bool TabContentsDelegate::OnGoToEntryOffset(int offset) {
   return true;
 }
 
-bool TabContentsDelegate::ShouldAddNavigationsToHistory() const {
+bool TabContentsDelegate::ShouldAddNavigationToHistory(
+    const history::HistoryAddPageArgs& add_page_args,
+    NavigationType::Type navigation_type) {
   return true;
 }
 
diff --git a/chrome/browser/tab_contents/tab_contents_delegate.h b/chrome/browser/tab_contents/tab_contents_delegate.h
index 921c9a7..629a8d4 100644
--- a/chrome/browser/tab_contents/tab_contents_delegate.h
+++ b/chrome/browser/tab_contents/tab_contents_delegate.h
@@ -12,6 +12,7 @@
 #include "chrome/browser/automation/automation_resource_routing_delegate.h"
 #include "chrome/browser/tab_contents/navigation_entry.h"
 #include "chrome/common/content_settings_types.h"
+#include "chrome/common/navigation_types.h"
 #include "chrome/common/page_transition_types.h"
 #include "gfx/native_widget_types.h"
 #include "webkit/glue/window_open_disposition.h"
@@ -22,6 +23,10 @@ class Rect;
 class Size;
 }
 
+namespace history {
+class HistoryAddPageArgs;
+}
+
 class DownloadItem;
 class ExtensionFunctionDispatcher;
 class GURL;
@@ -245,8 +250,11 @@ class TabContentsDelegate : public AutomationResourceRoutingDelegate {
   // Returns true to allow TabContents to continue with the default processing.
   virtual bool OnGoToEntryOffset(int offset);
 
-  // Returns whether this tab contents should add navigations to history.
-  virtual bool ShouldAddNavigationsToHistory() const;
+  // Returns whether this tab contents should add the specified navigation to
+  // history.
+  virtual bool ShouldAddNavigationToHistory(
+      const history::HistoryAddPageArgs& add_page_args,
+      NavigationType::Type navigation_type);
 
   // Notification when web app info data is available
   virtual void OnDidGetApplicationInfo(TabContents* tab_contents,