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

components/bookmarks/browser/bookmark_utils.cc

 // 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.
 
 #include "components/bookmarks/browser/bookmark_utils.h"
 
 #include <utility>
 
 #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"
 #include "components/bookmarks/browser/bookmark_model.h"
 #include "components/bookmarks/browser/scoped_group_bookmark_actions.h"
 #include "components/bookmarks/common/bookmark_pref_names.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/query_parser/query_parser.h"
 #include "net/base/net_util.h"
 #include "ui/base/clipboard/clipboard.h"
@@ skipping 168 matching lines @@
   if (remove_nodes) {
     ScopedGroupBookmarkActions group_cut(model);
     for (size_t i = 0; i < filtered_nodes.size(); ++i) {
       int index = filtered_nodes[i]->parent()->GetIndexOf(filtered_nodes[i]);
       if (index > -1)
         model->Remove(filtered_nodes[i]->parent(), index);
     }
   }
 }
 
+// This function will returns new title for the bookmark if some bookmark with
+// same |title|and |url| is already exist as sibling, else returns original
+// |title|.
+base::string16 UpdateCopiedBookmarkTitle(const BookmarkModel* model,
+                                         const BookmarkNode* parent,
+                                         const GURL& url,
+                                         const base::string16& title) {

[sky, 2014/10/01 19:28:26]

This should be a pointer and modified in place.

[Deepak, 2014/10/02 07:19:45]

Done.

+  base::hash_set<base::string16> titles;
+  base::string16 new_title = title;
+  for (int i = 0; i < parent->child_count(); i++) {
+    const BookmarkNode* node = parent->GetChild(i);
+    if (node->is_url() && (url == node->url()) &&
+        (new_title == node->GetTitle() ||

[sky, 2014/10/01 19:28:25]

Doesn't the StartsWith cover this case too?

[Deepak, 2014/10/02 07:19:45]

Done.

+         StartsWith(node->GetTitle(), new_title, false))) {
+      titles.insert(node->GetTitle());
+    }
+  }
+
+  if (titles.find(new_title) == titles.end())
+    return new_title;
+
+  for (size_t i = 0; i < titles.size(); i++) {
+    new_title = base::UTF8ToUTF16(
+        base::StringPrintf("%s(%lu)", base::UTF16ToUTF8(title).c_str(), i + 1));

[sky, 2014/10/01 19:28:26]

Add a space here.

[sky, 2014/10/01 19:28:26]

All this conversion is ugly. Can you do just one conversion?

[Deepak, 2014/10/02 07:19:45]

Done.

[Deepak, 2014/10/02 07:19:45]

As I am creating a new title everytime by changing the subscript in the input
title.and base::StringPrintf() return me std::string that is UTF8 and our
new_title is UTF16, so base::UTF8ToUTF16 in
base::UTF8ToUTF16(base::StringPrintf())
above I can not avoid.
and StringPrintf() takes const char* as I need to change title to UTF8 format.so
that %s can use that.It is also necessary for me.

+    if (titles.find(new_title) == titles.end())
+      break;

[sky, 2014/10/01 19:28:26]

You can return here, right?

[Deepak, 2014/10/02 07:19:45]

Done.

+  }
+  return new_title;
+}
+
 void PasteFromClipboard(BookmarkModel* model,
                         const BookmarkNode* parent,
                         int index) {
   if (!parent)
     return;
 
   BookmarkNodeData bookmark_data;
   if (!bookmark_data.ReadFromClipboard(ui::CLIPBOARD_TYPE_COPY_PASTE)) {
     GURL url = GetUrlFromClipboard();
     if (!url.is_valid())
       return;
     BookmarkNode node(url);
     node.SetTitle(base::ASCIIToUTF16(url.spec()));
     bookmark_data = BookmarkNodeData(&node);
   }
   if (index == -1)
     index = parent->child_count();
   ScopedGroupBookmarkActions group_paste(model);
+
+  if (bookmark_data.elements.size() == 1 &&
+      model->IsBookmarked(bookmark_data.elements[0].url)) {
+    bookmark_data.elements[0].title =
+        UpdateCopiedBookmarkTitle(model,
+                                  parent,
+                                  bookmark_data.elements[0].url,
+                                  bookmark_data.elements[0].title);
+  }
+
   CloneBookmarkNode(model, bookmark_data.elements, parent, index, true);
 }
 
 bool CanPasteFromClipboard(BookmarkModel* model, const BookmarkNode* node) {
   if (!node || !model->client()->CanBeEditedByUser(node))
     return false;
   return (BookmarkNodeData::ClipboardContainsBookmarks() ||
           GetUrlFromClipboard().is_valid());
 }
 
@@ skipping 224 matching lines @@
   }
   return false;
 }
 
 const BookmarkNode* GetBookmarkNodeByID(const BookmarkModel* model, int64 id) {
   // TODO(sky): TreeNode needs a method that visits all nodes using a predicate.
   return GetNodeByID(model->root_node(), id);
 }
 
 }  // namespace bookmarks