Introduce BookmarkClient interface to abstract embedder

chrome/browser/bookmarks/bookmark_model.h

 // Copyright (c) 2012 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_
 
 #include <map>
 #include <set>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/observer_list.h"
 #include "base/strings/string16.h"
 #include "base/synchronization/lock.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/task/cancelable_task_tracker.h"
 #include "chrome/browser/bookmarks/bookmark_service.h"
+#include "components/bookmarks/core/browser/bookmark_client.h"
 #include "components/bookmarks/core/browser/bookmark_node.h"
-#include "components/favicon_base/favicon_types.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "ui/gfx/image/image.h"
 #include "url/gurl.h"
 
 class BookmarkExpandedStateTracker;
 class BookmarkIndex;
 class BookmarkLoadDetails;
 class BookmarkModelObserver;
 class BookmarkStorage;
 struct BookmarkTitleMatch;
+class PrefService;
 class Profile;
 class ScopedGroupBookmarkActions;
 
 namespace base {
+class FilePath;
 class SequencedTaskRunner;
 }
 
-namespace chrome {
-struct FaviconImageResult;
-}
-
 // BookmarkModel --------------------------------------------------------------
 
 // BookmarkModel provides a directed acyclic graph of URLs and folders.
 // Three graphs are provided for the three entry points: those on the 'bookmarks
 // bar', those in the 'other bookmarks' folder and those in the 'mobile' folder.
 //
 // An observer may be attached to observe relevant events.
 //
 // You should NOT directly create a BookmarkModel, instead go through the
 // BookmarkModelFactory.
