Only send C2F ping for a search through homepage.

chrome/browser/rlz/rlz.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.
 //
 // This code glues the RLZ library DLL with Chrome. It allows Chrome to work
 // with or without the DLL being present. If the DLL is not present the
 // functions do nothing and just return false.
 
 #include "chrome/browser/rlz/rlz.h"
 
@@ skipping 12 matching lines @@
 #include "chrome/browser/omnibox/omnibox_log.h"
 #include "chrome/browser/prefs/session_startup_pref.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/ui/startup/startup_browser_creator.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "components/google/core/browser/google_util.h"
 #include "components/search_engines/template_url.h"
 #include "components/search_engines/template_url_service.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/navigation_controller.h"
+#include "content/public/browser/navigation_details.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_service.h"
 #include "net/http/http_util.h"
 
 #if defined(OS_WIN)
 #include "chrome/installer/util/google_update_settings.h"
 #else
 namespace GoogleUpdateSettings {
 static bool GetLanguage(base::string16* language) {
   // TODO(thakis): Implement.
   NOTIMPLEMENTED();
   return false;
 }
 
 // The referral program is defunct and not used. No need to implement these
 // functions on non-Win platforms.
 static bool GetReferral(base::string16* referral) {
   return true;
 }
 static bool ClearReferral() {
   return true;
 }
 }  // namespace GoogleUpdateSettings
 #endif
 
 using content::BrowserThread;
 using content::NavigationEntry;
+using content::NavigationController;
 
 namespace {
 
 // Maximum and minimum delay for financial ping we would allow to be set through
 // master preferences. Somewhat arbitrary, may need to be adjusted in future.
 const base::TimeDelta kMaxInitDelay = base::TimeDelta::FromSeconds(200);
 const base::TimeDelta kMinInitDelay = base::TimeDelta::FromSeconds(20);
 
 bool IsBrandOrganic(const std::string& brand) {
   return brand.empty() || google_brand::IsOrganic(brand);
@@ skipping 225 matching lines @@
 
   if (google_brand::GetBrand(&brand_) && !IsBrandOrganic(brand_)) {
     // Register for notifications from the omnibox so that we can record when
     // the user performs a first search.
     registrar_.Add(this, chrome::NOTIFICATION_OMNIBOX_OPENED_URL,
                    content::NotificationService::AllSources());
 
 #if !defined(OS_IOS)
     // Register for notifications from navigations, to see if the user has used
     // the home page.
-    registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_PENDING,
+    registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
                    content::NotificationService::AllSources());
 #endif  // !defined(OS_IOS)
   }
   google_brand::GetReactivationBrand(&reactivation_brand_);
 
   net::URLRequestContextGetter* context_getter =
       g_browser_process->system_request_context();
 
   // Could be NULL; don't run if so.  RLZ will try again next restart.
   if (context_getter) {
@@ skipping 98 matching lines @@
       // it did previously.  The RLZ folks want RLZ's "first search" detection
       // to remain as unaffected as possible by this change.  This test is
       // there to keep the old behavior.
       if (!content::Details<OmniboxLog>(details).ptr()->is_popup_open)
         break;
       RecordFirstSearch(ChromeOmnibox());
       registrar_.Remove(this, chrome::NOTIFICATION_OMNIBOX_OPENED_URL,
                         content::NotificationService::AllSources());
       break;
 #if !defined(OS_IOS)
-    case content::NOTIFICATION_NAV_ENTRY_PENDING: {
-      const NavigationEntry* entry =
-          content::Details<content::NavigationEntry>(details).ptr();
-      if (entry != NULL &&
-          ((entry->GetTransitionType() &
-            ui::PAGE_TRANSITION_HOME_PAGE) != 0)) {
-        RecordFirstSearch(ChromeHomePage());
-        registrar_.Remove(this, content::NOTIFICATION_NAV_ENTRY_PENDING,
-                          content::NotificationService::AllSources());
+    case content::NOTIFICATION_NAV_ENTRY_COMMITTED: {
+      // Firstly check if it is a Google search.
+      content::LoadCommittedDetails* load_details =
+          content::Details<content::LoadCommittedDetails>(details).ptr();
+      if (load_details == NULL) {
+        return;
+      }

[Roger Tawa OOO till Jul 10th, 2014/09/30 14:43:35]

No need for { and }

[yao, 2014/09/30 17:10:43]

Done.

+
+      NavigationEntry* entry = load_details->entry;
+      if (entry == NULL)
+        return;
+
+      if(google_util::IsGoogleSearchUrl(entry->GetURL())) {
+        // If it is a Google search, check if it originates from HOMEPAGE by
+        // getting the previous NavigationEntry.
+        NavigationController* controller =
+            content::Source<NavigationController>(source).ptr();
+        if (controller == NULL)
+          return;
+
+        int entry_index = controller->GetLastCommittedEntryIndex();
+        if (entry_index < 1)
+          return;
+
+        const NavigationEntry* previous_entry = controller->GetEntryAtIndex(
+            entry_index - 1);
+        if (previous_entry != NULL &&
+                ((previous_entry->GetTransitionType() &
+                  ui::PAGE_TRANSITION_HOME_PAGE) != 0)) {

[Roger Tawa OOO till Jul 10th, 2014/09/30 14:43:35]

Should we also check that the previous entry is a google search url?

[yao, 2014/09/30 17:10:44]

hm.. but the previous entry should not be a google search. 

in an extreme case, the homepage itself could be set to a google search url, and
the user did another search on that page. In this case, we could check to make
sure it's a google homepage, i.e. some form of google.xxx but not a search? 

[Roger Tawa OOO till Jul 10th, 2014/09/30 18:24:36]

How about the case where foo.com is the home page, and then the user clicks on a
google.com link.  Will that be counted as a first search?  I guess the exact
pattern we are looking for is a google *search-result* page following a google
search or search-result page, where the previous page is marked as HOME_PAGE. 
Does that make sense?

[yao, 2014/10/09 15:10:18]

Done.

+            RecordFirstSearch(ChromeHomePage());
+            registrar_.Remove(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
+                              content::NotificationService::AllSources());
+          }

[Roger Tawa OOO till Jul 10th, 2014/09/30 14:43:35]

Should all the return statements above be breaks?

[yao, 2014/09/30 17:10:44]

Done.

       }
       break;
     }
 #endif  // !defined(OS_IOS)
     default:
       NOTREACHED();
       break;
   }
 }
 
@@ skipping 187 matching lines @@
 void RLZTracker::EnableZeroDelayForTesting() {
   GetInstance()->min_init_delay_ = base::TimeDelta();
 }
 
 #if !defined(OS_IOS)
 // static
 void RLZTracker::RecordAppListSearch() {
   GetInstance()->RecordFirstSearch(RLZTracker::ChromeAppList());
 }
 #endif