Log NTP hovers in 1993 clients

chrome/browser/ui/search/instant_controller.cc

 // Copyright 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.
 
 #include "chrome/browser/ui/search/instant_controller.h"
 
 #include <iterator>
 
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_service.h"
@@ skipping 25 matching lines @@
 #include "chrome/common/content_settings_types.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "components/sessions/serialized_navigation_entry.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/render_widget_host_view.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_user_data.h"
 #include "content/public/browser/web_contents_view.h"
 #include "net/base/escape.h"
 #include "net/base/network_change_notifier.h"
 #include "third_party/icu/public/common/unicode/normalizer2.h"
 
 #if defined(TOOLKIT_VIEWS)
 #include "ui/views/widget/widget.h"
 #endif
 
 namespace {
@@ skipping 187 matching lines @@
 template <typename T>
 void DeletePageSoon(scoped_ptr<T> page) {
   if (page->contents()) {
     base::MessageLoop::current()->DeleteSoon(
         FROM_HERE, page->ReleaseContents().release());
   }
 
   base::MessageLoop::current()->DeleteSoon(FROM_HERE, page.release());
 }
 
+// Helper class for logging data from the NTP. Attached to each InstantNTP

[Jered, 2013/06/26 16:37:57]

instant_controller.cc feels like the wrong place for this class. Since it's
bound on NTP creation, how about instant_ntp.cc, from InitContents perhaps.

[annark1, 2013/06/28 15:30:49]

Done.

+// instance.
+class NtpLoggingUserData

[Jered, 2013/06/26 16:37:57]

Ntp -> NTP.

[annark1, 2013/06/28 15:30:49]

Done.

+    : public content::WebContentsUserData<NtpLoggingUserData> {
+ public:
+  virtual ~NtpLoggingUserData() {}
+
+  // Called each time the mouse hovers over an iframe or title.
+  void increment_number_of_mouseovers() {
+    number_of_mouseovers++;
+  }
+
+  // Logs total number of mouseovers per NTP session to UMA histogram. Called
+  // when a tab is about to be deactivated (be it by switching tabs, losing
+  // focus or closing the tab/shutting down Chrome) or when the contents of the
+  // current tab is replaced. Does not log 0 mouseover cases (for instance if a

[Jered, 2013/06/26 16:37:57]

Why not log the case with 0 hovers? This seems like interesting counterfactual
information. For example, I think it'd be useful to know how many interactions
had hovers at all to assess whether this is a common user action.

[annark1, 2013/06/28 15:30:49]

After offline discussions with Jered and Mark Pearson, I've now changed this so
we do log 0 mouseover cases.

On 2013/06/26 16:37:57, Jered wrote:

+  // user opens the NTP then immediately switches to another tab without
+  // interacting with the NTP).
+  void log_number_of_mouseovers() {
+    if (number_of_mouseovers > 0) {
+      UMA_HISTOGRAM_COUNTS("NewTabPage.NumberOfMouseOvers",
+                           number_of_mouseovers);
+      number_of_mouseovers = 0;
+    }
+  }
+
+ private:
+  explicit NtpLoggingUserData(content::WebContents* contents)
+      : number_of_mouseovers(0) {}
+  friend class content::WebContentsUserData<NtpLoggingUserData>;
+
+  int number_of_mouseovers;

[Jered, 2013/06/26 16:37:57]

Member variables should end with _.

[annark1, 2013/06/28 15:30:49]

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(NtpLoggingUserData);
+};
+
 }  // namespace
 
+DEFINE_WEB_CONTENTS_USER_DATA_KEY(NtpLoggingUserData);
+
 InstantController::InstantController(BrowserInstantController* browser,
                                      bool extended_enabled)
     : browser_(browser),
       extended_enabled_(extended_enabled),
       instant_enabled_(false),
       use_local_page_only_(true),
       preload_ntp_(true),
       model_(this),
       use_tab_for_suggestions_(false),
       last_omnibox_text_has_inline_autocompletion_(false),
