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

chrome/browser/bookmarks/bookmark_model.h

 // Copyright (c) 2011 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.
 
 #ifndef CHROME_BROWSER_BOOKMARKS_BOOKMARK_MODEL_H_
 #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"
 #include "base/synchronization/waitable_event.h"
 #include "chrome/browser/bookmarks/bookmark_model_observer.h"
 #include "chrome/browser/bookmarks/bookmark_service.h"
 #include "chrome/browser/favicon/favicon_service.h"
 #include "chrome/browser/history/history.h"
 #include "content/browser/cancelable_request.h"
 #include "content/common/notification_registrar.h"
 #include "googleurl/src/gurl.h"
 #include "testing/gtest/include/gtest/gtest_prod.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/base/models/tree_node_model.h"
 
 class BookmarkIndex;
 class BookmarkLoadDetails;
 class BookmarkModel;
 class BookmarkStorage;
 class Profile;
 
 namespace bookmark_utils {
 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 {
     URL,
     FOLDER,
     BOOKMARK_BAR,
     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; }
-
-  // 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; }
 
   // Returns the time the bookmark/folder was added.
   const base::Time& date_added() const { return date_added_; }
   // Sets the time the bookmark/folder was added.
   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;
   }
 
   // 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
   // BookmarkModel::GetFavicon rather than this. BookmarkModel::GetFavicon
   // takes care of loading the favicon if it isn't already loaded, where as
   // this does not.
   const SkBitmap& favicon() const { return favicon_; }
   void set_favicon(const SkBitmap& icon) { favicon_ = icon; }
 
   // 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
   // parent node is marked as invisible, a child node may return "Visible". This
   // function is primarily useful when traversing the model to generate a UI
   // representation but we may want to suppress some nodes.
   // TODO(yfriedman): Remove this when enable-synced-bookmarks-folder is
   // 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.

 
   // TODO(sky): Consider adding last visit time here, it'll greatly simplify
   // HistoryContentsProvider.
 
  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_;
 
+  // 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_;
+
+  // The type of this node. See enum above.
+  Type type_;
+
+  // Date of when this node was created.
+  base::Time date_added_;
+
+  // 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 loaded_favicon_;
-
-  // The favicon.
-  SkBitmap favicon_;
+  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_;
 
-  // The URL. BookmarkModel maintains maps off this URL, it is important that
-  // changes to the URL is done through the bookmark model.
-  GURL url_;
-
-  // Type of node.
-  Type type_;
-
-  // Date we were created.
-  base::Time date_added_;
-
-  // Time last modified. Only used for folders.
-  base::Time date_folder_modified_;
-
   DISALLOW_COPY_AND_ASSIGN(BookmarkNode);
 };
 
 // BookmarkModel --------------------------------------------------------------
 
 // BookmarkModel provides a directed acyclic graph of the starred entries
 // and folders. Two graphs are provided for the two entry points: those on
 // the bookmark bar, and those in the other folder.
 //
 // An observer may be attached to observer relevant events.
@@ skipping 294 matching lines @@
   scoped_refptr<BookmarkStorage> store_;
 
   scoped_ptr<BookmarkIndex> index_;
 
   base::WaitableEvent loaded_signal_;
 
   DISALLOW_COPY_AND_ASSIGN(BookmarkModel);
 };
 
 #endif  // CHROME_BROWSER_BOOKMARKS_BOOKMARK_MODEL_H_