Add UMA metrics for Android Chrome Google Search.

To analyze the impact of prefetching high-confidence search suggestions in Android Chrome, this CL,
- Adds a listener for counting Google searches from various search access points. No actual search query content is observed.
- Records the search count based on the prerendering settings.

(see trybot results in patchset #8).

BUG=382694
R=asvitkine@chromium.org, davidben@chromium.org, jam@chromium.org, pkasting@chromium.org, samarth@chromium.org

Review URL: https://codereview.chromium.org/342053002

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

chrome/browser/chrome_browser_main_android.cc

@@ -7,6 +7,7 @@
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
 #include "base/path_service.h"
+#include "chrome/browser/google/google_search_counter_android.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "components/breakpad/app/breakpad_linux.h"
@@ -51,6 +52,11 @@ void ChromeBrowserMainPartsAndroid::PreProfileInit() {
   ChromeBrowserMainParts::PreProfileInit();
 }
 
+void ChromeBrowserMainPartsAndroid::PostProfileInit() {
+  search_counter_.reset(new GoogleSearchCounterAndroid(profile()));
+  ChromeBrowserMainParts::PostProfileInit();
+}
+
 void ChromeBrowserMainPartsAndroid::PreEarlyInitialization() {
   TRACE_EVENT0("startup",
     "ChromeBrowserMainPartsAndroid::PreEarlyInitialization")

chrome/browser/chrome_browser_main_android.h

@@ -7,6 +7,8 @@
 
 #include "chrome/browser/chrome_browser_main.h"
 
+class GoogleSearchCounterAndroid;
+
 namespace breakpad {
 class CrashDumpManager;
 }
@@ -19,6 +21,7 @@ class ChromeBrowserMainPartsAndroid : public ChromeBrowserMainParts {
 
   // content::BrowserMainParts overrides.
   virtual void PreProfileInit() OVERRIDE;
+  virtual void PostProfileInit() OVERRIDE;
   virtual void PreEarlyInitialization() OVERRIDE;
 
   // ChromeBrowserMainParts overrides.
@@ -27,6 +30,7 @@ class ChromeBrowserMainPartsAndroid : public ChromeBrowserMainParts {
  private:
   scoped_ptr<base::MessageLoop> main_message_loop_;
   scoped_ptr<breakpad::CrashDumpManager> crash_dump_manager_;
+  scoped_ptr<GoogleSearchCounterAndroid> search_counter_;
 
   DISALLOW_COPY_AND_ASSIGN(ChromeBrowserMainPartsAndroid);
 };

chrome/browser/google/google_search_counter.cc

@@ -12,29 +12,6 @@
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 
-namespace {
-
-// Returns true iff |entry| represents a Google search from the Omnibox.
-// This method assumes that we have already verified that |entry|'s URL is a
-// Google search URL.
-bool IsOmniboxGoogleSearchNavigation(const content::NavigationEntry& entry) {
-  const content::PageTransition stripped_transition =
-      PageTransitionStripQualifier(entry.GetTransitionType());
-  DCHECK(google_util::IsGoogleSearchUrl(entry.GetURL()));
-  return stripped_transition == content::PAGE_TRANSITION_GENERATED;
-}
-
-// Returns true iff |entry| represents a Google search from the Google Search
-// App. This method assumes that we have already verified that |entry|'s URL is
-// a Google search URL.
-bool IsSearchAppGoogleSearchNavigation(const content::NavigationEntry& entry) {
-  DCHECK(google_util::IsGoogleSearchUrl(entry.GetURL()));
-  return entry.GetURL().query().find("source=search_app") !=
-         std::string::npos;
-}
-
-}  // namespace
-
 // static
 void GoogleSearchCounter::RegisterForNotifications() {
   GoogleSearchCounter::GetInstance()->RegisterForNotificationsInternal();
@@ -45,6 +22,36 @@ GoogleSearchCounter* GoogleSearchCounter::GetInstance() {
   return Singleton<GoogleSearchCounter>::get();
 }
 
+GoogleSearchMetrics::AccessPoint
+GoogleSearchCounter::GetGoogleSearchAccessPointForSearchNavEntry(
+    const content::NavigationEntry& entry) const {
+  DCHECK(google_util::IsGoogleSearchUrl(entry.GetURL()));
+
+  // If the |entry| is FROM_ADDRESS_BAR, it comes from the omnibox; if it's
+  // GENERATED, the user was doing a search, rather than doing a navigation to a
+  // search URL (e.g. from hisotry, or pasted in).
+  if (entry.GetTransitionType() == (content::PAGE_TRANSITION_GENERATED |
+      content::PAGE_TRANSITION_FROM_ADDRESS_BAR)) {
+    return GoogleSearchMetrics::AP_OMNIBOX;
+  }
+
+  // The string "source=search_app" in the |entry| URL represents a Google
+  // search from the Google Search App.
+  if (entry.GetURL().query().find("source=search_app") != std::string::npos)
+    return GoogleSearchMetrics::AP_SEARCH_APP;
+
+  // For all other cases that we have not yet implemented or care to measure, we
+  // log a generic "catch-all" metric.
+  return GoogleSearchMetrics::AP_OTHER;
+}
+
+bool GoogleSearchCounter::ShouldRecordCommittedDetails(
+    const content::NotificationDetails& details) const {
+  const content::LoadCommittedDetails* commit =
+      content::Details<content::LoadCommittedDetails>(details).ptr();
+  return google_util::IsGoogleSearchUrl(commit->entry->GetURL());
+}
+
 GoogleSearchCounter::GoogleSearchCounter()
     : search_metrics_(new GoogleSearchMetrics) {
 }
@@ -55,27 +62,15 @@ GoogleSearchCounter::~GoogleSearchCounter() {
 void GoogleSearchCounter::ProcessCommittedEntry(
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
+  // Note that GoogleSearchMetrics logs metrics through UMA, which will only
+  // transmit these counts to the server if the user has opted into sending
+  // usage stats.
   const content::LoadCommittedDetails* commit =
       content::Details<content::LoadCommittedDetails>(details).ptr();
   const content::NavigationEntry& entry = *commit->entry;
-
-  // First see if this is a Google search URL at all.
-  if (!google_util::IsGoogleSearchUrl(entry.GetURL()))
-    return;
-
-  // If the commit is a GENERATED commit with a Google search URL, we know it's
-  // an Omnibox search.
-  if (IsOmniboxGoogleSearchNavigation(entry)) {
-    // Note that GoogleSearchMetrics logs metrics through UMA, which will only
-    // transmit these counts to the server if the user has opted into sending
-    // usage stats.
-    search_metrics_->RecordGoogleSearch(GoogleSearchMetrics::AP_OMNIBOX);
-  } else if (IsSearchAppGoogleSearchNavigation(entry)) {
-    search_metrics_->RecordGoogleSearch(GoogleSearchMetrics::AP_SEARCH_APP);
-  } else {
-    // For all other cases that we have not yet implemented or care to measure,
-    // we log a generic "catch-all" metric.
-    search_metrics_->RecordGoogleSearch(GoogleSearchMetrics::AP_OTHER);
+  if (ShouldRecordCommittedDetails(details)) {
+    search_metrics_->RecordGoogleSearch(
+        GetGoogleSearchAccessPointForSearchNavEntry(entry));
   }
 }
 

chrome/browser/google/google_search_counter.h

@@ -10,6 +10,10 @@
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 
+namespace content {
+class NavigationEntry;
+}
+
 // A listener for counting Google searches from various search access points. No
 // actual search query content is observed. See GoogleSearchMetrics for more
 // details about these access points.
@@ -21,6 +25,21 @@ class GoogleSearchCounter : content::NotificationObserver {
   // Return the singleton instance of GoogleSearchCounter.
   static GoogleSearchCounter* GetInstance();
 
+  // Returns the Google search access point for the given |entry|. This method
+  // assumes that we have already verified that |entry|'s URL is a Google search
+  // URL.
+  GoogleSearchMetrics::AccessPoint GetGoogleSearchAccessPointForSearchNavEntry(
+      const content::NavigationEntry& entry) const;
+
+  // Returns true if |details| is valid and corresponds to a search results
+  // page.
+  bool ShouldRecordCommittedDetails(
+      const content::NotificationDetails& details) const;
+
+  const GoogleSearchMetrics* search_metrics() const {
+    return search_metrics_.get();
+  }
+
  private:
   friend struct DefaultSingletonTraits<GoogleSearchCounter>;
   friend class GoogleSearchCounterTest;

chrome/browser/google/google_search_counter_android.cc

+// Copyright (c) 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 "chrome/browser/google/google_search_counter_android.h"
+
+#include "base/logging.h"
+#include "chrome/browser/google/google_search_counter.h"
+#include "chrome/browser/prerender/prerender_manager.h"
+#include "chrome/browser/prerender/prerender_manager_factory.h"
+#include "components/google/core/browser/google_search_metrics.h"
+#include "content/public/browser/navigation_details.h"
+#include "content/public/browser/navigation_entry.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_types.h"
+
+GoogleSearchCounterAndroid::GoogleSearchCounterAndroid(Profile* profile)
+    : profile_(profile) {
+  // We always listen for all COMMITTED navigations from all sources, as any
+  // one of them could be a navigation of interest.
+  registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
+                 content::NotificationService::AllSources());
+}
+
+GoogleSearchCounterAndroid::~GoogleSearchCounterAndroid() {
+}
+
+void GoogleSearchCounterAndroid::ProcessCommittedEntry(
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  GoogleSearchCounter* counter = GoogleSearchCounter::GetInstance();
+  DCHECK(counter);
+  if (!counter->ShouldRecordCommittedDetails(details))
+    return;
+
+  const content::NavigationEntry& entry =
+      *content::Details<content::LoadCommittedDetails>(details)->entry;
+  prerender::PrerenderManager* prerender_manager =
+      prerender::PrerenderManagerFactory::GetForProfile(profile_);
+  DCHECK(prerender_manager);
+  counter->search_metrics()->RecordAndroidGoogleSearch(
+      counter->GetGoogleSearchAccessPointForSearchNavEntry(entry),
+      prerender_manager->IsEnabled());
+}
+
+void GoogleSearchCounterAndroid::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  DCHECK_EQ(content::NOTIFICATION_NAV_ENTRY_COMMITTED, type);
+  ProcessCommittedEntry(source, details);
+}

chrome/browser/google/google_search_counter_android.h

+// Copyright (c) 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.
+
+#ifndef CHROME_BROWSER_GOOGLE_GOOGLE_SEARCH_COUNTER_ANDROID_H_
+#define CHROME_BROWSER_GOOGLE_GOOGLE_SEARCH_COUNTER_ANDROID_H_
+
+#include "base/macros.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
+
+class Profile;
+
+// A listener for counting Google searches in Android Chrome from various search
+// access points. No actual search query content is observed. See
+// GoogleSearchMetrics for more details about these access points.
+class GoogleSearchCounterAndroid : content::NotificationObserver {
+ public:
+  explicit GoogleSearchCounterAndroid(Profile* profile);
+  virtual ~GoogleSearchCounterAndroid();
+
+ private:
+  void ProcessCommittedEntry(const content::NotificationSource& source,
+                             const content::NotificationDetails& details);
+
+  // content::NotificationObserver:
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
+
+  Profile* profile_;
+  content::NotificationRegistrar registrar_;
+
+  DISALLOW_COPY_AND_ASSIGN(GoogleSearchCounterAndroid);
+};
+
+#endif  // CHROME_BROWSER_GOOGLE_GOOGLE_SEARCH_COUNTER_ANDROID_H_

chrome/browser/google/google_search_counter_unittest.cc

@@ -75,8 +75,11 @@ void GoogleSearchCounterTest::TestGoogleSearch(
   content::LoadCommittedDetails details;
   scoped_ptr<content::NavigationEntry> entry(
       content::NavigationEntry::Create());
-  if (is_omnibox)
-    entry->SetTransitionType(content::PAGE_TRANSITION_GENERATED);
+  if (is_omnibox) {
+    entry->SetTransitionType(content::PageTransitionFromInt(
+        content::PAGE_TRANSITION_GENERATED |
+            content::PAGE_TRANSITION_FROM_ADDRESS_BAR));
+  }
   entry->SetURL(GURL(url));
   details.entry = entry.get();
 

chrome/browser/prerender/prerender_manager.cc

@@ -1630,21 +1630,6 @@ void PrerenderManager::RecordFinalStatusWithoutCreatingPrerenderContents(
       final_status);
 }
 
-bool PrerenderManager::IsEnabled() const {
-  DCHECK(CalledOnValidThread());
-  if (!enabled_)
-    return false;
-  for (std::list<const PrerenderCondition*>::const_iterator it =
-           prerender_conditions_.begin();
-       it != prerender_conditions_.end();
-       ++it) {
-    const PrerenderCondition* condition = *it;
-    if (!condition->CanPrerender())
-      return false;
-  }
-  return true;
-}
-
 void PrerenderManager::Observe(int type,
                                const content::NotificationSource& source,
                                const content::NotificationDetails& details) {
@@ -1880,6 +1865,21 @@ void PrerenderManager::RecordNetworkBytes(Origin origin,
       origin, used, prerender_bytes, recent_profile_bytes);
 }
 
+bool PrerenderManager::IsEnabled() const {
+  DCHECK(CalledOnValidThread());
+  if (!enabled_)
+    return false;
+  for (std::list<const PrerenderCondition*>::const_iterator it =
+           prerender_conditions_.begin();
+       it != prerender_conditions_.end();
+       ++it) {
+    const PrerenderCondition* condition = *it;
+    if (!condition->CanPrerender())
+      return false;
+  }
+  return true;
+}
+
 void PrerenderManager::AddProfileNetworkBytesIfEnabled(int64 bytes) {
   DCHECK_GE(bytes, 0);
   if (IsEnabled() && ActuallyPrerendering())

chrome/browser/prerender/prerender_manager.h

@@ -360,6 +360,9 @@ class PrerenderManager : public base::SupportsWeakPtr<PrerenderManager>,
   // recorded.
   void RecordNetworkBytes(Origin origin, bool used, int64 prerender_bytes);
 
+  // Returns whether prerendering is currently enabled for this manager.
+  bool IsEnabled() const;
+
   // Add to the running tally of bytes transferred over the network for this
   // profile if prerendering is currently enabled.
   void AddProfileNetworkBytesIfEnabled(int64 bytes);
@@ -660,9 +663,6 @@ class PrerenderManager : public base::SupportsWeakPtr<PrerenderManager>,
       const GURL& url, Origin origin, uint8 experiment_id,
       FinalStatus final_status) const;
 
-  // Returns whether prerendering is currently enabled for this manager.
-  // Must be called on the UI thread.
-  bool IsEnabled() const;
 
   void CookieChanged(ChromeCookieDetails* details);
   void CookieChangedAnyCookiesLeftLookupResult(const std::string& domain_key,

chrome/chrome_browser.gypi

@@ -709,6 +709,8 @@
         'browser/google/chrome_google_url_tracker_client.h',
         'browser/google/google_search_counter.cc',
         'browser/google/google_search_counter.h',
+        'browser/google/google_search_counter_android.cc',
+        'browser/google/google_search_counter_android.h',
         'browser/google/google_profile_helper.cc',
         'browser/google/google_profile_helper.h',
         'browser/google/google_update_settings_posix.cc',

components/google/core/browser/google_search_metrics.cc

@@ -17,3 +17,18 @@ void GoogleSearchMetrics::RecordGoogleSearch(AccessPoint ap) const {
   DCHECK_NE(AP_BOUNDARY, ap);
   UMA_HISTOGRAM_ENUMERATION("GoogleSearch.AccessPoint", ap, AP_BOUNDARY);
 }
+
+#if defined(OS_ANDROID)
+void GoogleSearchMetrics::RecordAndroidGoogleSearch(
+    AccessPoint ap,
+    bool prerender_enabled) const {
+  DCHECK_NE(AP_BOUNDARY, ap);
+  if (prerender_enabled) {
+    UMA_HISTOGRAM_ENUMERATION("GoogleSearch.AccessPoint_PrerenderEnabled",
+                              ap, AP_BOUNDARY);
+  } else {
+    UMA_HISTOGRAM_ENUMERATION("GoogleSearch.AccessPoint_PrerenderDisabled",
+                              ap, AP_BOUNDARY);
+  }
+}
+#endif

components/google/core/browser/google_search_metrics.h

@@ -5,6 +5,8 @@
 #ifndef COMPONENTS_GOOGLE_CORE_BROWSER_GOOGLE_SEARCH_METRICS_H_
 #define COMPONENTS_GOOGLE_CORE_BROWSER_GOOGLE_SEARCH_METRICS_H_
 
+#include "build/build_config.h"
+
 // A thin helper class used by parties interested in reporting Google search
 // metrics (mostly counts of searches from different access points). This class
 // partly exists to make testing easier.
@@ -31,6 +33,13 @@ class GoogleSearchMetrics {
 
   // Record a single Google search from source |ap|.
   virtual void RecordGoogleSearch(AccessPoint ap) const;
+
+#if defined(OS_ANDROID)
+  // Record a single Android Google search from source |ap|. |prerender_enabled|
+  // is set to true when prerendering is enabled via settings.
+  virtual void RecordAndroidGoogleSearch(AccessPoint ap,
+                                         bool prerender_enabled) const;
+#endif
 };
 
 #endif  // COMPONENTS_GOOGLE_CORE_BROWSER_GOOGLE_SEARCH_METRICS_H_

tools/metrics/histograms/histograms.xml

@@ -8178,6 +8178,13 @@ Therefore, the affected-histogram name has to have at least one dot in it.
   </summary>
 </histogram>
 
+<histogram name="GoogleSearch.AccessPoint" enum="SearchAccessPoint">
+  <owner>kmadhusu@chromium.org</owner>
+  <summary>
+    Counts number of Google searches from various access points in the browser.
+  </summary>
+</histogram>
+
 <histogram name="GoogleUpdate.EffectivePolicy" enum="UpdatePolicy">
   <owner>gab@chromium.org</owner>
   <summary>
@@ -43656,6 +43663,19 @@ Therefore, the affected-histogram name has to have at least one dot in it.
   <int value="3" label="SCT_STATUS_OK"/>
 </enum>
 
+<enum name="SearchAccessPoint" type="int">
+  <int value="0" label="Omnibox"/>
+  <int value="1" label="Omnibox Instant"/>
+  <int value="2" label="Direct Navigation"/>
+  <int value="3" label="Direct Navigation Instant"/>
+  <int value="4" label="Home Page"/>
+  <int value="5" label="Home Page Instant"/>
+  <int value="6" label="Search App"/>
+  <int value="7" label="Search App Instant"/>
+  <int value="8" label="Other"/>
+  <int value="9" label="Other Instant"/>
+</enum>
+
 <enum name="SearchEngine" type="int">
   <obsolete>
     Deprecated 8/2013. No longer generated.
@@ -46385,6 +46405,21 @@ Therefore, the affected-histogram name has to have at least one dot in it.
   <affected-histogram name="PLT.PT_StartToFinish"/>
 </histogram_suffixes>
 
+<histogram_suffixes name="GoogleSearchVariations">
+  <owner>kmadhusu@chromium.org</owner>
+  <suffix name="_PrerenderDisabled"
+      label="Counts number of Google searches from various access points in
+             the Android Chrome browser when prerendering is disabled via
+             &quot;Bandwidth management&quot; settings or &quot;Privacy&quot;
+             settings. Only recorded on Android."/>
+  <suffix name="_PrerenderEnabled"
+      label="Counts number of Google searches from various access points in
+             the Android Chrome browser when prerendering is enabled via
+             &quot;Bandwidth management&quot; settings or &quot;Privacy&quot;
+             settings. Only recorded on Android."/>
+  <affected-histogram name="GoogleSearch.AccessPoint"/>
+</histogram_suffixes>
+
 <histogram_suffixes name="GWSChromeJointExperiment">
   <suffix name="Experiment1"
       label="Only page loads that are a result of a navigation from a web