Add TitledUrlIndex for indexing arbitrary title/URL pairs

components/bookmarks/browser/bookmark_client.h

 // Copyright 2014 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 COMPONENTS_BOOKMARKS_BROWSER_BOOKMARK_CLIENT_H_
 #define COMPONENTS_BOOKMARKS_BROWSER_BOOKMARK_CLIENT_H_
 
 #include <set>
 #include <utility>
 #include <vector>
 
 #include "base/callback_forward.h"
 #include "base/task/cancelable_task_tracker.h"
 #include "components/bookmarks/browser/bookmark_storage.h"
 #include "components/favicon_base/favicon_callback.h"
 #include "components/favicon_base/favicon_types.h"
 #include "components/keyed_service/core/keyed_service.h"
 
 class GURL;
 
 namespace base {
 struct UserMetricsAction;
 }
 
 namespace bookmarks {
 
 class BookmarkModel;
-class BookmarkNode;
 class BookmarkPermanentNode;
+class TitledUrlNode;
 
 // This class abstracts operations that depends on the embedder's environment,
 // e.g. Chrome.
 class BookmarkClient {

[sky, 2016/12/02 00:21:24]

Why do you need to touch this class? I was assuming you would be changing the
index not to take a client but something that has the single function used from
this class. Also, as BookmarkNodes are TitledUrlNode you shouldn't need to
change these typedefs, right?

[mattreynolds, 2016/12/02 21:02:06]

Even if I factor out the sorting logic, at some point we still need to get from
TitledUrlNode to whatever BookmarkClient::GetTypedCountForNode expects. I felt
the cleanest solution was to modify GetTypedCountForNode to take
set<TitledUrlNode>. I think it would also be reasonable for it to take set<GURL>
since the only BookmarkNode method it needs is the URL accessor. WDYT?

[sky, 2016/12/02 21:30:29]

BookmarkClient should be able to assume it is operating on BookmarkNodes, not
TitledUrlNode. But I agree that GetTypedCountForNodes doesn't need a node, just
the url. So, I'm fine with changing it to take a set<url> I suggest you do that
patch first in hopes of more focused smaller patches.

  public:
-  // Types representing a set of BookmarkNode and a mapping from BookmarkNode
-  // to the number of time the corresponding URL has been typed by the user in
+  // Types representing a set of TitledUrlNodes and a mapping from TitledUrlNode
+  // to the number of times the corresponding URL has been typed by the user in
   // the Omnibox.
-  typedef std::set<const BookmarkNode*> NodeSet;
-  typedef std::pair<const BookmarkNode*, int> NodeTypedCountPair;
+  typedef std::set<const TitledUrlNode*> NodeSet;
+  typedef std::pair<const TitledUrlNode*, int> NodeTypedCountPair;
   typedef std::vector<NodeTypedCountPair> NodeTypedCountPairs;
 
   virtual ~BookmarkClient() {}
 
   // Called during initialization of BookmarkModel.
   virtual void Init(BookmarkModel* model);
 
   // Returns true if the embedder favors touch icons over favicons.
   virtual bool PreferTouchIcon();
 
@@ skipping 41 matching lines @@
   // Returns true if this node can be edited by the user.
   // TODO(joaodasilva): the model should check this more aggressively, and
   // should give the client a means to temporarily disable those checks.
   // http://crbug.com/49598
   virtual bool CanBeEditedByUser(const BookmarkNode* node) = 0;
 };
 
 }  // namespace bookmarks
 
 #endif  // COMPONENTS_BOOKMARKS_BROWSER_BOOKMARK_CLIENT_H_