bookmarks: Rename |loaded_favicon_| to |is_favicon_loaded_| so it matches with the getter accessor.

chrome/browser/bookmarks/bookmark_model.h

Index: chrome/browser/bookmarks/bookmark_model.h
diff --git a/chrome/browser/bookmarks/bookmark_model.h b/chrome/browser/bookmarks/bookmark_model.h
index a6a74602b5ae6c90d62767d27476910e1885338d..7eec09329eb2c62a2295aca5cb5eecadb6e6ef7d 100644
--- a/chrome/browser/bookmarks/bookmark_model.h
+++ b/chrome/browser/bookmarks/bookmark_model.h
@@ -6,11 +6,10 @@
 #define CHROME_BROWSER_BOOKMARKS_BOOKMARK_MODEL_H_
 #pragma once
 
-#include "build/build_config.h"
-
 #include <set>
 #include <vector>
 
+#include "base/basictypes.h"
 #include "base/observer_list.h"
 #include "base/string16.h"
 #include "base/synchronization/lock.h"
@@ -38,9 +37,8 @@ struct TitleMatch;
 
 // BookmarkNode ---------------------------------------------------------------
 
-// BookmarkNode contains information about a starred entry: title, URL, favicon,
-// star id and type. BookmarkNodes are returned from a BookmarkModel.

[sky, 2011/07/07 15:52:17]

I prefer the old description, minus the 'star' part.

[tfarina, 2011/07/07 17:03:29]

Done.

