Only send C2F ping for a search through homepage.

BUG=8424708

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

Cr-Commit-Position: refs/heads/master@{#312421}

5 files changed, 103 insertions, 31 deletions

diff --git a/chrome/browser/rlz/rlz.cc b/chrome/browser/rlz/rlz.cc
index 2bc26fd..d2c8b00 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,43 @@ 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)
+        break;
+
+      NavigationEntry* entry = load_details->entry;
+      if (entry == NULL)
+        break;
+
+      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)
+          break;
+
+        int entry_index = controller->GetLastCommittedEntryIndex();
+        if (entry_index < 1)
+          break;
+
+        const NavigationEntry* previous_entry = controller->GetEntryAtIndex(
+            entry_index - 1);
+
+        if (previous_entry == NULL)
+          break;
+
+        // Make sure it is a Google web page originated from HOMEPAGE.
+        if (google_util::IsGoogleHomePageUrl(previous_entry->GetURL()) &&
+            ((previous_entry->GetTransitionType() &
+                  ui::PAGE_TRANSITION_HOME_PAGE) != 0)) {
+            RecordFirstSearch(ChromeHomePage());
+            registrar_.Remove(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
+                              content::NotificationService::AllSources());
+        }
       }
       break;
     }
diff --git a/chrome/browser/rlz/rlz_unittest.cc b/chrome/browser/rlz/rlz_unittest.cc
index 83c422a..5560a2f 100644
--- a/chrome/browser/rlz/rlz_unittest.cc
+++ b/chrome/browser/rlz/rlz_unittest.cc
@@ -14,19 +14,24 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/installer/util/browser_distribution.h"
 #include "chrome/installer/util/google_update_constants.h"
+#include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "components/metrics/proto/omnibox_event.pb.h"
+#include "content/public/browser/navigation_details.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
+#include "content/public/test/test_renderer_host.h"
 #include "rlz/test/rlz_test_helpers.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
 
 #if defined(OS_WIN)
 #include "base/win/registry.h"
 #endif
 
 using content::NavigationEntry;
+using content::LoadCommittedDetails;
 using testing::AssertionResult;
 using testing::AssertionSuccess;
 using testing::AssertionFailure;
@@ -161,9 +166,10 @@ class TestRLZTracker : public RLZTracker {
   DISALLOW_COPY_AND_ASSIGN(TestRLZTracker);
 };
 
