Lands http://codereview.chromium.org/155128 for Thiago....

chrome/browser/bookmarks/bookmark_model.cc

 // Copyright (c) 2006-2008 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/bookmarks/bookmark_model.h"
 
 #include "app/l10n_util.h"
 #include "base/gfx/png_decoder.h"
 #include "base/scoped_vector.h"
 #include "build/build_config.h"
@@ skipping 26 matching lines @@
 
 BookmarkNode::BookmarkNode(int id, const GURL& url)
     : url_(url){
   Initialize(id);
 }
 
 void BookmarkNode::Initialize(int id) {
   id_ = id;
   loaded_favicon_ = false;
   favicon_load_handle_ = 0;
-  type_ = !url_.is_empty() ? history::StarredEntry::URL :
-          history::StarredEntry::BOOKMARK_BAR;
+  type_ = !url_.is_empty() ? URL : BOOKMARK_BAR;
   date_added_ = Time::Now();
 }
 
 void BookmarkNode::Reset(const history::StarredEntry& entry) {
-  DCHECK(entry.type != history::StarredEntry::URL ||
-         entry.url == url_);
+  DCHECK(entry.type != history::StarredEntry::URL || entry.url == url_);
 
   favicon_ = SkBitmap();
-  type_ = entry.type;
+  switch (entry.type) {
+    case history::StarredEntry::URL:
+      type_ = BookmarkNode::URL;
+      break;
+    case history::StarredEntry::USER_GROUP:
+      type_ = BookmarkNode::FOLDER;
+      break;
+    case history::StarredEntry::BOOKMARK_BAR:
+      type_ = BookmarkNode::BOOKMARK_BAR;
+      break;
+    case history::StarredEntry::OTHER:
+      type_ = BookmarkNode::OTHER_NODE;
+      break;
+    default:
+      NOTREACHED();
+  }
   date_added_ = entry.date_added;
   date_group_modified_ = entry.date_group_modified;
   SetTitle(entry.title);
 }
 
 // BookmarkModel --------------------------------------------------------------
 
 namespace {
 
 // Constant for persist IDs prefernece.
@@ skipping 212 matching lines @@
   if (!loaded_ || parent == &root_ || !IsValidIndex(parent, index, true)) {
     // Can't add to the root.
     NOTREACHED();
     return NULL;
   }
 
   BookmarkNode* new_node = new BookmarkNode(generate_next_node_id(),
                                             GURL());
   new_node->set_date_group_modified(Time::Now());
   new_node->SetTitle(title);
-  new_node->SetType(history::StarredEntry::USER_GROUP);
+  new_node->SetType(BookmarkNode::FOLDER);
 
   return AddNode(AsMutable(parent), index, new_node, false);
 }
 
 const BookmarkNode* BookmarkModel::AddURL(const BookmarkNode* parent,
                                           int index,
                                           const std::wstring& title,
                                           const GURL& url) {
   return AddURLWithCreationTime(parent, index, title, url, Time::Now());
 }
@@ skipping 10 matching lines @@
     return NULL;
   }
 
   bool was_bookmarked = IsBookmarked(url);
 
   SetDateGroupModified(parent, creation_time);
 
   BookmarkNode* new_node = new BookmarkNode(generate_next_node_id(), url);
   new_node->SetTitle(title);
   new_node->set_date_added(creation_time);
-  new_node->SetType(history::StarredEntry::URL);
+  new_node->SetType(BookmarkNode::URL);
 
   {
     // Only hold the lock for the duration of the insert.
     AutoLock url_lock(url_lock_);
     nodes_ordered_by_url_set_.insert(new_node);
   }
 
   return AddNode(AsMutable(parent), index, new_node, was_bookmarked);
 }
 
