Allow the NTP to be bookmarked.

BUG=none
TEST=Try to bookmark the NTP.
Review URL: http://codereview.chromium.org/269028

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

4 files changed, 16 insertions, 42 deletions

diff --git a/chrome/browser/bookmarks/bookmark_utils.cc b/chrome/browser/bookmarks/bookmark_utils.cc
index edc3a46..e64c42d 100644
--- a/chrome/browser/bookmarks/bookmark_utils.cc
+++ b/chrome/browser/bookmarks/bookmark_utils.cc
@@ -611,19 +611,11 @@ void RegisterUserPrefs(PrefService* prefs) {
   prefs->RegisterIntegerPref(prefs::kBookmarkTablePathWidth, -1);
 }
 
-bool GetURLAndTitleToBookmark(TabContents* tab_contents,
+void GetURLAndTitleToBookmark(TabContents* tab_contents,
                               GURL* url,
                               std::wstring* title) {
-  if (!tab_contents || !tab_contents->ShouldDisplayURL())
-    return false;
-  GURL tab_url = tab_contents->GetURL();
-  if (!tab_url.is_valid())
-    return false;
-  if (url != NULL)
-    *url = tab_url;
-  if (title != NULL)
-    *title = UTF16ToWideHack(tab_contents->GetTitle());
-  return true;
+  *url = tab_contents->GetURL();
+  *title = UTF16ToWideHack(tab_contents->GetTitle());
 }
 
 const BookmarkNode* CreateBookmarkForAllTabs(Browser* browser) {
@@ -636,8 +628,8 @@ const BookmarkNode* CreateBookmarkForAllTabs(Browser* browser) {
   for (int i = 0; i < browser->tab_count(); ++i) {
     GURL url;
     std::wstring title;
-    if (GetURLAndTitleToBookmark(browser->GetTabContentsAt(i), &url, &title))
-      model->AddURL(folder, folder->GetChildCount(), title, url);
+    GetURLAndTitleToBookmark(browser->GetTabContentsAt(i), &url, &title);
+    model->AddURL(folder, folder->GetChildCount(), title, url);
   }
   return folder;
 }
diff --git a/chrome/browser/bookmarks/bookmark_utils.h b/chrome/browser/bookmarks/bookmark_utils.h
index 0b5718d..116d41f 100644
--- a/chrome/browser/bookmarks/bookmark_utils.h
+++ b/chrome/browser/bookmarks/bookmark_utils.h
@@ -182,9 +182,8 @@ void RegisterPrefs(PrefService* prefs);
 // Register user prefs for BookmarkBar, BookmarkView, ...
 void RegisterUserPrefs(PrefService* prefs);
 
-// Returns whether |tab_contents| can be bookmarked.  If it can, |url| and
-// |title| are filled in.  Any of the parameters may be NULL.
-bool GetURLAndTitleToBookmark(TabContents* tab_contents,
+// Fills in the URL and title for a bookmark of |tab_contents|.
+void GetURLAndTitleToBookmark(TabContents* tab_contents,
                               GURL* url,
                               std::wstring* title);
 
diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index eb63048..9920e26 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -897,16 +897,14 @@ void Browser::Exit() {
 void Browser::BookmarkCurrentPage() {
   UserMetrics::RecordAction(L"Star", profile_);
 
-  TabContents* contents = GetSelectedTabContents();
-  GURL url;
-  std::wstring title;
-  if (!bookmark_utils::GetURLAndTitleToBookmark(contents, &url, &title))
-    return;
-
-  BookmarkModel* model = contents->profile()->GetBookmarkModel();
+  BookmarkModel* model = profile()->GetBookmarkModel();
   if (!model || !model->IsLoaded())
     return;  // Ignore requests until bookmarks are loaded.
 
+  GURL url;
+  std::wstring title;
+  bookmark_utils::GetURLAndTitleToBookmark(GetSelectedTabContents(), &url,
+                                           &title);
   bool was_bookmarked = model->IsBookmarked(url);
   model->SetURLStarred(url, title, true);
   if (window_->IsActive()) {
@@ -1682,15 +1680,7 @@ bool Browser::CanCloseContentsAt(int index) {
 
 bool Browser::CanBookmarkAllTabs() const {
   BookmarkModel* model = profile()->GetBookmarkModel();
-  if (!model || !model->IsLoaded())
-    return false;
-
-  int bookmarkable_tab_contents = 0;
-  for (int i = 0; i < tab_count(); ++i) {
-    if (CanBookmarkTabContents(GetTabContentsAt(i)))
-      ++bookmarkable_tab_contents;
-  }
-  return (bookmarkable_tab_contents > 1);
+  return (model && model->IsLoaded() && (tab_count() > 1));
 }
 
 void Browser::BookmarkAllTabs() {
@@ -2433,6 +2423,9 @@ void Browser::InitCommandState() {
     command_updater_.UpdateCommandEnabled(IDC_RESTORE_TAB,
         normal_window && !profile_->IsOffTheRecord());
 
+  // Page-related commands
+    command_updater_.UpdateCommandEnabled(IDC_BOOKMARK_PAGE, normal_window);
+
     // Show various bits of UI
     command_updater_.UpdateCommandEnabled(IDC_CLEAR_BROWSING_DATA,
                                           normal_window);
@@ -2464,8 +2457,6 @@ void Browser::UpdateCommandsForTabState() {
 
   // Page-related commands
   window_->SetStarredState(current_tab->is_starred());
-  command_updater_.UpdateCommandEnabled(IDC_BOOKMARK_PAGE,
-                                        CanBookmarkTabContents(current_tab));
   command_updater_.UpdateCommandEnabled(IDC_BOOKMARK_ALL_TABS,
                                         CanBookmarkAllTabs());
 
@@ -2498,11 +2489,6 @@ void Browser::UpdateStopGoState(bool is_loading, bool force) {
   command_updater_.UpdateCommandEnabled(IDC_STOP, is_loading);
 }
 
-bool Browser::CanBookmarkTabContents(TabContents* tab_contents) const {
-  return (type() == TYPE_NORMAL) &&
-      bookmark_utils::GetURLAndTitleToBookmark(tab_contents, NULL, NULL);
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 // Browser, UI update coalescing and handling (private):
 
diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h
index 715559a..dc4b808 100644
--- a/chrome/browser/browser.h
+++ b/chrome/browser/browser.h
@@ -564,9 +564,6 @@ class Browser : public TabStripModelDelegate,
   // loading.  |force| is true if the button should change its icon immediately.
   void UpdateStopGoState(bool is_loading, bool force);
 
-  // Returns true if the specified tab contents can be bookmarked.
-  bool CanBookmarkTabContents(TabContents* tab_contents) const;
-
   // UI update coalescing and handling ////////////////////////////////////////
 
   // Asks the toolbar (and as such the location bar) to update its state to