Skip to content

Commit

Permalink
Adding search answer UMAs.
Browse files Browse the repository at this point in the history 
New user action are designed to be used with sequencer. Since sequencer has limited analysis depth, and to reduce noise, I marked some other actions as not_user_triggered. They will still be present in internal dashboards, but will be ignored by sequencer. These actions are either not directly caused by user and unrelated to the launcher works, or caused by the user, but are duplicated by other actions that I left untouched.

Bug=712331

Review-Url: https://codereview.chromium.org/2856063002
Cr-Commit-Position: refs/heads/master@{#469085}
  • Loading branch information
vadimt authored and Commit bot committed May 3, 2017
1 parent 840c7ec commit fd8809b
Show file tree
Hide file tree
Showing 5 changed files with 157 additions and 7 deletions.
79 changes: 77 additions & 2 deletions chrome/browser/ui/app_list/search_answer_web_contents_delegate.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#include "chrome/browser/ui/app_list/search_answer_web_contents_delegate.h"

#include "base/command_line.h"
#include "base/metrics/histogram_macros.h"
#include "base/metrics/user_metrics.h"
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser_navigator.h"
Expand All @@ -20,15 +22,61 @@
#include "ui/app_list/app_list_switches.h"
#include "ui/app_list/search_box_model.h"
#include "ui/views/controls/webview/webview.h"
#include "ui/views/widget/widget.h"

namespace app_list {

namespace {

enum class SearchAnswerRequestResult {
REQUEST_RESULT_ANOTHER_REQUEST_STARTED = 0,
REQUEST_RESULT_REQUEST_FAILED = 1,
REQUEST_RESULT_NO_ANSWER = 2,
REQUEST_RESULT_RECEIVED_ANSWER = 3,
REQUEST_RESULT_MAX = 4
};

void RecordRequestResult(SearchAnswerRequestResult request_result) {
UMA_HISTOGRAM_ENUMERATION("SearchAnswer.RequestResult", request_result,
SearchAnswerRequestResult::REQUEST_RESULT_MAX);
}

class SearchAnswerWebView : public views::WebView {
public:
SearchAnswerWebView(content::BrowserContext* browser_context)
: WebView(browser_context) {}

// views::WebView overrides:
void VisibilityChanged(View* starting_from, bool is_visible) override {
WebView::VisibilityChanged(starting_from, is_visible);

if (GetWidget()->IsVisible() && IsDrawn()) {
if (shown_time_.is_null())
shown_time_ = base::TimeTicks::Now();
} else {
if (!shown_time_.is_null()) {
UMA_HISTOGRAM_MEDIUM_TIMES("SearchAnswer.AnswerVisibleTime",
base::TimeTicks::Now() - shown_time_);
shown_time_ = base::TimeTicks();
}
}
}

private:
// Time when the answer became visible to the user.
base::TimeTicks shown_time_;

DISALLOW_COPY_AND_ASSIGN(SearchAnswerWebView);
};

} // namespace

SearchAnswerWebContentsDelegate::SearchAnswerWebContentsDelegate(
Profile* profile,
app_list::AppListModel* model)
: profile_(profile),
model_(model),
web_view_(base::MakeUnique<views::WebView>(profile)),
web_view_(base::MakeUnique<SearchAnswerWebView>(profile)),
web_contents_(
content::WebContents::Create(content::WebContents::CreateParams(
profile,
Expand Down Expand Up @@ -65,6 +113,7 @@ void SearchAnswerWebContentsDelegate::Update() {
received_answer_ = false;
model_->SetSearchAnswerAvailable(false);
current_request_url_ = GURL();
server_request_start_time_ = answer_loaded_time_ = base::TimeTicks();

if (model_->search_box()->is_voice_query()) {
// No need to send a server request and show a card because launcher
Expand Down Expand Up @@ -93,6 +142,7 @@ void SearchAnswerWebContentsDelegate::Update() {
load_params.transition_type = ui::PAGE_TRANSITION_AUTO_TOPLEVEL;
load_params.should_clear_history_list = true;
web_contents_->GetController().LoadURLWithParams(load_params);
server_request_start_time_ = base::TimeTicks::Now();

// We are going to call WebContents::GetPreferredSize().
web_contents_->GetRenderViewHost()->EnablePreferredSizeMode();
Expand All @@ -102,6 +152,10 @@ void SearchAnswerWebContentsDelegate::UpdatePreferredSize(
content::WebContents* web_contents,
const gfx::Size& pref_size) {
web_view_->SetPreferredSize(pref_size);
if (!answer_loaded_time_.is_null()) {
UMA_HISTOGRAM_TIMES("SearchAnswer.ResizeAfterLoadTime",
base::TimeTicks::Now() - answer_loaded_time_);
}
}

content::WebContents* SearchAnswerWebContentsDelegate::OpenURLFromTab(
Expand All @@ -127,6 +181,8 @@ content::WebContents* SearchAnswerWebContentsDelegate::OpenURLFromTab(

chrome::Navigate(&new_tab_params);

base::RecordAction(base::UserMetricsAction("SearchAnswer_OpenedUrl"));

return new_tab_params.target_contents;
}

Expand All @@ -138,29 +194,48 @@ bool SearchAnswerWebContentsDelegate::HandleContextMenu(

void SearchAnswerWebContentsDelegate::DidFinishNavigation(
content::NavigationHandle* navigation_handle) {
if (navigation_handle->GetURL() != current_request_url_)
if (navigation_handle->GetURL() != current_request_url_) {
RecordRequestResult(
SearchAnswerRequestResult::REQUEST_RESULT_ANOTHER_REQUEST_STARTED);
return;
}

if (!navigation_handle->HasCommitted() || navigation_handle->IsErrorPage() ||
!navigation_handle->IsInMainFrame()) {
RecordRequestResult(
SearchAnswerRequestResult::REQUEST_RESULT_REQUEST_FAILED);
return;
}

const net::HttpResponseHeaders* headers =
navigation_handle->GetResponseHeaders();
if (!headers || headers->response_code() != net::HTTP_OK ||
!headers->HasHeaderValue("has_answer", "true")) {
RecordRequestResult(SearchAnswerRequestResult::REQUEST_RESULT_NO_ANSWER);
return;
}

received_answer_ = true;
UMA_HISTOGRAM_TIMES("SearchAnswer.NavigationTime",
base::TimeTicks::Now() - server_request_start_time_);
RecordRequestResult(
SearchAnswerRequestResult::REQUEST_RESULT_RECEIVED_ANSWER);
}

void SearchAnswerWebContentsDelegate::DidStopLoading() {
if (!received_answer_)
return;

model_->SetSearchAnswerAvailable(true);
answer_loaded_time_ = base::TimeTicks::Now();
UMA_HISTOGRAM_TIMES("SearchAnswer.LoadingTime",
answer_loaded_time_ - server_request_start_time_);
base::RecordAction(base::UserMetricsAction("SearchAnswer_StoppedLoading"));
}

void SearchAnswerWebContentsDelegate::DidGetUserInteraction(
const blink::WebInputEvent::Type type) {
base::RecordAction(base::UserMetricsAction("SearchAnswer_UserInteraction"));
}

void SearchAnswerWebContentsDelegate::OnSearchEngineIsGoogleChanged(
Expand Down
8 changes: 8 additions & 0 deletions chrome/browser/ui/app_list/search_answer_web_contents_delegate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <memory>
#include <string>

#include "base/time/time.h"
#include "content/public/browser/web_contents_delegate.h"
#include "content/public/browser/web_contents_observer.h"
#include "ui/app_list/app_list_model_observer.h"
Expand Down Expand Up @@ -59,6 +60,7 @@ class SearchAnswerWebContentsDelegate : public content::WebContentsDelegate,
void DidFinishNavigation(
content::NavigationHandle* navigation_handle) override;
void DidStopLoading() override;
void DidGetUserInteraction(const blink::WebInputEvent::Type type) override;

// AppListModelObserver overrides:
void OnSearchEngineIsGoogleChanged(bool is_google) override;
Expand Down Expand Up @@ -86,6 +88,12 @@ class SearchAnswerWebContentsDelegate : public content::WebContentsDelegate,
// URL of the current answer server request.
GURL current_request_url_;

// Time when the current server request started.
base::TimeTicks server_request_start_time_;

// Time when the current server response loaded.
base::TimeTicks answer_loaded_time_;

DISALLOW_COPY_AND_ASSIGN(SearchAnswerWebContentsDelegate);
};

Expand Down
30 changes: 25 additions & 5 deletions tools/metrics/actions/actions.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1085,12 +1085,12 @@ should be able to be added at any place in this file.
<description>Please enter the description of this user action.</description>
</action>

<action name="ActiveBrowserChanged">
<action name="ActiveBrowserChanged" not_user_triggered="true">
<owner>Please list the metric's owners. Add more owner tags as needed.</owner>
<description>Please enter the description of this user action.</description>
</action>

<action name="ActiveTabChanged">
<action name="ActiveTabChanged" not_user_triggered="true">
<owner>Please list the metric's owners. Add more owner tags as needed.</owner>
<description>Please enter the description of this user action.</description>
</action>
Expand Down Expand Up @@ -8783,15 +8783,15 @@ should be able to be added at any place in this file.
<description>Please enter the description of this user action.</description>
</action>

<action name="Launcher_ButtonPressed_Mouse">
<action name="Launcher_ButtonPressed_Mouse" not_user_triggered="true">
<owner>bruthig@google.com</owner>
<owner>tdanderson@google.com</owner>
<description>
Recorded when the user activates a shelf button using the mouse.
</description>
</action>

<action name="Launcher_ButtonPressed_Touch">
<action name="Launcher_ButtonPressed_Touch" not_user_triggered="true">
<owner>bruthig@google.com</owner>
<owner>tdanderson@google.com</owner>
<description>
Expand All @@ -8809,6 +8809,11 @@ should be able to be added at any place in this file.
<description>Please enter the description of this user action.</description>
</action>

<action name="Launcher_Dismiss">
<owner>vadimt@chromium.org</owner>
<description>Launcher got dismissed.</description>
</action>

<action name="Launcher_LaunchTask">
<owner>bruthig@google.com</owner>
<owner>tdanderson@google.com</owner>
Expand Down Expand Up @@ -14356,6 +14361,21 @@ should be able to be added at any place in this file.
</description>
</action>

<action name="SearchAnswer_OpenedUrl">
<owner>vadimt@chromium.org</owner>
<description>User opens URL from an answer card.</description>
</action>

<action name="SearchAnswer_StoppedLoading">
<owner>vadimt@chromium.org</owner>
<description>Answer card successfully loads.</description>
</action>

<action name="SearchAnswer_UserInteraction">
<owner>vadimt@chromium.org</owner>
<description>User interacts with an answer card.</description>
</action>

<action name="SearchEngine_ManualChange">
<owner>ianwen@chromium.org</owner>
<description>
Expand Down Expand Up @@ -16243,7 +16263,7 @@ should be able to be added at any place in this file.
<description>Logged on an ET_SCROLL_FLING_START gesture event.</description>
</action>

<action name="Touchscreen_Down">
<action name="Touchscreen_Down" not_user_triggered="true">
<owner>Please list the metric's owners. Add more owner tags as needed.</owner>
<description>Please enter the description of this user action.</description>
</action>
Expand Down
45 changes: 45 additions & 0 deletions tools/metrics/histograms/histograms.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64719,6 +64719,44 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
</summary>
</histogram>

<histogram name="SearchAnswer.AnswerVisibleTime" units="ms">
<owner>vadimt@chromium.org</owner>
<summary>Time between showing and hiding the answer card.</summary>
</histogram>

<histogram name="SearchAnswer.LoadingTime" units="ms">
<owner>vadimt@chromium.org</owner>
<summary>
Time between sending an answer server request and the end of loading of the
answer card. Failed requests and requests not returning an answer are not
counted.
</summary>
</histogram>

<histogram name="SearchAnswer.NavigationTime" units="ms">
<owner>vadimt@chromium.org</owner>
<summary>
Time between sending an answer server request and successful finish of
navigation. Failed requests and requests not returning an answer are not
counted.
</summary>
</histogram>

<histogram name="SearchAnswer.RequestResult" enum="SearchAnswerRequestResult">
<owner>vadimt@chromium.org</owner>
<summary>Result of a answer server request.</summary>
</histogram>

<histogram name="SearchAnswer.ResizeAfterLoadTime" units="ms">
<owner>vadimt@chromium.org</owner>
<summary>
Time between successfully loading an answer card request and updating its
preferred size. There might be no or more than one size update per card
load. Size updates that happen before the card finishes loading are ignored
since they are not visible.
</summary>
</histogram>

<histogram
name="Security.HTTPBad.NavigationStartedAfterUserWarnedAboutSensitiveInput"
units="ms">
Expand Down Expand Up @@ -113645,6 +113683,13 @@ from previous Chrome versions.
<int value="9" label="Other Instant"/>
</enum>

<enum name="SearchAnswerRequestResult" type="int">
<int value="0" label="Another Request Started"/>
<int value="1" label="Failed"/>
<int value="2" label="No Answer"/>
<int value="3" label="Received Answer"/>
</enum>

<enum name="SearchEngine" type="int">
<obsolete>
Deprecated 8/2013. No longer generated.
Expand Down
2 changes: 2 additions & 0 deletions ui/app_list/presenter/app_list_presenter_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "ui/app_list/presenter/app_list_presenter_impl.h"

#include "base/metrics/user_metrics.h"
#include "ui/app_list/app_list_constants.h"
#include "ui/app_list/app_list_switches.h"
#include "ui/app_list/pagination_model.h"
Expand Down Expand Up @@ -98,6 +99,7 @@ void AppListPresenterImpl::Dismiss() {

presenter_delegate_->OnDismissed();
ScheduleAnimation();
base::RecordAction(base::UserMetricsAction("Launcher_Dismiss"));
}

void AppListPresenterImpl::ToggleAppList(int64_t display_id) {
Expand Down

0 comments on commit fd8809b

Please sign in to comment.