Emit correct NewTabPage.SuggestionsType metrics

chrome/browser/ui/search/search_tab_helper.cc

 // Copyright 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/ui/search/search_tab_helper.h"
 
 #include <set>
 
 #include "base/memory/scoped_ptr.h"
 #include "base/metrics/histogram.h"
@@ skipping 379 matching lines @@
 void SearchTabHelper::ThemeInfoChanged(const ThemeBackgroundInfo& theme_info) {
   ipc_router_.SendThemeBackgroundInfo(theme_info);
 }
 
 void SearchTabHelper::MostVisitedItemsChanged(
     const std::vector<InstantMostVisitedItem>& items) {
   // When most visited change, the NTP usually reloads the tiles. This means
   // our metrics get inconsistent. So we'd rather emit stats now.
   InstantTab::EmitNtpStatistics(web_contents_);
   ipc_router_.SendMostVisitedItems(items);
+  this->LogMostVisitedItemsSource(items);

[fserb, 2015/10/13 14:34:40]

no need for "this->" here.

[zmin, 2015/10/13 15:08:38]

Done.

+}
+
+void SearchTabHelper::LogMostVisitedItemsSource(
+    const std::vector<InstantMostVisitedItem>& items) {
+  NTPLoggingEventType event;

[fserb, 2015/10/13 14:34:40]

you can move it inside the for().

[zmin, 2015/10/13 15:08:38]

Talk with @fserb. Moving event and time into the for loop as they are only used
by for loop and  there isn't any performance issue here.

+  base::TimeDelta time = base::TimeDelta();

[fserb, 2015/10/13 14:34:40]

you can just pass 0 as time. I'm not even sure this TimeDelta is what is usually
passed.

[zmin, 2015/10/13 15:08:38]

Talk with @fserb, 0 is the default value of TimeDelta and there is no ctor with
parameters. So no change here

+  for (auto item : items) {
+    if (item.is_server_side_suggestion) {
+      event = NTP_SERVER_SIDE_SUGGESTION;
+    } else {
+      event = NTP_CLIENT_SIDE_SUGGESTION;
+    }
+    // Note that the ntp_user_data_logger.cc now only supports the scenario:
+    // all suggestions are provided by server OR
+    // all suggestions are provided by client

[fserb, 2015/10/13 14:34:40]

nit: "." at the end of the comment.

[zmin, 2015/10/13 15:08:38]

Done.

+    this->OnLogEvent(event, time);
+  }
 }
 
 void SearchTabHelper::OmniboxStartMarginChanged(int omnibox_start_margin) {
   ipc_router_.SetOmniboxStartMargin(omnibox_start_margin);
 }
 
 void SearchTabHelper::FocusOmnibox(OmniboxFocusState state) {
 // TODO(kmadhusu): Move platform specific code from here and get rid of #ifdef.
 #if !defined(OS_ANDROID)
   OmniboxView* omnibox = GetOmniboxView();
@@ skipping 177 matching lines @@
 
 bool SearchTabHelper::IsInputInProgress() const {
   OmniboxView* omnibox = GetOmniboxView();
   return !model_.mode().is_ntp() && omnibox &&
       omnibox->model()->focus_state() == OMNIBOX_FOCUS_VISIBLE;
 }
 
 OmniboxView* SearchTabHelper::GetOmniboxView() const {
   return delegate_ ? delegate_->GetOmniboxView() : NULL;
 }