-//
+// BookmarkNode contains information about a starred entry: id, URL, type,
+// title, favicon. BookmarkNodes are returned from BookmarkModel.
 class BookmarkNode : public ui::TreeNode<BookmarkNode> {
  public:
   enum Type {
@@ -50,25 +48,23 @@ class BookmarkNode : public ui::TreeNode<BookmarkNode> {
     OTHER_NODE,
     SYNCED
   };
-  // Creates a new node with the specified url and id of 0
+
+  // Creates a new node with id of 0 and |url|.

[sky, 2011/07/07 15:52:17]

'with id' -> 'with an id'

[tfarina, 2011/07/07 17:03:29]

Done.

   explicit BookmarkNode(const GURL& url);
-  // Creates a new node with the specified url and id.
+  // Creates a new node with |id| and |url|.
   BookmarkNode(int64 id, const GURL& url);
-  virtual ~BookmarkNode();
 
-  // Returns the URL.
-  const GURL& GetURL() const { return url_; }
-  // Sets the URL to the given value.
-  void SetURL(const GURL& url) { url_ = url; }
+  virtual ~BookmarkNode();
 
-  // Returns a unique id for this node.
+  // Returns an unique id for this node.
   // For bookmark nodes that are managed by the bookmark model, the IDs are
   // persisted across sessions.
   int64 id() const { return id_; }
-  // Sets the id to the given value.
   void set_id(int64 id) { id_ = id; }
 
-  // Returns the type of this node.
+  const GURL& GetURL() const { return url_; }
+  void SetURL(const GURL& url) { url_ = url; }
+
   Type type() const { return type_; }
   void set_type(Type type) { type_ = type; }
 
@@ -78,11 +74,10 @@ class BookmarkNode : public ui::TreeNode<BookmarkNode> {
   void set_date_added(const base::Time& date) { date_added_ = date; }
 
   // Returns the last time the folder was modified. This is only maintained
-  // for folders (including the bookmark and other folder).
+  // for folders (including the bookmark bar and other folder).
   const base::Time& date_folder_modified() const {
     return date_folder_modified_;
   }
-  // Sets the last time the folder was modified.
   void set_date_folder_modified(const base::Time& date) {
     date_folder_modified_ = date;
   }
@@ -90,8 +85,6 @@ class BookmarkNode : public ui::TreeNode<BookmarkNode> {
   // Convenience for testing if this nodes represents a folder. A folder is a
   // node whose type is not URL.
   bool is_folder() const { return type_ != URL; }
-
-  // Is this a URL?
   bool is_url() const { return type_ == URL; }
 
   // Returns the favicon. In nearly all cases you should use the method
@@ -104,8 +97,15 @@ class BookmarkNode : public ui::TreeNode<BookmarkNode> {
   // The following methods are used by the bookmark model, and are not
   // really useful outside of it.
 
-  bool is_favicon_loaded() const { return loaded_favicon_; }
-  void set_favicon_loaded(bool value) { loaded_favicon_ = value; }
+  bool favicon_loaded() const { return favicon_loaded_; }
+  void set_favicon_loaded(bool loaded) { favicon_loaded_ = loaded; }

[sky, 2011/07/07 15:52:17]

I prefer the old, so can you rename the field to is_favicon_loaded_ and rename
the setter to match.

[tfarina, 2011/07/07 17:03:29]

Done.

+
+  HistoryService::Handle favicon_load_handle() const {
+    return favicon_load_handle_;
+  }
+  void set_favicon_load_handle(HistoryService::Handle handle) {
+    favicon_load_handle_ = handle;
+  }
 
   // Accessor method for controlling the visibility of a bookmark node/sub-tree.
   // Note that visibility is not propagated down the tree hierarchy so if a
@@ -116,13 +116,6 @@ class BookmarkNode : public ui::TreeNode<BookmarkNode> {
   // no longer a command line flag.
   bool IsVisible() const;
 
-  HistoryService::Handle favicon_load_handle() const {
-    return favicon_load_handle_;
-  }
-  void set_favicon_load_handle(HistoryService::Handle handle) {
-    favicon_load_handle_ = handle;
-  }
-
   // Called when the favicon becomes invalid.
   void InvalidateFavicon();

[sky, 2011/07/07 15:52:17]

Move this to be with other favicon methods too.

[tfarina, 2011/07/07 17:03:29]

Done.

 
@@ -132,35 +125,35 @@ class BookmarkNode : public ui::TreeNode<BookmarkNode> {
  private:
   friend class BookmarkModel;
 
-  // Helper to initialize various fields during construction.
+  // A helper function to initialize some of the variables below.

[sky, 2011/07/07 15:52:17]

Old description is better.

[tfarina, 2011/07/07 17:03:29]

Done.

   void Initialize(int64 id);
 
-  // Unique identifier for this node.
+  // The unique identifier for this node.
   int64 id_;
 
-  // Whether the favicon has been loaded.
-  bool loaded_favicon_;
-
-  // The favicon.
-  SkBitmap favicon_;
-
-  // If non-zero, it indicates we're loading the favicon and this is the handle
-  // from the HistoryService.
-  HistoryService::Handle favicon_load_handle_;
-
-  // The URL. BookmarkModel maintains maps off this URL, it is important that
-  // changes to the URL is done through the bookmark model.
+  // The URL of this node. BookmarkModel maintains maps off this URL, so changes
+  // changes to the URL should be done through the BookmarkModel.

[sky, 2011/07/07 15:52:17]

'changes changes' -> changes.
'should be' -> 'must be'

[tfarina, 2011/07/07 17:03:29]

Done.

   GURL url_;
 
-  // Type of node.
+  // The type of this node. See enum above.
   Type type_;
 
-  // Date we were created.
+  // Date of when this node was created.
   base::Time date_added_;
 
-  // Time last modified. Only used for folders.
+  // Date of the last modification. Only used for folders.
   base::Time date_folder_modified_;
 
+  // The favicon of this node.
+  SkBitmap favicon_;
+
+  // Whether the favicon has been loaded.
+  bool favicon_loaded_;
+
+  // If non-zero, it indicates we're loading the favicon and this is the handle
+  // from the HistoryService.
+  HistoryService::Handle favicon_load_handle_;
+
   DISALLOW_COPY_AND_ASSIGN(BookmarkNode);
 };