Adds the NewTabPage.SuggestedSitesAction stat.

Adds the NewTabPage.SuggestedSitesAction stat.

BUG=118427
TEST=With the "NTP Suggestions page" flag enabled, go to the NTP and switch between panes (e.g. Apps and Suggested), then click on a link in the Suggestd pane. Go back to the NTP and type a URL in the Omnibox. Open a new tab and close it. Then, go to about:histograms and verify that there are data points at least for values 1, 11, 12 and 13 in the NewTabPage.SuggestedSitesAction stat. Other values are OK, as long as these are present.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=132249

[Evan Stade, 2012-04-03 22:52:33]

http://codereview.chromium.org/9958116/diff/5/chrome/browser/resources/ntp4/s...
File chrome/browser/resources/ntp4/suggestions_page.js (right):

http://codereview.chromium.org/9958116/diff/5/chrome/browser/resources/ntp4/s...
chrome/browser/resources/ntp4/suggestions_page.js:137:
chrome.send('suggestedSitesAction', [1]);
you need an enum, don't just toss magic values around.

why is any other kind of navigation away from suggested (omnibox, use of
bookmark) at all interesting?

[rudygalfi, 2012-04-04 16:26:51]

The goal of suggestions is to be highly predictive. If the user is choosing not
to use suggestions, then it's interesting to know the other means by which a
user browses.

On 2012/04/03 22:52:33, Evan Stade wrote:
>
http://codereview.chromium.org/9958116/diff/5/chrome/browser/resources/ntp4/s...
> File chrome/browser/resources/ntp4/suggestions_page.js (right):
> 
>
http://codereview.chromium.org/9958116/diff/5/chrome/browser/resources/ntp4/s...
> chrome/browser/resources/ntp4/suggestions_page.js:137:
> chrome.send('suggestedSitesAction', [1]);
> you need an enum, don't just toss magic values around.
> 
> why is any other kind of navigation away from suggested (omnibox, use of
> bookmark) at all interesting?

[macourteau, 2012-04-04 19:41:09]

http://codereview.chromium.org/9958116/diff/5/chrome/browser/resources/ntp4/s...
File chrome/browser/resources/ntp4/suggestions_page.js (right):

http://codereview.chromium.org/9958116/diff/5/chrome/browser/resources/ntp4/s...
chrome/browser/resources/ntp4/suggestions_page.js:137:
chrome.send('suggestedSitesAction', [1]);
On 2012/04/03 22:52:33, Evan Stade wrote:
> you need an enum, don't just toss magic values around.
> 
> why is any other kind of navigation away from suggested (omnibox, use of
> bookmark) at all interesting?

Done.

[Evan Stade, 2012-04-04 20:33:35]

https://chromiumcodereview.appspot.com/9958116/diff/9001/chrome/browser/ui/we...
File chrome/browser/ui/webui/ntp/suggestions_page_handler.cc (right):

https://chromiumcodereview.appspot.com/9958116/diff/9001/chrome/browser/ui/we...
chrome/browser/ui/webui/ntp/suggestions_page_handler.cc:46:
SUGGESTED_SITES_ACTION_CLICKED_SUGGESTED_TILE,
comments for these.

I'm also still a bit confused about why most of them exist. What is the
usefulness of logging SUGGESTED_SITES_ACTION_OMNIBOX_NAVIGATION?

[macourteau, 2012-04-04 21:00:36]

re: logging Omnibox navigation: that's probably the most frequent exit point
from suggestions, and we think that it would be interesting to track if
suggestions lessen the amount of Omnibox navigations over time. We could
probably get at that info with a boolean, but we think that Omnibox is important
enough to call out on its own since it's the most likely source of movement.

https://chromiumcodereview.appspot.com/9958116/diff/9001/chrome/browser/ui/we...
File chrome/browser/ui/webui/ntp/suggestions_page_handler.cc (right):

https://chromiumcodereview.appspot.com/9958116/diff/9001/chrome/browser/ui/we...
chrome/browser/ui/webui/ntp/suggestions_page_handler.cc:46:
SUGGESTED_SITES_ACTION_CLICKED_SUGGESTED_TILE,
On 2012/04/04 20:33:35, Evan Stade wrote:
> comments for these.
> 
> I'm also still a bit confused about why most of them exist. What is the
> usefulness of logging SUGGESTED_SITES_ACTION_OMNIBOX_NAVIGATION?

Done.

[Evan Stade, 2012-04-09 19:46:32]

if this is what you want, why don't you create a histogram of all PageTransition
types for users who are looking at a suggestions page? as it stands, I don't
think you're actually measuring omnibox usage as a suggestion from the omnibox,
or instant, probably wouldn't count as TYPED; also, other things besides omnibox
usage do count as TYPED