-class RlzLibTest : public RlzLibTestNoMachineState {
+class RlzLibTest : public ChromeRenderViewHostTestHarness {
  protected:
   void SetUp() override;
+  void TearDown() override;
 
   void SetMainBrand(const char* brand);
   void SetReactivationBrand(const char* brand);
@@ -181,13 +187,15 @@ class RlzLibTest : public RlzLibTestNoMachineState {
   void ExpectReactivationRlzPingSent(bool expected);
 
   TestRLZTracker tracker_;
+  RlzLibTestNoMachineStateHelper m_rlz_test_helper_;
 #if defined(OS_POSIX)
   scoped_ptr<google_brand::BrandForTesting> brand_override_;
 #endif
 };
 
 void RlzLibTest::SetUp() {
-  RlzLibTestNoMachineState::SetUp();
+  ChromeRenderViewHostTestHarness::SetUp();
+  m_rlz_test_helper_.SetUp();
 
   // Make sure a non-organic brand code is set in the registry or the RLZTracker
   // is pretty much a no-op.
@@ -195,6 +203,11 @@ void RlzLibTest::SetUp() {
   SetReactivationBrand("");
 }
 
+void RlzLibTest::TearDown() {
+  ChromeRenderViewHostTestHarness::TearDown();
+  m_rlz_test_helper_.TearDown();
+}
+
 void RlzLibTest::SetMainBrand(const char* brand) {
 #if defined(OS_WIN)
   SetRegistryBrandValue(google_update::kRegRLZBrandField, brand);
@@ -250,12 +263,16 @@ void RlzLibTest::SimulateOmniboxUsage() {
 }
 
 void RlzLibTest::SimulateHomepageUsage() {
-  scoped_ptr<NavigationEntry> entry(NavigationEntry::Create());
-  entry->SetPageID(0);
-  entry->SetTransitionType(ui::PAGE_TRANSITION_HOME_PAGE);
-  tracker_.Observe(content::NOTIFICATION_NAV_ENTRY_PENDING,
-                   content::NotificationService::AllSources(),
-                   content::Details<NavigationEntry>(entry.get()));
+  GURL home_url = GURL("https://www.google.com/");
+  GURL search_url = GURL("https://www.google.com/#q=search");
+
+  content::RenderFrameHostTester* rfht =
+      content::RenderFrameHostTester::For(main_rfh());
+
+  // Simulate a navigation to homepage first.
+  rfht->SendNavigateWithTransition(0, home_url, ui::PAGE_TRANSITION_HOME_PAGE);
+  // Then simulate a search from homepage.
+  rfht->SendNavigateWithTransition(1, search_url, ui::PAGE_TRANSITION_LINK);
 }
 
 void RlzLibTest::SimulateAppListUsage() {
@@ -825,15 +842,17 @@ TEST_F(RlzLibTest, PingUpdatesRlzCache) {
 }
 
 TEST_F(RlzLibTest, ObserveHandlesBadArgs) {
-  scoped_ptr<NavigationEntry> entry(NavigationEntry::Create());
-  entry->SetPageID(0);
-  entry->SetTransitionType(ui::PAGE_TRANSITION_LINK);
-  tracker_.Observe(content::NOTIFICATION_NAV_ENTRY_PENDING,
+  scoped_ptr<LoadCommittedDetails> details(new LoadCommittedDetails());
+  details->entry = NavigationEntry::Create();
+  details->entry->SetPageID(0);
+  details->entry->SetTransitionType(ui::PAGE_TRANSITION_LINK);
+
+  tracker_.Observe(content::NOTIFICATION_NAV_ENTRY_COMMITTED,
                    content::NotificationService::AllSources(),
                    content::Details<NavigationEntry>(NULL));
-  tracker_.Observe(content::NOTIFICATION_NAV_ENTRY_PENDING,
+  tracker_.Observe(content::NOTIFICATION_NAV_ENTRY_COMMITTED,
                    content::NotificationService::AllSources(),
-                   content::Details<NavigationEntry>(entry.get()));
+                   content::Details<LoadCommittedDetails>(details.get()));
 }
 
 // TODO(thakis): Reactivation doesn't exist on Mac yet.
diff --git a/rlz/lib/rlz_lib_test.cc b/rlz/lib/rlz_lib_test.cc
index 9b07789..fd6f419 100644
--- a/rlz/lib/rlz_lib_test.cc
+++ b/rlz/lib/rlz_lib_test.cc
@@ -844,7 +844,8 @@ class ReadonlyRlzDirectoryTest : public RlzLibTestNoMachineState {
 void ReadonlyRlzDirectoryTest::SetUp() {
   RlzLibTestNoMachineState::SetUp();
   // Make the rlz directory non-writeable.
-  int chmod_result = chmod(temp_dir_.path().value().c_str(), 0500);
+  int chmod_result = chmod(m_rlz_test_helper_.temp_dir_.path().value().c_str(),
+                           0500);
   ASSERT_EQ(0, chmod_result);
 }
 
diff --git a/rlz/test/rlz_test_helpers.cc b/rlz/test/rlz_test_helpers.cc
index 53c9732..b056a81 100644
--- a/rlz/test/rlz_test_helpers.cc
+++ b/rlz/test/rlz_test_helpers.cc
@@ -125,7 +125,7 @@ void InitializeRegistryOverridesForTesting(
 
 #endif  // defined(OS_WIN)
 
-void RlzLibTestNoMachineState::SetUp() {
+void RlzLibTestNoMachineStateHelper::SetUp() {
 #if defined(OS_WIN)
   InitializeRegistryOverridesForTesting(&override_manager_);
 #elif defined(OS_MACOSX)
@@ -137,12 +137,20 @@ void RlzLibTestNoMachineState::SetUp() {
 #endif  // defined(OS_POSIX)
 }
 
-void RlzLibTestNoMachineState::TearDown() {
+void RlzLibTestNoMachineStateHelper::TearDown() {
 #if defined(OS_POSIX)
   rlz_lib::testing::SetRlzStoreDirectory(base::FilePath());
 #endif  // defined(OS_POSIX)
 }
 
+void RlzLibTestNoMachineState::SetUp() {
+  m_rlz_test_helper_.SetUp();
+}
+
+void RlzLibTestNoMachineState::TearDown() {
+  m_rlz_test_helper_.TearDown();
+}
+
 void RlzLibTestBase::SetUp() {
   RlzLibTestNoMachineState::SetUp();
 #if defined(OS_WIN)
diff --git a/rlz/test/rlz_test_helpers.h b/rlz/test/rlz_test_helpers.h
index 685a358..c02aadc 100644
--- a/rlz/test/rlz_test_helpers.h
+++ b/rlz/test/rlz_test_helpers.h
@@ -18,10 +18,13 @@
 #include "base/test/test_reg_util_win.h"
 #endif
 
-class RlzLibTestNoMachineState : public ::testing::Test {
- protected:
-  void SetUp() override;
-  void TearDown() override;
+// A test helper class that constructs and destructs platform dependent machine
+// state. It's used by src/chrome/browser/rlz/rlz_unittest.cc and
+// src/rlz/lib/rlz_lib_test.cc
+class RlzLibTestNoMachineStateHelper {
+ public:
+  void SetUp();
+  void TearDown();
 
 #if defined(OS_POSIX)
   base::ScopedTempDir temp_dir_;
@@ -32,9 +35,19 @@ class RlzLibTestNoMachineState : public ::testing::Test {
 #endif
 };
 
+class RlzLibTestNoMachineState : public ::testing::Test {
+ protected:
+  void SetUp() override;
+  void TearDown() override;
+
+  RlzLibTestNoMachineStateHelper m_rlz_test_helper_;
+};
+
 class RlzLibTestBase : public RlzLibTestNoMachineState {
  protected:
   void SetUp() override;
+
+  RlzLibTestNoMachineStateHelper m_rlz_test_helper_;
 };
 
 #endif  // RLZ_TEST_RLZ_TEST_HELPERS_H