Removing un-necessary conversion from std::string to base::string16.

chrome/browser/metrics/plugin_metrics_provider.cc

Index: chrome/browser/metrics/plugin_metrics_provider.cc
diff --git a/chrome/browser/metrics/plugin_metrics_provider.cc b/chrome/browser/metrics/plugin_metrics_provider.cc
index acc5fa2de5c4ee86d252bded3f7764afbccfc910..19ff98a6441c34fbdf1401d23a85214191333637 100644
--- a/chrome/browser/metrics/plugin_metrics_provider.cc
+++ b/chrome/browser/metrics/plugin_metrics_provider.cc
@@ -224,14 +224,12 @@ void PluginMetricsProvider::RecordCurrentState() {
       continue;
     }
 
-    // TODO(viettrungluu): remove conversions
-    base::string16 name16 = base::UTF8ToUTF16(plugin_name);
-    if (child_process_stats_buffer_.find(name16) ==
+    if (child_process_stats_buffer_.find(plugin_name) ==
         child_process_stats_buffer_.end()) {
       continue;
     }
 
-    ChildProcessStats stats = child_process_stats_buffer_[name16];
+    ChildProcessStats stats = child_process_stats_buffer_[plugin_name];
     if (stats.process_launches) {
       int launches = 0;
       plugin_dict->GetInteger(prefs::kStabilityPluginLaunches, &launches);
@@ -259,12 +257,12 @@ void PluginMetricsProvider::RecordCurrentState() {
                               loading_errors);
     }
 
-    child_process_stats_buffer_.erase(name16);
+    child_process_stats_buffer_.erase(plugin_name);
   }
 
   // Now go through and add dictionaries for plugins that didn't already have
   // reports in Local State.
-  for (std::map<base::string16, ChildProcessStats>::iterator cache_iter =
+  for (std::map<std::string, ChildProcessStats>::iterator cache_iter =

[Ilya Sherman, 2015/10/16 22:09:33]

nit: Please update this type to be "auto"

[Deepak, 2015/10/18 05:59:41]

Done.

            child_process_stats_buffer_.begin();
        cache_iter != child_process_stats_buffer_.end(); ++cache_iter) {
     ChildProcessStats stats = cache_iter->second;
@@ -273,12 +271,9 @@ void PluginMetricsProvider::RecordCurrentState() {
     if (!IsPluginProcess(stats.process_type))
       continue;
 
-    // TODO(viettrungluu): remove conversion
-    std::string plugin_name = base::UTF16ToUTF8(cache_iter->first);
-
     base::DictionaryValue* plugin_dict = new base::DictionaryValue;
 
-    plugin_dict->SetString(prefs::kStabilityPluginName, plugin_name);
+    plugin_dict->SetString(prefs::kStabilityPluginName, cache_iter->first);
     plugin_dict->SetInteger(prefs::kStabilityPluginLaunches,
                             stats.process_launches);
     plugin_dict->SetInteger(prefs::kStabilityPluginCrashes,
@@ -299,7 +294,8 @@ void PluginMetricsProvider::LogPluginLoadingError(
       content::PluginService::GetInstance()->GetPluginInfoByPath(plugin_path,
                                                                  &plugin);
   DCHECK(success);
-  ChildProcessStats& stats = child_process_stats_buffer_[plugin.name];
+  ChildProcessStats& stats =
+      child_process_stats_buffer_[base::UTF16ToUTF8(plugin.name)];
   // Initialize the type if this entry is new.
   if (stats.process_type == content::PROCESS_TYPE_UNKNOWN) {
     // The plugin process might not actually be of type PLUGIN (which means
@@ -339,7 +335,7 @@ void PluginMetricsProvider::OnGotPlugins(
 PluginMetricsProvider::ChildProcessStats&
 PluginMetricsProvider::GetChildProcessStats(

[Ilya Sherman, 2015/10/16 22:09:33]

This method is fairly frequently called, so I'm not sure that it helps to trade
string conversions in a different place for string conversions here.  Best would
be to simply not have string conversions.

[Deepak, 2015/10/18 05:59:41]

I understand, It is best if we can avoid all string conversions.
But as kStabilityPluginName i.e plugin name should be stored in the dictionary
as std::string.
and plugin name in ChildProcessStats and in WebPluginInfo is base:string16.
So we have to do string conversion.

PluginMetricsProvider::RecordCurrentState() is also called frequently and here
we are doing conversion for every plugin in the map in 2 "for loops". that we
tried to avoid.
But again I completely agree with you,that it is trade off.
Please correct me if I misunderstood something.

[Ilya Sherman, 2015/10/20 06:24:32]

What makes you say that PluginMetricsProvider::RecordCurrentState() is
frequently called?

Regardless, I think the point of this TODO was to avoid all string conversions,
not just to trade some for others.  If I had to guess, I'd guess that the idea
was to unify the representation to consistently be either UTF-8 or UTF-16.

     const content::ChildProcessData& data) {
-  const base::string16& child_name = data.name;
+  const std::string& child_name = base::UTF16ToUTF8(data.name);
   if (!ContainsKey(child_process_stats_buffer_, child_name)) {
     child_process_stats_buffer_[child_name] =
         ChildProcessStats(data.process_type);