[InstantExtended] Fixing how PageLoadSRP is emitted. Previously it was emitted only for Instant sea…

chrome/browser/ui/uma_browsing_activity_observer.cc

Index: chrome/browser/ui/uma_browsing_activity_observer.cc
diff --git a/chrome/browser/ui/uma_browsing_activity_observer.cc b/chrome/browser/ui/uma_browsing_activity_observer.cc
index 1db2777fd4a3f6bcbd54173997354c7f88d77eca..eacb3a8e81e4b1713026ec104701075380528491 100644
--- a/chrome/browser/ui/uma_browsing_activity_observer.cc
+++ b/chrome/browser/ui/uma_browsing_activity_observer.cc
@@ -6,14 +6,19 @@
 
 #include "base/metrics/histogram.h"
 #include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/search_engines/template_url_service.h"
+#include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_iterator.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.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 "content/public/browser/render_process_host.h"
+#include "content/public/browser/user_metrics.h"
 
 namespace chrome {
 namespace {
@@ -47,6 +52,24 @@ void UMABrowsingActivityObserver::Observe(
   if (type == content::NOTIFICATION_NAV_ENTRY_COMMITTED) {
     const content::LoadCommittedDetails& load =
         *content::Details<content::LoadCommittedDetails>(details).ptr();
+
+    content::NavigationController* controller =
+        content::Source<content::NavigationController>(source).ptr();
+    if (controller) {

[Mark P, 2013/08/06 19:38:36]

What can cause controller to be NULL here?

[Peter Kasting, 2013/08/06 21:32:19]

I don't think it can be.

[rpetterson, 2013/08/06 23:04:17]

I was being overcautious. Since it's the source, I agree that it can't be NULL
or else what would have sent the notification?

I've pulled out the surrounding if statement.

+      // Track whether the page loaded is a search results page (SRP). Track
+      // the non-SRP navigations as well so there is a control.
+      content::RecordAction(content::UserMetricsAction("NavEntryCommitted"));
+      Profile* profile =
+          Profile::FromBrowserContext(controller->GetBrowserContext());
+      bool is_search_results =
+          TemplateURLServiceFactory::GetForProfile(profile)->
+              IsSearchResultsPageFromDefaultSearchProvider(
+                  load.entry->GetURL());
+      if (is_search_results)

[Mark P, 2013/08/06 19:38:36]

optional nit: consider inlining the alias above rather than using an extra
variable.  Or at least making the alias const.

[rpetterson, 2013/08/06 23:04:17]

Done.

+          content::RecordAction(
+              content::UserMetricsAction("NavEntryCommitted.SRP"));
+    }
+
     if (!load.is_navigation_to_different_page())
       return;  // Don't log for subframes or other trivial types.