Omnibox metrics logging patch splitout, part 1: Random miscellaneous small style and similar changes. In one or two cases the motivation is not obvious without looking at the original patch.

Review URL: http://codereview.chromium.org/10822

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@5261 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 70 insertions, 86 deletions

diff --git a/chrome/browser/autocomplete/history_contents_provider.cc b/chrome/browser/autocomplete/history_contents_provider.cc
index 9c50bd3..1d3ed28 100644
--- a/chrome/browser/autocomplete/history_contents_provider.cc
+++ b/chrome/browser/autocomplete/history_contents_provider.cc
@@ -22,6 +22,11 @@ const int kDaysToSearch = 30;
 // a single result. It allows the results to be sorted and processed without
 // modifying the larger and slower results structure.
 struct MatchReference {
+  MatchReference(const history::URLResult* result, int relevance)
+      : result(result),
+        relevance(relevance) {
+  }
+
   const history::URLResult* result;
   int relevance;  // Score of relevance computed by CalculateRelevance.
 };
@@ -144,9 +149,7 @@ void HistoryContentsProvider::ConvertResults() {
   std::vector<MatchReference> result_refs;
   result_refs.reserve(results_.size());
   for (size_t i = 0; i < results_.size(); i++) {
-    MatchReference ref;
-    ref.result = &results_[i];
-    ref.relevance = CalculateRelevance(*ref.result);
+    MatchReference ref(&results_[i], CalculateRelevance(results_[i]));
     result_refs.push_back(ref);
   }
 
@@ -180,6 +183,10 @@ void HistoryContentsProvider::ConvertResults() {
       matches_[i].relevance = -matches_[i].relevance;
 }
 
+static bool MatchInTitle(const history::URLResult& result) {
+  return !result.title_match_positions().empty();
+}
+
 AutocompleteMatch HistoryContentsProvider::ResultToMatch(
     const history::URLResult& result,
     int score) {
@@ -227,8 +234,8 @@ void HistoryContentsProvider::ClassifyDescription(
 
 int HistoryContentsProvider::CalculateRelevance(
     const history::URLResult& result) {
-  bool in_title = !!result.title_match_positions().size();
-  bool is_starred =
+  const bool in_title = MatchInTitle(result);
+  const bool is_starred =
       (profile_->GetBookmarkModel() &&
        profile_->GetBookmarkModel()->IsBookmarked(result.url()));
 
@@ -236,22 +243,18 @@ int HistoryContentsProvider::CalculateRelevance(
     case AutocompleteInput::UNKNOWN:
     case AutocompleteInput::REQUESTED_URL:
       if (is_starred) {
-        return in_title ? 1000 + star_title_count_++ :
-                          550 + star_contents_count_++;
-      } else {
-        return in_title ? 700 + title_count_++ :
-                          500 + contents_count_++;
+        return in_title ?
+            (1000 + star_title_count_++) : (550 + star_contents_count_++);
       }
+      return in_title ? (700 + title_count_++) : (500 + contents_count_++);
 
     case AutocompleteInput::QUERY:
     case AutocompleteInput::FORCED_QUERY:
       if (is_starred) {
-        return in_title ? 1200 + star_title_count_++ :
-                          750 + star_contents_count_++;
-      } else {
-        return in_title ? 900 + title_count_++ :
-                          700 + contents_count_++;
+        return in_title ?
+            (1200 + star_title_count_++) : (750 + star_contents_count_++);
       }
+      return in_title ? (900 + title_count_++) : (700 + contents_count_++);
 
     default:
       NOTREACHED();
diff --git a/chrome/browser/autocomplete/keyword_provider.cc b/chrome/browser/autocomplete/keyword_provider.cc
index 4674d85..7c9f55b 100644
--- a/chrome/browser/autocomplete/keyword_provider.cc
+++ b/chrome/browser/autocomplete/keyword_provider.cc
@@ -227,7 +227,7 @@ int KeywordProvider::CalculateRelevance(AutocompleteInput::Type type,
 }
 
 AutocompleteMatch KeywordProvider::CreateAutocompleteMatch(
-    TemplateURLModel *model,
+    TemplateURLModel* model,
     const std::wstring keyword,
     const AutocompleteInput& input,
     size_t prefix_length,
diff --git a/chrome/browser/autocomplete/search_provider.cc b/chrome/browser/autocomplete/search_provider.cc
index b7b260e..96fd52f 100644
--- a/chrome/browser/autocomplete/search_provider.cc
+++ b/chrome/browser/autocomplete/search_provider.cc
@@ -360,11 +360,11 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() {
   for (MatchMap::const_iterator i(map.begin()); i != map.end(); ++i)
     matches_.push_back(i->second);
 
-  if (navigation_results_.size()) {
+  if (!navigation_results_.empty()) {
     // TODO(kochi): http://b/1170574  We add only one results for navigational
     // suggestions. If we can get more useful information about the score,
     // consider adding more results.
-    matches_.push_back(NavigationToMatch(navigation_results_[0],
+    matches_.push_back(NavigationToMatch(navigation_results_.front(),
                                          CalculateRelevanceForNavigation(0)));
   }
 
diff --git a/chrome/browser/metrics_log.cc b/chrome/browser/metrics_log.cc
index 908db3e..350f5b6 100644
--- a/chrome/browser/metrics_log.cc
+++ b/chrome/browser/metrics_log.cc
@@ -129,14 +129,13 @@ void MetricsLog::RecordUserAction(const wchar_t* key) {
     return;
   }
 
-  StartElement("uielement");
+  OPEN_ELEMENT_FOR_SCOPE("uielement");
   WriteAttribute("action", "command");
   WriteAttribute("targetidhash", command_hash);
 
   // TODO(jhughes): Properly track windows.
   WriteIntAttribute("window", 0);
   WriteCommonEventAttributes();
-  EndElement();
 
   ++num_events_;
 }
@@ -148,7 +147,7 @@ void MetricsLog::RecordLoadEvent(int window_id,
                                  TimeDelta load_time) {
   DCHECK(!locked_);
 
-  StartElement("document");
+  OPEN_ELEMENT_FOR_SCOPE("document");
   WriteAttribute("action", "load");
   WriteIntAttribute("docid", session_index);
   WriteIntAttribute("window", window_id);
@@ -197,44 +196,21 @@ void MetricsLog::RecordLoadEvent(int window_id,
     WriteAttribute("origin", origin_string);
 
   WriteCommonEventAttributes();
-  EndElement();
 
   ++num_events_;
 }
 
-// static
-const char* MetricsLog::WindowEventTypeToString(WindowEventType type) {
-  switch (type) {
-    case WINDOW_CREATE:
-      return "create";
-
-    case WINDOW_OPEN:
-      return "open";
-
-    case WINDOW_CLOSE:
-      return "close";
-
-    case WINDOW_DESTROY:
-      return "destroy";
-
-    default:
-      NOTREACHED();
-      return "unknown";
-  }
-}
-
 void MetricsLog::RecordWindowEvent(WindowEventType type,
                                    int window_id,
                                    int parent_id) {
   DCHECK(!locked_);
 
-  StartElement("window");
+  OPEN_ELEMENT_FOR_SCOPE("window");
   WriteAttribute("action", WindowEventTypeToString(type));
   WriteAttribute("windowid", IntToString(window_id));
   if (parent_id >= 0)
     WriteAttribute("parent", IntToString(parent_id));
   WriteCommonEventAttributes();
-  EndElement();
 
   ++num_events_;
 }
@@ -268,6 +244,21 @@ void MetricsLog::WriteInt64Attribute(const std::string& name, int64 value) {
   WriteAttribute(name, Int64ToString(value));
 }
 
+// static
+const char* MetricsLog::WindowEventTypeToString(WindowEventType type) {
+  switch (type) {
+    case WINDOW_CREATE:  return "create";
+    case WINDOW_OPEN:    return "open";
+    case WINDOW_CLOSE:   return "close";
+    case WINDOW_DESTROY: return "destroy";
+
+    default:
+      NOTREACHED();
+      return "unknown";  // Can't return NULL as this is used in a required
+                         // attribute.
+  }
+}
+
 void MetricsLog::StartElement(const char* name) {
   DCHECK(!locked_);
   DCHECK(name);
@@ -318,7 +309,7 @@ void MetricsLog::WriteStabilityElement() {
   // NOTE: This could lead to some data loss if this report isn't successfully
   //       sent, but that's true for all the metrics.
 
-  StartElement("stability");
+  OPEN_ELEMENT_FOR_SCOPE("stability");
 
   WriteIntAttribute("launchcount",
                     pref->GetInteger(prefs::kStabilityLaunchCount));
@@ -361,7 +352,7 @@ void MetricsLog::WriteStabilityElement() {
   const ListValue* plugin_stats_list = pref->GetList(
       prefs::kStabilityPluginStats);
   if (plugin_stats_list) {
-    StartElement("plugins");
+    OPEN_ELEMENT_FOR_SCOPE("plugins");
     for (ListValue::const_iterator iter = plugin_stats_list->begin();
          iter != plugin_stats_list->end(); ++iter) {
       if (!(*iter)->IsType(Value::TYPE_DICTIONARY)) {
@@ -378,7 +369,7 @@ void MetricsLog::WriteStabilityElement() {
         continue;
       }
 
-      StartElement("pluginstability");
+      OPEN_ELEMENT_FOR_SCOPE("pluginstability");
       WriteAttribute("filename", CreateBase64Hash(WideToUTF8(plugin_path)));
 
       int launches = 0;
@@ -392,25 +383,21 @@ void MetricsLog::WriteStabilityElement() {
       int crashes = 0;
       plugin_dict->GetInteger(prefs::kStabilityPluginCrashes, &crashes);
       WriteIntAttribute("crashcount", crashes);
-      EndElement();
     }
 
     pref->ClearPref(prefs::kStabilityPluginStats);
-    EndElement();
   }
-
-  EndElement();
 }
 
 void MetricsLog::WritePluginList(
          const std::vector<WebPluginInfo>& plugin_list) {
   DCHECK(!locked_);
 
-  StartElement("plugins");
+  OPEN_ELEMENT_FOR_SCOPE("plugins");
 
   for (std::vector<WebPluginInfo>::const_iterator iter = plugin_list.begin();
        iter != plugin_list.end(); ++iter) {
-    StartElement("plugin");
+    OPEN_ELEMENT_FOR_SCOPE("plugin");
 
     // Plugin name and filename are hashed for the privacy of those
     // testing unreleased new extensions.
@@ -419,11 +406,7 @@ void MetricsLog::WritePluginList(
     WriteAttribute("filename", CreateBase64Hash(WideToUTF8(filename)));
 
     WriteAttribute("version", WideToUTF8((*iter).version));
-
-    EndElement();
   }
-
-  EndElement();
 }
 
 void MetricsLog::RecordEnvironment(
@@ -433,14 +416,15 @@ void MetricsLog::RecordEnvironment(
 
   PrefService* pref = g_browser_process->local_state();
 
-  StartElement("profile");
+  OPEN_ELEMENT_FOR_SCOPE("profile");
   WriteCommonEventAttributes();
 
-  StartElement("install");
-  WriteAttribute("installdate", GetInstallDate());
-  WriteIntAttribute("buildid", 0);  // means that we're using appversion instead
-  WriteAttribute("appversion", GetVersionString());
-  EndElement();
+  {
+    OPEN_ELEMENT_FOR_SCOPE("install");
+    WriteAttribute("installdate", GetInstallDate());
+    WriteIntAttribute("buildid", 0);  // We're using appversion instead.
+    WriteAttribute("appversion", GetVersionString());
+  }
 
   WritePluginList(plugin_list);
 
@@ -519,8 +503,6 @@ void MetricsLog::RecordEnvironment(
 
   if (profile_metrics)
     WriteAllProfilesMetrics(*profile_metrics);
-
-  EndElement();  // profile
 }
 
 void MetricsLog::WriteAllProfilesMetrics(
@@ -588,31 +570,30 @@ void MetricsLog::WriteProfileMetrics(const std::wstring& profileidhash,
 void MetricsLog::RecordOmniboxOpenedURL(const AutocompleteLog& log) {
   DCHECK(!locked_);
 
-  StartElement("uielement");
+  OPEN_ELEMENT_FOR_SCOPE("uielement");
   WriteAttribute("action", "autocomplete");
   WriteAttribute("targetidhash", "");
   // TODO(kochi): Properly track windows.
   WriteIntAttribute("window", 0);
   WriteCommonEventAttributes();
 
-  StartElement("autocomplete");
-
-  WriteIntAttribute("typedlength", static_cast<int>(log.text.length()));
-  WriteIntAttribute("completedlength",
-                    static_cast<int>(log.inline_autocompleted_length));
-  WriteIntAttribute("selectedindex", static_cast<int>(log.selected_index));
-
-  for (AutocompleteResult::const_iterator i(log.result.begin());
-       i != log.result.end(); ++i) {
-    StartElement("autocompleteitem");
-    if (i->provider)
-      WriteAttribute("provider", i->provider->name());
-    WriteIntAttribute("relevance", i->relevance);
-    WriteIntAttribute("isstarred", i->starred ? 1 : 0);
-    EndElement();  // autocompleteitem
+  {
+    OPEN_ELEMENT_FOR_SCOPE("autocomplete");
+
+    WriteIntAttribute("typedlength", static_cast<int>(log.text.length()));
+    WriteIntAttribute("selectedindex", static_cast<int>(log.selected_index));
+    WriteIntAttribute("completedlength",
+                      static_cast<int>(log.inline_autocompleted_length));
+
+    for (AutocompleteResult::const_iterator i(log.result.begin());
+         i != log.result.end(); ++i) {
+      OPEN_ELEMENT_FOR_SCOPE("autocompleteitem");
+      if (i->provider)
+        WriteAttribute("provider", i->provider->name());
+      WriteIntAttribute("relevance", i->relevance);
+      WriteIntAttribute("isstarred", i->starred ? 1 : 0);
+    }
   }
-  EndElement();  // autocomplete
-  EndElement();  // uielement
 
   ++num_events_;
 }
diff --git a/chrome/browser/metrics_log.h b/chrome/browser/metrics_log.h
index 1be1d9f..225e2a3 100644
--- a/chrome/browser/metrics_log.h
+++ b/chrome/browser/metrics_log.h
@@ -44,8 +44,6 @@ class MetricsLog {
     WINDOW_DESTROY
   };
 
-  static const char* WindowEventTypeToString(WindowEventType type);
-
   void RecordWindowEvent(WindowEventType type, int window_id, int parent_id);
 
   // Records a page load.
@@ -129,6 +127,8 @@ class MetricsLog {
   };
   friend class ScopedElement;
 
+  static const char* WindowEventTypeToString(WindowEventType type);
+
   // Convenience versions of xmlWriter functions
   void StartElement(const char* name);
   void EndElement();