Simplify Ampersand escaping logic in RenderViewContextMenu.

chrome/browser/tab_contents/render_view_context_menu.cc

 // 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.
 
 #include "chrome/browser/tab_contents/render_view_context_menu.h"
 
 #include <algorithm>
 #include <set>
 #include <utility>
 
@@ skipping 371 matching lines @@
   if (sorted_ids.empty())
     return;
 
   int index = 0;
   base::TimeTicks begin = base::TimeTicks::Now();
   std::vector<std::pair<std::string, std::string> >::const_iterator i;
   for (i = sorted_ids.begin();
        i != sorted_ids.end(); ++i) {
     string16 printable_selection_text = PrintableSelectionText();
     // Escape "&" as "&&".
-    for (size_t position = printable_selection_text.find('&');
-         position != string16::npos;
-         position = printable_selection_text.find('&', position + 2))
-      printable_selection_text.insert(position, 1, '&');
+    ReplaceChars(printable_selection_text, ASCIIToUTF16("&").c_str(),

[Yoyo Zhou, 2013/02/19 18:34:01]

Try L"&" and L"&&" here.

[Joe Thomas, 2013/02/19 19:17:50]

This works only on platforms like windows where wchar_t is 2 bytes
(http://code.google.com/searchframe#OAMlx_jo-ck/src/base/string16.h&type=cs&l=35).

We get type conversion error on other platforms

[Joe Thomas, 2013/02/19 21:32:11]

Looks like my above explanation is not fully clear. Trying to explain it in a
better way.

ReplaceChars's second argument takes char16.  wchar_t takes 2 bytes on WINDOWS
but 4 bytes on POSIX platforms. And in WINDOWS wchar_t is typedef'ed to char16
(http://code.google.com/searchframe#OAMlx_jo-ck/src/base/string16.h&type=cs&l=35)

So using Unicode string directly compiles only on Windows but not on other
platforms as we lack suitable type conversion for unicode string to char16.

[Yoyo Zhou, 2013/02/19 21:57:02]

Bummer. I guess C++11 initializer lists would be really useful here if we could
use them inline.

The viable alternative I see is defining a helper function EscapeAmpersands, in
which you define

const char16 ampersand[] = {'&', 0};

What do you think about that?

[Joe Thomas, 2013/02/19 22:39:05]

That can be done. Patch updated.

+                 ASCIIToUTF16("&&"), &printable_selection_text);
 
     extension_items_.AppendExtensionItems(i->second, printable_selection_text,
                                           &index);
   }
   UMA_HISTOGRAM_TIMES("Extensions.ContextMenus_BuildTime",
                       base::TimeTicks::Now() - begin);
   UMA_HISTOGRAM_COUNTS("Extensions.ContextMenus_ItemCount", index);
 }
 
 void RenderViewContextMenu::InitMenu() {
@@ skipping 417 matching lines @@
 
   AutocompleteMatch match;
   AutocompleteClassifierFactory::GetForProfile(profile_)->Classify(
       params_.selection_text, string16(), false, false, &match, NULL);
   selection_navigation_url_ = match.destination_url;
   if (!selection_navigation_url_.is_valid())
     return;
 
   string16 printable_selection_text = PrintableSelectionText();
   // Escape "&" as "&&".
-  for (size_t i = printable_selection_text.find('&'); i != string16::npos;
-       i = printable_selection_text.find('&', i + 2))
-    printable_selection_text.insert(i, 1, '&');
+  ReplaceChars(printable_selection_text, ASCIIToUTF16("&").c_str(),
+               ASCIIToUTF16("&&"), &printable_selection_text);
+
 
   if (AutocompleteMatch::IsSearchType(match.type)) {
     const TemplateURL* const default_provider =
         TemplateURLServiceFactory::GetForProfile(profile_)->
         GetDefaultSearchProvider();
     if (!default_provider)
       return;
     menu_model_.AddItem(
         IDC_CONTENT_CONTEXT_SEARCHWEBFOR,
         l10n_util::GetStringFUTF16(IDS_CONTENT_CONTEXT_SEARCHWEBFOR,
@@ skipping 1026 matching lines @@
   source_web_contents_->GetRenderViewHost()->
       ExecuteMediaPlayerActionAtLocation(location, action);
 }
 
 void RenderViewContextMenu::PluginActionAt(
     const gfx::Point& location,
     const WebPluginAction& action) {
   source_web_contents_->GetRenderViewHost()->
       ExecutePluginActionAtLocation(location, action);
 }