@@ skipping 790 matching lines @@
     ResetInstantTab();
 }
 
 void InstantController::TabDeactivated(content::WebContents* contents) {
   LOG_INSTANT_DEBUG_EVENT(this, "TabDeactivated");
   if (extended_enabled() && !contents->IsBeingDestroyed())
     CommitIfPossible(INSTANT_COMMIT_FOCUS_LOST);
 
   if (GetOverlayContents())
     HideOverlay();
+
+  if (NtpLoggingUserData::FromWebContents(contents))
+    NtpLoggingUserData::FromWebContents(contents)->log_number_of_mouseovers();
 }
 
 void InstantController::SetInstantEnabled(bool instant_enabled,
                                           bool use_local_page_only) {
   LOG_INSTANT_DEBUG_EVENT(this, base::StringPrintf(
       "SetInstantEnabled: instant_enabled=%d, use_local_page_only=%d",
       instant_enabled, use_local_page_only));
 
   // Non extended mode does not care about |use_local_page_only|.
   if (instant_enabled == instant_enabled_ &&
@@ skipping 520 matching lines @@
     return;
 
   // Instant NTP is only used in extended mode so we should always have a
   // non-empty URL to use.
   DCHECK(!instant_url.empty());
   ntp_.reset(new InstantNTP(this, instant_url,
                             browser_->profile()->IsOffTheRecord()));
   ntp_->InitContents(profile(), browser_->GetActiveWebContents(),
                      base::Bind(&InstantController::ReloadStaleNTP,
                                 base::Unretained(this)));
+
+  if (ntp_->contents())
+    NtpLoggingUserData::CreateForWebContents(ntp_->contents());
+
   LOG_INSTANT_DEBUG_EVENT(this, base::StringPrintf(
       "ResetNTP: instant_url='%s'", instant_url.c_str()));
 }
 
 void InstantController::ReloadStaleNTP() {
   ResetNTP(GetInstantURL());
 }
 
 bool InstantController::ShouldSwitchToLocalNTP() const {
   if (!ntp())
@@ skipping 53 matching lines @@
       instant_tab_->Init(active_tab);
       UpdateInfoForInstantTab();
       use_tab_for_suggestions_ = true;
     }
 
     // Hide the |overlay_| since we are now using |instant_tab_| instead.
     HideOverlay();
   } else {
     instant_tab_.reset();
   }
+
+  if (NtpLoggingUserData::FromWebContents(browser_->GetActiveWebContents())) {

[Jered, 2013/06/26 16:37:57]

Why are you calling this here?

[annark1, 2013/06/28 15:30:49]

I was calling this here to capture the cases when the user navigates to a URL
from the NTP but it was called more often than was needed, hence why before I
was not logging 0 mouseover cases. My new patchset  adds a WebContentsObserver
to the NTPLoggingUserData class, whose function NavigationEntryCommitted is used
to trigger the log_number_of_mouseover function when the user navigates to URLs
from the NTP. So it's no longer necessary to call the function here.

On 2013/06/26 16:37:57, Jered wrote:

+    NtpLoggingUserData::FromWebContents(
+        browser_->GetActiveWebContents())->log_number_of_mouseovers();
+  }
 }
 
 void InstantController::UpdateInfoForInstantTab() {
   if (instant_tab_) {
     instant_tab_->sender()->SetDisplayInstantResults(instant_enabled_);
     instant_tab_->sender()->SetOmniboxBounds(omnibox_bounds_);
 
     // Update theme details.
     InstantService* instant_service = GetInstantService();
     if (instant_service)
@@ skipping 205 matching lines @@
   result->relevance = match.relevance;
   result->autocomplete_match_index = autocomplete_match_index;
 
   DVLOG(1) << "    " << result->relevance << " "
       << UTF8ToUTF16(AutocompleteMatchType::ToString(result->type)) << " "
       << result->provider << " " << result->destination_url << " '"
       << result->description << "' '" << result->search_query << "' "
       << result->transition <<  " " << result->autocomplete_match_index;
 }
 
+void InstantController::LogIframeHover() {
+  if (!browser_ || !browser_->GetActiveWebContents())
+    return;
+
+  if(NtpLoggingUserData::FromWebContents(browser_->GetActiveWebContents())) {
+    NtpLoggingUserData::FromWebContents(
+        browser_->GetActiveWebContents())->increment_number_of_mouseovers();
+  }
+}
+
 bool InstantController::IsJavascriptEnabled() const {
   GURL instant_url(GetInstantURL());
   GURL origin(instant_url.GetOrigin());
   ContentSetting js_setting = profile()->GetHostContentSettingsMap()->
       GetContentSetting(origin, origin, CONTENT_SETTINGS_TYPE_JAVASCRIPT,
                         NO_RESOURCE_IDENTIFIER);
   // Javascript can be disabled either in content settings or via a WebKit
   // preference, so check both. Disabling it through the Settings page affects
   // content settings. I'm not sure how to disable the WebKit preference, but
   // it's theoretically possible some users have it off.
   bool js_content_enabled =
       js_setting == CONTENT_SETTING_DEFAULT ||
       js_setting == CONTENT_SETTING_ALLOW;
   bool js_webkit_enabled = profile()->GetPrefs()->GetBoolean(
       prefs::kWebKitJavascriptEnabled);
   return js_content_enabled && js_webkit_enabled;
 }
 
 bool InstantController::InStartup() const {
   // TODO(shishir): This is not completely reliable. Find a better way to detect
   // startup time.
   return !browser_->GetActiveWebContents();
 }
 
 InstantService* InstantController::GetInstantService() const {
   return InstantServiceFactory::GetForProfile(profile());
 }