-class BookmarkModel : public content::NotificationObserver,
-                      public BookmarkService,
-                      public KeyedService {
+class BookmarkModel : public BookmarkService {
  public:
-  explicit BookmarkModel(Profile* profile);
+  explicit BookmarkModel(BookmarkClient* client);
   virtual ~BookmarkModel();
 
   // Invoked prior to destruction to release any necessary resources.
-  virtual void Shutdown() OVERRIDE;
+  void Shutdown();
 
   // Loads the bookmarks. This is called upon creation of the
   // BookmarkModel. You need not invoke this directly.
-  // All load operations will be executed on |task_runner|.
-  void Load(const scoped_refptr<base::SequencedTaskRunner>& task_runner);
+  // All load operations will be executed on |io_task_runner|, and callback
+  // will be called from |io_task_runner|.
+  void Load(PrefService* pref_service,
+            const base::FilePath& profile_path,
+            const scoped_refptr<base::SequencedTaskRunner>& io_task_runner,
+            const scoped_refptr<base::SequencedTaskRunner>& ui_task_runner);
 
   // Returns true if the model finished loading.
   bool loaded() const { return loaded_; }
 
   // Returns the root node. The 'bookmark bar' node and 'other' node are
   // children of the root node.
   const BookmarkNode* root_node() { return &root_; }
 
   // Returns the 'bookmark bar' node. This is NULL until loaded.
   const BookmarkNode* bookmark_bar_node() { return bookmark_bar_node_; }
@@ skipping 160 matching lines @@
                        const std::string& value);
   void SetNodeMetaInfoMap(const BookmarkNode* node,
                           const BookmarkNode::MetaInfoMap& meta_info_map);
   void DeleteNodeMetaInfo(const BookmarkNode* node,
                           const std::string& key);
 
   // Sets the sync transaction version of |node|.
   void SetNodeSyncTransactionVersion(const BookmarkNode* node,
                                      int64 sync_transaction_version);
 
-  // Returns the profile that corresponds to this BookmarkModel.
-  Profile* profile() { return profile_; }
+  // Returns the BookmarkClient that corresponds to this BookmarkModel.
+  BookmarkClient* client() { return client_; }
+
+  void OnFaviconChanged(const std::set<GURL>& urls);
 
  private:
   friend class BookmarkCodecTest;
   friend class BookmarkModelTest;
   friend class BookmarkStorage;
   friend class ScopedGroupBookmarkActions;
 
   // Used to order BookmarkNodes by URL.
   class NodeURLComparator {
    public:
@@ skipping 26 matching lines @@
   void RemoveNodeAndGetRemovedUrls(BookmarkNode* node,
                                    std::set<GURL>* removed_urls);
 
   // Removes the node from its parent, sends notification, and deletes it.
   // type specifies how the node should be removed.
   void RemoveAndDeleteNode(BookmarkNode* delete_me);
 
   // Remove |node| from |nodes_ordered_by_url_set_|.
   void RemoveNodeFromURLSet(BookmarkNode* node);
 
-  // Notifies the history backend about urls of removed bookmarks.
-  void NotifyHistoryAboutRemovedBookmarks(
-      const std::set<GURL>& removed_bookmark_urls) const;
-
   // Adds the |node| at |parent| in the specified |index| and notifies its
   // observers.
   BookmarkNode* AddNode(BookmarkNode* parent,
                         int index,
                         BookmarkNode* node);
 
   // Implementation of GetNodeByID.
   const BookmarkNode* GetNodeByID(const BookmarkNode* node, int64 id) const;
 
   // Returns true if the parent and index are valid.
@@ skipping 17 matching lines @@
   void FaviconLoaded(const BookmarkNode* node);
 
   // If we're waiting on a favicon for node, the load request is canceled.
   void CancelPendingFaviconLoadRequests(BookmarkNode* node);
 
   // Notifies the observers that a set of changes initiated by a single user
   // action is about to happen and has completed.
   void BeginGroupedChanges();
   void EndGroupedChanges();
 
-  // content::NotificationObserver:
-  virtual void Observe(int type,
-                       const content::NotificationSource& source,
-                       const content::NotificationDetails& details) OVERRIDE;
-
   // Generates and returns the next node ID.
   int64 generate_next_node_id();
 
   // Sets the maximum node ID to the given value.
   // This is used by BookmarkCodec to report the maximum ID after it's done
   // decoding since during decoding codec assigns node IDs.
   void set_next_node_id(int64 id) { next_node_id_ = id; }
 
   // Creates and returns a new BookmarkLoadDetails. It's up to the caller to
   // delete the returned object.
   BookmarkLoadDetails* CreateLoadDetails();
 
-  content::NotificationRegistrar registrar_;
-
-  Profile* profile_;
+  BookmarkClient* client_;
 
   // Whether the initial set of data has been loaded.
   bool loaded_;
 
   // The root node. This contains the bookmark bar node and the 'other' node as
   // children.
   BookmarkNode root_;
 
   BookmarkPermanentNode* bookmark_bar_node_;
   BookmarkPermanentNode* other_node_;
@@ skipping 24 matching lines @@
   base::WaitableEvent loaded_signal_;
 
   // See description of IsDoingExtensiveChanges above.
   int extensive_changes_;
 
   scoped_ptr<BookmarkExpandedStateTracker> expanded_state_tracker_;
 
   DISALLOW_COPY_AND_ASSIGN(BookmarkModel);
 };
 
+class ChromeBookmarkClient : public content::NotificationObserver,

[tfarina, 2014/04/22 17:22:34]

No, don't do that. Instead move this class declaration into
chrome_bookmark_client.h please.

[sdefresne, 2014/04/22 20:25:49]

Done.

+                             public BookmarkClient,

[tfarina, 2014/04/22 17:22:34]

Can you make BookmarkClient be the first in the inheritance list? I.e., swap its
place with NotificationObserver. I'd also put NotificationObserver as the third.

[sdefresne, 2014/04/22 20:25:49]

Done.

+                             public KeyedService {
+ public:
+  ChromeBookmarkClient(Profile* profile);

[tfarina, 2014/04/22 17:22:34]

explicit

[sdefresne, 2014/04/22 20:25:49]

Done.

+  virtual ~ChromeBookmarkClient();
+
+  // Returns the BookmarkModel that corresponds to this ChromeBookmarkClient.
+  BookmarkModel* model() { return model_.get(); }
+
+  // Returns the Profile that corresponds to this ChromeBookmarkClient.
+  Profile* profile() { return profile_; }
+
+  // KeyedService:
+  virtual void Shutdown() OVERRIDE;

[tfarina, 2014/04/22 17:22:34]

Please, make sure the overrides follow the same sequence of the inherited
classes.

[sdefresne, 2014/04/22 20:25:49]

Done.

+
+  // content::NotificationObserver:
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
+
+  // BookmarkClient:
+  virtual void OnLoad() OVERRIDE;
+

[tfarina, 2014/04/22 17:22:34]

Do not put blank lines between these overrides. We usually group methods
overridden from the same class together.

[sdefresne, 2014/04/22 20:25:49]

Done.

+  virtual void OnClearStore() OVERRIDE;
+
+  virtual void NotifyHistoryAboutRemovedBookmarks(
+      const std::set<GURL>& removed_bookmarks_urls) const OVERRIDE;
+
+  virtual base::CancelableTaskTracker::TaskId GetFaviconImageForURL(
+      const GURL& page_url,
+      int icon_types,
+      int desired_size_in_dip,
+      const FaviconImageCallback& callback,
+      base::CancelableTaskTracker* tracker) OVERRIDE;
+
+  virtual bool SupportsTypedCountForNodes() OVERRIDE;
+
+  virtual void GetTypedCountForNodes(
+      const NodeSet& nodes,
+      NodeTypedCountPairs* node_typed_count_pairs) OVERRIDE;
+
+  virtual void RecordAction(const base::UserMetricsAction& action) OVERRIDE;
+
+ private:
+  Profile* profile_;
+
+  content::NotificationRegistrar registrar_;
+
+  scoped_ptr<BookmarkModel> model_;
+
+  DISALLOW_COPY_AND_ASSIGN(ChromeBookmarkClient);
+};
+
 #endif  // CHROME_BROWSER_BOOKMARKS_BOOKMARK_MODEL_H_