Title: Same Bookmark url is getting pasted on the Bookmarkbar with same title.

components/bookmarks/browser/bookmark_utils.cc

Index: components/bookmarks/browser/bookmark_utils.cc
diff --git a/components/bookmarks/browser/bookmark_utils.cc b/components/bookmarks/browser/bookmark_utils.cc
index a60a86d0020c6eeb0f231b13e5ee0be270b581e2..af10c347dc03529e218b2ed3ef1a5947da922350 100644
--- a/components/bookmarks/browser/bookmark_utils.cc
+++ b/components/bookmarks/browser/bookmark_utils.cc
@@ -8,11 +8,14 @@
 
 #include "base/basictypes.h"
 #include "base/bind.h"
+#include "base/containers/hash_tables.h"
 #include "base/files/file_path.h"
 #include "base/i18n/case_conversion.h"
 #include "base/i18n/string_search.h"
 #include "base/metrics/user_metrics_action.h"
 #include "base/prefs/pref_service.h"
+#include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "components/bookmarks/browser/bookmark_client.h"
@@ -201,6 +204,35 @@ void CopyToClipboard(BookmarkModel* model,
   }
 }
 
+// This function will update title for the bookmark with numeric subscript if

[sky, 2014/10/07 16:21:23]

// Updates |title| such that |url| and |title| pair are unique among the
children
// of |parent|.
MakeTitleUnique()

[Deepak, 2014/10/08 09:02:23]

Done.

+// some bookmark with same |title|and |url| is already exist as sibling.
+void UpdateCopiedBookmarkTitle(const BookmarkModel* model,
+                               const BookmarkNode* parent,
+                               const GURL& url,
+                               base::string16* title) {
+  base::hash_set<base::string16> titles;
+  base::string16 new_title = *title;

[sky, 2014/10/07 16:21:23]

Keep variables close to where you need them. You should only need this at 227.

[Deepak, 2014/10/08 09:02:23]

Done.

+  for (int i = 0; i < parent->child_count(); i++) {
+    const BookmarkNode* node = parent->GetChild(i);
+    if (node->is_url() && (url == node->url()) &&
+        StartsWith(node->GetTitle(), new_title, false)) {
+      titles.insert(node->GetTitle());
+    }
+  }
+
+  if (titles.size() == 0 || titles.find(new_title) == titles.end())

[sky, 2014/10/07 16:21:23]

Why bother with the size check (empty() is better here anyway). Doesn't the find
cover the case you care about?

[Deepak, 2014/10/08 09:02:23]

Done.

+    return;
+
+  for (size_t i = 0; i < titles.size(); i++) {
+    new_title = base::UTF8ToUTF16(base::StringPrintf(

[sky, 2014/10/07 16:21:23]

This is still more verbose than you need.

[Deepak, 2014/10/08 09:02:23]

This verbose is due to way StringPrintf() works.
As It takes utf8 data and our *title and new_title are utf16.
and I need to take titles hash_set string16,as node->GetTitle() gives me
string16.
If we are making StringPrintf() statement short then conversion shift at other
places.I think that will do more cluttering.

please suggest if you have some other opinion for this.

+        "%s (%lu)", base::UTF16ToUTF8(*title).c_str(), i + 1));
+    if (titles.find(new_title) == titles.end()) {
+      *title = new_title;
+      return;
+    }
+  }
+}
+
 void PasteFromClipboard(BookmarkModel* model,
                         const BookmarkNode* parent,
                         int index) {
@@ -219,6 +251,15 @@ void PasteFromClipboard(BookmarkModel* model,
   if (index == -1)
     index = parent->child_count();
   ScopedGroupBookmarkActions group_paste(model);
+
+  if (bookmark_data.elements.size() == 1 &&
+      model->IsBookmarked(bookmark_data.elements[0].url)) {
+    UpdateCopiedBookmarkTitle(model,
+                              parent,
+                              bookmark_data.elements[0].url,
+                              &bookmark_data.elements[0].title);
+  }
+
   CloneBookmarkNode(model, bookmark_data.elements, parent, index, true);
 }