On 2012/04/04 21:00:36, macourteau wrote:
> re: logging Omnibox navigation: that's probably the most frequent exit point
> from suggestions, and we think that it would be interesting to track if
> suggestions lessen the amount of Omnibox navigations over time. We could
> probably get at that info with a boolean, but we think that Omnibox is
important
> enough to call out on its own since it's the most likely source of movement.
> 
>
https://chromiumcodereview.appspot.com/9958116/diff/9001/chrome/browser/ui/we...
> File chrome/browser/ui/webui/ntp/suggestions_page_handler.cc (right):
> 
>
https://chromiumcodereview.appspot.com/9958116/diff/9001/chrome/browser/ui/we...
> chrome/browser/ui/webui/ntp/suggestions_page_handler.cc:46:
> SUGGESTED_SITES_ACTION_CLICKED_SUGGESTED_TILE,
> On 2012/04/04 20:33:35, Evan Stade wrote:
> > comments for these.
> > 
> > I'm also still a bit confused about why most of them exist. What is the
> > usefulness of logging SUGGESTED_SITES_ACTION_OMNIBOX_NAVIGATION?
> 
> Done.

[macourteau, 2012-04-11 20:06:19]

On 2012/04/09 19:46:32, Evan Stade wrote:
> if this is what you want, why don't you create a histogram of all
PageTransition
> types for users who are looking at a suggestions page? as it stands, I don't
> think you're actually measuring omnibox usage as a suggestion from the
omnibox,
> or instant, probably wouldn't count as TYPED; also, other things besides
omnibox
> usage do count as TYPED
> 

Done. I've also sent the relevant histograms.xml change for review.

[Evan Stade, 2012-04-12 03:30:53]

http://codereview.chromium.org/9958116/diff/25004/chrome/browser/ui/webui/ntp...
File chrome/browser/ui/webui/ntp/suggestions_page_handler.cc (right):

http://codereview.chromium.org/9958116/diff/25004/chrome/browser/ui/webui/ntp...
chrome/browser/ui/webui/ntp/suggestions_page_handler.cc:79:
content::PageTransitionStripQualifier(entry->GetTransitionType());
2 more spaces indent

http://codereview.chromium.org/9958116/diff/25004/chrome/browser/ui/webui/ntp...
chrome/browser/ui/webui/ntp/suggestions_page_handler.cc:82:
UMA_HISTOGRAM_ENUMERATION("NewTabPage.SuggestedSitesAction", action_id, 4);
4?

http://codereview.chromium.org/9958116/diff/25004/chrome/browser/ui/webui/ntp...
chrome/browser/ui/webui/ntp/suggestions_page_handler.cc:195:
static_cast<int>(action_id), 4);
4?

http://codereview.chromium.org/9958116/diff/25004/chrome/browser/ui/webui/ntp...
File chrome/browser/ui/webui/ntp/suggestions_page_handler.h (right):

http://codereview.chromium.org/9958116/diff/25004/chrome/browser/ui/webui/ntp...
chrome/browser/ui/webui/ntp/suggestions_page_handler.h:107: bool
valid_user_action_;
i don't like this variable name much, how about "user_action_logged_"

[macourteau, 2012-04-12 13:59:43]

http://codereview.chromium.org/9958116/diff/25004/chrome/browser/ui/webui/ntp...
File chrome/browser/ui/webui/ntp/suggestions_page_handler.cc (right):

http://codereview.chromium.org/9958116/diff/25004/chrome/browser/ui/webui/ntp...
chrome/browser/ui/webui/ntp/suggestions_page_handler.cc:79:
content::PageTransitionStripQualifier(entry->GetTransitionType());
On 2012/04/12 03:30:53, Evan Stade wrote:
> 2 more spaces indent

Done.

http://codereview.chromium.org/9958116/diff/25004/chrome/browser/ui/webui/ntp...
chrome/browser/ui/webui/ntp/suggestions_page_handler.cc:82:
UMA_HISTOGRAM_ENUMERATION("NewTabPage.SuggestedSitesAction", action_id, 4);
On 2012/04/12 03:30:53, Evan Stade wrote:
> 4?

Done.

http://codereview.chromium.org/9958116/diff/25004/chrome/browser/ui/webui/ntp...
chrome/browser/ui/webui/ntp/suggestions_page_handler.cc:195:
static_cast<int>(action_id), 4);
On 2012/04/12 03:30:53, Evan Stade wrote:
> 4?

Done.

http://codereview.chromium.org/9958116/diff/25004/chrome/browser/ui/webui/ntp...
File chrome/browser/ui/webui/ntp/suggestions_page_handler.h (right):

http://codereview.chromium.org/9958116/diff/25004/chrome/browser/ui/webui/ntp...
chrome/browser/ui/webui/ntp/suggestions_page_handler.h:107: bool
valid_user_action_;
On 2012/04/12 03:30:53, Evan Stade wrote:
> i don't like this variable name much, how about "user_action_logged_"

Done.

[Evan Stade, 2012-04-13 16:59:43]

this code LGTM, but I will point out that to analyze this UMA you will need to
have some kind of baseline to compare against. Probably that means logging the
same UMA for Most Visited.

[commit-bot: I haz the power, 2012-04-13 17:53:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/macourteau@chromium.org/9958116/31001

[commit-bot: I haz the power, 2012-04-13 19:53:28]

Change committed as 132249