Only send C2F ping for a search through homepage.

chrome/browser/rlz/rlz.cc

Index: chrome/browser/rlz/rlz.cc
diff --git a/chrome/browser/rlz/rlz.cc b/chrome/browser/rlz/rlz.cc
index 90223fc1e27650b9148ac354546817b502f753ef..d0845f14e93bfc36984c95eb0ca463a9d330cb8f 100644
--- a/chrome/browser/rlz/rlz.cc
+++ b/chrome/browser/rlz/rlz.cc
@@ -30,6 +30,8 @@
 #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"
@@ -57,6 +59,7 @@ static bool ClearReferral() {
 
 using content::BrowserThread;
 using content::NavigationEntry;
+using content::NavigationController;
 
 namespace {
 
@@ -302,7 +305,7 @@ bool RLZTracker::Init(bool first_run,
 #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)
   }
@@ -421,15 +424,39 @@ void RLZTracker::Observe(int type,
                         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;
     }