* Add ctrl-shift-d to bookmark all tabs.

* Enable or disable bookmark commands based on whether we're actually going to be able to bookmark anything.
* Rename IDC_STAR to IDC_BOOKMARK_PAGE, which is more obvious.

BUG=2935
TEST=Hit ctrl-shift-d, make sure it bookmarks all tabs
Review URL: http://codereview.chromium.org/266029

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

2 files changed, 12 insertions, 13 deletions

diff --git a/chrome/browser/bookmarks/bookmark_utils.cc b/chrome/browser/bookmarks/bookmark_utils.cc
index 95cfe64..edc3a46 100644
--- a/chrome/browser/bookmarks/bookmark_utils.cc
+++ b/chrome/browser/bookmarks/bookmark_utils.cc
@@ -614,20 +614,21 @@ void RegisterUserPrefs(PrefService* prefs) {
 bool GetURLAndTitleToBookmark(TabContents* tab_contents,
                               GURL* url,
                               std::wstring* title) {
-  if (!tab_contents->ShouldDisplayURL())
+  if (!tab_contents || !tab_contents->ShouldDisplayURL())
     return false;
-  *url = tab_contents->GetURL();
-  if (url->is_empty() || !url->is_valid())
+  GURL tab_url = tab_contents->GetURL();
+  if (!tab_url.is_valid())
     return false;
-  *title = UTF16ToWideHack(tab_contents->GetTitle());
+  if (url != NULL)
+    *url = tab_url;
+  if (title != NULL)
+    *title = UTF16ToWideHack(tab_contents->GetTitle());
   return true;
 }
 
 const BookmarkNode* CreateBookmarkForAllTabs(Browser* browser) {
   BookmarkModel* model = browser->profile()->GetBookmarkModel();
-  if (!model || !model->IsLoaded())
-    return NULL;
-
+  DCHECK(model && model->IsLoaded());
   const BookmarkNode* parent = model->GetParentForNewNodes();
   const BookmarkNode* folder = model->AddGroup(
       parent, parent->GetChildCount(),
@@ -635,9 +636,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)) {
+    if (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 d45ecad..0b5718d 100644
--- a/chrome/browser/bookmarks/bookmark_utils.h
+++ b/chrome/browser/bookmarks/bookmark_utils.h
@@ -182,15 +182,14 @@ void RegisterPrefs(PrefService* prefs);
 // Register user prefs for BookmarkBar, BookmarkView, ...
 void RegisterUserPrefs(PrefService* prefs);
 
-// Gets the url and title to use in creating a bookmark for the specified
-// TabContents. Returns false if a bookmark shouldn't be created for the
-// specified TabContents.
+// 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,
                               GURL* url,
                               std::wstring* title);
 
 // Creates a new folder containing a bookmark for each of the tabs in
-// |browser|. This returns null if the bookmark model isn't loaded.
+// |browser|.
 const BookmarkNode* CreateBookmarkForAllTabs(Browser* browser);
 
 // Number of bookmarks we'll open before prompting the user to see if they