Set version field when changes are made to enhanced bookmarks.

components/enhanced_bookmarks/enhanced_bookmark_model.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_ENHANCED_BOOKMARKS_ENHANCED_BOOKMARK_MODEL_H_
+#define COMPONENTS_ENHANCED_BOOKMARKS_ENHANCED_BOOKMARK_MODEL_H_
+
+#include <string>
+
+#include "base/strings/string16.h"
+#include "components/bookmarks/browser/bookmark_node.h"
+#include "components/keyed_service/core/keyed_service.h"
+
+namespace base {
+class Time;
+}  // namespace base
+
+class BookmarkModel;
+class GURL;
+
+namespace enhanced_bookmarks {
+// Wrapper around BookmarkModel providing utility functions for enhanced
+// bookmarks.
+class EnhancedBookmarkModel : public KeyedService {

[noyau (Ping after 24h), 2014/09/05 12:04:43]

This doesn't track changes in the model, and as such won't be able to detect
changes to the model done without using this API (changes from other synced
client, changes from extensions manipulating the bookmark_model directly,
copy/paste…

[Rune Fevang, 2014/09/06 00:01:56]

Right, that will come in a later CL (task 4 in the timeline).

[noyau (Ping after 24h), 2014/09/08 09:04:18]

Acknowledged.

+ public:
+  EnhancedBookmarkModel(BookmarkModel* bookmark_model,
+                        const std::string& version);
+  virtual ~EnhancedBookmarkModel();
+
+  // Moves |node| to |new_parent| and inserts it at the given |index|.
+  void Move(const BookmarkNode* node,
+            const BookmarkNode* new_parent,
+            int index);
+
+  // Adds a new folder node at the specified position.
+  const BookmarkNode* AddFolder(const BookmarkNode* parent,
+                                int index,
+                                const base::string16& title,
+                                const BookmarkNode::MetaInfoMap* meta_info);

[noyau (Ping after 24h), 2014/09/05 12:04:43]

Why does this takes a metainfo? I thought the whole point of that class was to
hide all the metainfo malarkey from the user of the API…

[Rune Fevang, 2014/09/06 00:01:56]

Yes that is the (at least long term) point, but as I'm not sure how you're
currently creating nodes, I thought it was better to be flexible for now. You
could always just pass NULL if you don't have anything to set.

However, as the only things that are critical to set on node creation are id and
placement, I agree it shouldn't be there. Removed.

+
+  // Adds a url at the specified position.
+  const BookmarkNode* AddURL(const BookmarkNode* parent,
+                             int index,
+                             const base::string16& title,
+                             const GURL& url,
+                             const base::Time& creation_time,
+                             const BookmarkNode::MetaInfoMap* meta_info);

[noyau (Ping after 24h), 2014/09/05 12:04:43]

Same here. I'd expect addURL to take an optional description and image, but not
a whole map that as a user I can't create anyway as the constants to do so are
in the .cc…

Note that on iOS the bookmarks are created without any meta info as the image is
retrieved asynchronously, so it is set later.

[Rune Fevang, 2014/09/06 00:01:56]

That's fine, the main things that need to be set on creation is stars.id and
position, which I was planning on adding in a later CL. Changed the
implementation to set stars.id in this CL as the necessary framework is already
in.

+
+  // Returns the remote id for a bookmark.
+  std::string GetRemoteIdForNode(const BookmarkNode* node);
+
+  // Sets the description of a bookmark. |user_edit| should be true if
+  // setting the description is the direct result of a user action.
+  void SetDescriptionForNode(const BookmarkNode* node,
+                             const std::string& description,
+                             bool user_edit);

[noyau (Ping after 24h), 2014/09/05 12:04:43]

I don't understand user_edit. I thought the description was always user edited,
and if not preset the snippet would be used instead.

[Rune Fevang, 2014/09/06 00:01:56]

Discussed userEdit with mcolbert@. Decided that it was enough to just have it
set on initialization/dup correction. So removed them again from the API and
added a section in the design document.

+
+  // Returns the description of a bookmark.
+  std::string GetDescriptionForNode(const BookmarkNode* node);
+
+  // Sets the URL of an image representative of the page.
+  // Expects the URL to be valid and not empty. |user_edit| should be true if
+  // setting the image is the direct result of a user action.
+  // Returns true if the metainfo is successfully populated.
+  bool SetOriginalImageForNode(const BookmarkNode* node,
+                               const GURL& url,
+                               int width,
+                               int height,
+                               bool user_edit);

[noyau (Ping after 24h), 2014/09/05 12:04:43]

Here user_edit is confusing, this is always the result of a user action, he
visited a bookmarked page: user action. Bookmarked a page: user action. So this
always be true. Or this need a better explanation in the comment.

+
+  // Returns the url and dimensions of the original scraped image.
+  // Returns true if the out variables are populated, false otherwise.
+  bool GetOriginalImageForNode(const BookmarkNode* node,
+                               GURL* url,
+                               int* width,
+                               int* height);
+
+  // Returns the url and dimensions of the server provided thumbnail image.
+  // Returns true if the out variables are populated, false otherwise.
+  bool GetThumbnailImageForNode(const BookmarkNode* node,
+                                GURL* url,
+                                int* width,
+                                int* height);
+
+  // Returns a brief server provided synopsis of the bookmarked page.
+  // Returns the empty string if the snippet could not be extracted.
+  std::string GetSnippetForNode(const BookmarkNode* node);
+
+  // Sets a custom suffix to be added to the version field when writing meta
+  // info fields.
+  void SetVersionSuffix(const std::string& version_suffix);
+
+  // TODO(rfevang): Add method + enum for accessing/writing flags.
+
+  // Used for testing, simulates the process that creates the thumbnails. Will
+  // remove existing entries for empty urls or set them if the url is not empty.
+  // Expects valid or empty urls. Returns true if the metainfo is successfully
+  // populated.
+  // TODO(rfevang): Move this to a testing only utility file.
+  static bool SetAllImagesForBookmark(BookmarkModel* bookmark_model,

[noyau (Ping after 24h), 2014/09/05 12:04:43]

The bookmark_model argument is not needed here anymore.

[Rune Fevang, 2014/09/06 00:01:56]

Done.

+                                      const BookmarkNode* node,
+                                      const GURL& image_url,
+                                      int image_width,
+                                      int image_height,
+                                      const GURL& thumbnail_url,
+                                      int thumbnail_width,
+                                      int thumbnail_height);
+
+ private:
+  // Generates and sets a remote id for the given bookmark. Returns the id set.
+  std::string SetRemoteIdForNode(const BookmarkNode* node);
+
+  // Helper method for setting a meta info field on a node. Also updates the
+  // version and userEdits fields.
+  void SetNodeMetaInfo(const BookmarkNode* node,
+                       const std::string& field,
+                       const std::string& value,
+                       bool user_edit);
+
+  // Returns the version string to use when setting stars.version.
+  std::string GetVersionString();
+
+  BookmarkModel* bookmark_model_;
+  std::string version_;
+  std::string version_suffix_;
+};
+
+}  // namespace enhanced_bookmarks
+
+#endif  // COMPONENTS_ENHANCED_BOOKMARKS_ENHANCED_BOOKMARK_MODEL_H_