@@ skipping 88 matching lines @@
                     BookmarkNodeFavIconLoaded(this, node));
 }
 
 void BookmarkModel::RemoveNode(BookmarkNode* node,
                                std::set<GURL>* removed_urls) {
   if (!loaded_ || !node || is_permanent_node(node)) {
     NOTREACHED();
     return;
   }
 
-  if (node->GetType() == history::StarredEntry::URL) {
+  if (node->GetType() == BookmarkNode::URL) {
     // NOTE: this is called in such a way that url_lock_ is already held. As
     // such, this doesn't explicitly grab the lock.
     NodesOrderedByURLSet::iterator i = nodes_ordered_by_url_set_.find(node);
     DCHECK(i != nodes_ordered_by_url_set_.end());
     // i points to the first node with the URL, advance until we find the
     // node we're removing.
     while (*i != node)
       ++i;
     nodes_ordered_by_url_set_.erase(i);
     removed_urls->insert(node->GetURL());
@@ skipping 110 matching lines @@
   parent->Add(index, node);
 
   if (store_.get())
     store_->ScheduleSave();
 
   FOR_EACH_OBSERVER(BookmarkModelObserver, observers_,
                     BookmarkNodeAdded(this, parent, index));
 
   index_->Add(node);
 
-  if (node->GetType() == history::StarredEntry::URL && !was_bookmarked) {
+  if (node->GetType() == BookmarkNode::URL && !was_bookmarked) {
     history::URLsStarredDetails details(true);
     details.changed_urls.insert(node->GetURL());
     NotificationService::current()->Notify(
         NotificationType::URLS_STARRED,
         Source<Profile>(profile_),
         Details<history::URLsStarredDetails>(&details));
   }
   return node;
 }
 
 void BookmarkModel::BlockTillLoaded() {
   loaded_signal_.Wait();
 }
 
 const BookmarkNode* BookmarkModel::GetNodeByID(const BookmarkNode* node,
                                                int id) {
   if (node->id() == id)
     return node;
 
-  for (int i = 0; i < node->GetChildCount(); ++i) {
+  for (int i = 0, child_count = node->GetChildCount(); i < child_count; ++i) {
     const BookmarkNode* result = GetNodeByID(node->GetChild(i), id);
     if (result)
       return result;
   }
   return NULL;
 }
 
 bool BookmarkModel::IsValidIndex(const BookmarkNode* parent,
                                  int index,
                                  bool allow_end) {
   return (parent && parent->is_folder() &&
           (index >= 0 && (index < parent->GetChildCount() ||
                           (allow_end && index == parent->GetChildCount()))));
-  }
+}
 
 void BookmarkModel::SetDateGroupModified(const BookmarkNode* parent,
                                          const Time time) {
   DCHECK(parent);
   AsMutable(parent)->set_date_group_modified(time);
 
   if (store_.get())
     store_->ScheduleSave();
 }
 
@@ skipping 35 matching lines @@
   DCHECK(node);
   node->set_favicon_load_handle(0);
   if (know_favicon && data.get() &&
       PNGDecoder::Decode(&data->data, &fav_icon)) {
     node->set_favicon(fav_icon);
     FavIconLoaded(node);
   }
 }
 
 void BookmarkModel::LoadFavIcon(BookmarkNode* node) {
-  if (node->GetType() != history::StarredEntry::URL)
+  if (node->GetType() != BookmarkNode::URL)
     return;
 
   DCHECK(node->GetURL().is_valid());
   HistoryService* history_service =
       profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
   if (!history_service)
     return;
 
   HistoryService::Handle handle = history_service->GetFavIconForURL(
       node->GetURL(), &load_consumer_,
@@ skipping 78 matching lines @@
   if (!pref_service->IsPrefRegistered(kPrefPersistIDs))
     pref_service->RegisterBooleanPref(kPrefPersistIDs, false);
 }
 
 void BookmarkModel::LoadPreferences() {
   if (!profile_)
     return;
   PrefService* pref_service = profile_->GetPrefs();
   persist_ids_ = pref_service->GetBoolean(kPrefPersistIDs);
 }