Commit de9437e0 authored by Catherine Chung's avatar Catherine Chung Committed by Commit Bot

Notify the NewTabFeatureEngagementTracker of tab creation and omnibox navigations

This CL uses the NewTabFeatureEngagementTracker, which you can view
in this CL: http://crrev.com/7d54c1b52c11ef28e9a19f3c6a5a7a41a25fbf5d

The New Tab FeatureEngagementTracker will be notified if:
* The user presses the New Tab button
* The user presses Control+T
* The user uses the tabstrip context menu, window frame context menu,
  or the app menu to open a New tab.
* The user uses the omnibox to start a session in the same tab.
* The user focuses on the omnibox.

Bug: 734132
Change-Id: I5c9f3c394a2f2001b812a18e788fdc68220dcd20
Reviewed-on: https://chromium-review.googlesource.com/549074Reviewed-by: default avatarJustin Cohen <justincohen@chromium.org>
Reviewed-by: default avatarRobert Liao <robliao@chromium.org>
Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Reviewed-by: default avatarErik Chen <erikchen@chromium.org>
Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Commit-Queue: Catherine Chung <catherinechung@google.com>
Cr-Commit-Position: refs/heads/master@{#488771}
parent b15287cb
......@@ -82,6 +82,11 @@
#include "ui/base/ime/linux/text_edit_key_bindings_delegate_auralinux.h"
#endif
#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS) && !defined(OS_MACOSX)
#include "chrome/browser/feature_engagement_tracker/new_tab/new_tab_tracker.h"
#include "chrome/browser/feature_engagement_tracker/new_tab/new_tab_tracker_factory.h"
#endif
using content::NavigationEntry;
using content::NavigationController;
using content::WebContents;
......@@ -350,6 +355,13 @@ void BrowserCommandController::ExecuteCommandWithDisposition(
CloseWindow(browser_);
break;
case IDC_NEW_TAB:
#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS) && !defined(OS_MACOSX)
// This is not in NewTab() to avoid tracking programmatic creation of new
// tabs by extensions.
feature_engagement_tracker::NewTabTrackerFactory::GetInstance()
->GetForProfile(profile())
->OnNewTabOpened();
#endif
NewTab(browser_);
break;
case IDC_CLOSE_TAB:
......
......@@ -13,6 +13,7 @@
#include "base/metrics/histogram_macros.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "build/build_config.h"
#include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/autocomplete/autocomplete_classifier_factory.h"
#include "chrome/browser/autocomplete/chrome_autocomplete_provider_client.h"
......@@ -45,6 +46,7 @@
#include "components/prefs/pref_service.h"
#include "components/search/search.h"
#include "components/search_engines/template_url_service.h"
#include "components/toolbar/toolbar_model.h"
#include "content/public/browser/navigation_controller.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/web_contents.h"
......@@ -54,6 +56,11 @@
#include "ui/base/window_open_disposition.h"
#include "url/gurl.h"
#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS) && !defined(OS_MACOSX)
#include "chrome/browser/feature_engagement_tracker/new_tab/new_tab_tracker.h"
#include "chrome/browser/feature_engagement_tracker/new_tab/new_tab_tracker_factory.h"
#endif
using predictors::AutocompleteActionPredictor;
namespace {
......@@ -181,12 +188,12 @@ bool ChromeOmniboxClient::IsPasteAndGoEnabled() const {
return controller_->command_updater()->IsCommandEnabled(IDC_OPEN_CURRENT_URL);
}
bool ChromeOmniboxClient::IsNewTabPage(const std::string& url) const {
return url == chrome::kChromeUINewTabURL;
bool ChromeOmniboxClient::IsNewTabPage(const GURL& url) const {
return url.spec() == chrome::kChromeUINewTabURL;
}
bool ChromeOmniboxClient::IsHomePage(const std::string& url) const {
return url == profile_->GetPrefs()->GetString(prefs::kHomePage);
bool ChromeOmniboxClient::IsHomePage(const GURL& url) const {
return url.spec() == profile_->GetPrefs()->GetString(prefs::kHomePage);
}
const SessionID& ChromeOmniboxClient::GetSessionID() const {
......@@ -441,6 +448,20 @@ void ChromeOmniboxClient::OnRevert() {
}
void ChromeOmniboxClient::OnURLOpenedFromOmnibox(OmniboxLog* log) {
// The new tab tracker tracks when a user starts a session in the same
// tab as a previous one. If ShouldDisplayURL() is true, that's a good
// signal that the previous page was part of some other session.
// We could go further to try to analyze the difference between the previous
// and current URLs, but users edit URLs rarely enough that this is a
// reasonable approximation.
#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS) && !defined(OS_MACOSX)
if (controller_->GetToolbarModel()->ShouldDisplayURL()) {
feature_engagement_tracker::NewTabTrackerFactory::GetInstance()
->GetForProfile(profile_)
->OnOmniboxNavigation();
}
#endif
predictors::AutocompleteActionPredictorFactory::GetForProfile(profile_)
->OnOmniboxOpenedUrl(*log);
}
......
......@@ -13,6 +13,7 @@
#include "components/omnibox/browser/omnibox_client.h"
class ChromeOmniboxEditController;
class GURL;
class OmniboxEditController;
class Profile;
......@@ -36,8 +37,8 @@ class ChromeOmniboxClient : public OmniboxClient {
bool IsSearchResultsPage() const override;
bool IsLoading() const override;
bool IsPasteAndGoEnabled() const override;
bool IsNewTabPage(const std::string& url) const override;
bool IsHomePage(const std::string& url) const override;
bool IsNewTabPage(const GURL& url) const override;
bool IsHomePage(const GURL& url) const override;
const SessionID& GetSessionID() const override;
bookmarks::BookmarkModel* GetBookmarkModel() override;
TemplateURLService* GetTemplateURLService() override;
......
......@@ -13,6 +13,7 @@
#include "base/memory/ptr_util.h"
#include "base/metrics/histogram_macros.h"
#include "base/metrics/user_metrics.h"
#include "build/build_config.h"
#include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/browser_shutdown.h"
#include "chrome/browser/defaults.h"
......@@ -29,6 +30,12 @@
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_contents_observer.h"
#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS) && !defined(OS_MACOSX)
#include "chrome/browser/feature_engagement_tracker/new_tab/new_tab_tracker.h"
#include "chrome/browser/feature_engagement_tracker/new_tab/new_tab_tracker_factory.h"
#endif
using base::UserMetricsAction;
using content::WebContents;
......@@ -906,6 +913,12 @@ void TabStripModel::ExecuteContextMenuCommand(
UMA_HISTOGRAM_ENUMERATION("Tab.NewTab",
TabStripModel::NEW_TAB_CONTEXT_MENU,
TabStripModel::NEW_TAB_ENUM_COUNT);
#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS) && !defined(OS_MACOSX)
feature_engagement_tracker::NewTabTrackerFactory::GetInstance()
->GetForProfile(profile_)
->OnNewTabOpened();
#endif
delegate()->AddTabAt(GURL(), context_index + 1, true);
break;
......
......@@ -15,6 +15,7 @@
#include "build/build_config.h"
#include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/command_updater.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/omnibox/clipboard_utils.h"
#include "chrome/browser/ui/view_ids.h"
#include "chrome/browser/ui/views/location_bar/location_bar_view.h"
......@@ -59,6 +60,11 @@
#include "chrome/browser/browser_process.h"
#endif
#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS) && !defined(OS_MACOSX)
#include "chrome/browser/feature_engagement_tracker/new_tab/new_tab_tracker.h"
#include "chrome/browser/feature_engagement_tracker/new_tab/new_tab_tracker_factory.h"
#endif
namespace {
// OmniboxState ---------------------------------------------------------------
......@@ -773,6 +779,20 @@ void OmniboxViewViews::OnFocus() {
// Focus changes can affect the visibility of any keyword hint.
if (model()->is_keyword_hint())
location_bar_view_->Layout();
// The user must be starting a session in the same tab as a previous one
// in order to display the new tab in-product help promo.
// While focusing the omnibox is not always a precursor to starting a new
// session, we don't want to wait until the user is in the middle of editing
// or navigating, because we'd like to show them the promo at the time when
// it would be immediately useful.
#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS) && !defined(OS_MACOSX)
if (controller()->GetToolbarModel()->ShouldDisplayURL()) {
feature_engagement_tracker::NewTabTrackerFactory::GetInstance()
->GetForProfile(location_bar_view_->profile())
->OnOmniboxFocused();
}
#endif
}
void OmniboxViewViews::OnBlur() {
......
......@@ -71,6 +71,7 @@ class TestingOmniboxView : public OmniboxViewViews {
// OmniboxViewViews:
void EmphasizeURLComponents() override;
void OnFocus() override;
private:
// OmniboxViewViews:
......@@ -126,6 +127,12 @@ void TestingOmniboxView::EmphasizeURLComponents() {
UpdateTextStyle(text(), model()->client()->GetSchemeClassifier());
}
void TestingOmniboxView::OnFocus() {
views::Textfield::OnFocus();
model()->OnSetFocus(false);
GetRenderText()->SetElideBehavior(gfx::NO_ELIDE);
}
void TestingOmniboxView::UpdatePopup() {
++update_popup_call_count_;
update_popup_text_ = text();
......
......@@ -9,6 +9,7 @@
#include "base/metrics/user_metrics.h"
#include "base/task_scheduler/post_task.h"
#include "base/threading/sequenced_worker_pool.h"
#include "build/build_config.h"
#include "chrome/browser/autocomplete/autocomplete_classifier_factory.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/chrome_notification_types.h"
......@@ -49,6 +50,11 @@
#include "ui/views/widget/widget.h"
#include "url/origin.h"
#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS) && !defined(OS_MACOSX)
#include "chrome/browser/feature_engagement_tracker/new_tab/new_tab_tracker.h"
#include "chrome/browser/feature_engagement_tracker/new_tab/new_tab_tracker_factory.h"
#endif
using base::UserMetricsAction;
using content::WebContents;
......@@ -357,6 +363,11 @@ bool BrowserTabStripController::IsCompatibleWith(TabStrip* other) const {
}
void BrowserTabStripController::CreateNewTab() {
#if !defined(OS_ANDROID) && !defined(OS_CHROMEOS) && !defined(OS_MACOSX)
feature_engagement_tracker::NewTabTrackerFactory::GetInstance()
->GetForProfile(browser_view_->browser()->profile())
->OnNewTabOpened();
#endif
model_->delegate()->AddTabAt(GURL(), -1, true);
}
......
......@@ -47,11 +47,11 @@ bool OmniboxClient::IsPasteAndGoEnabled() const {
return false;
}
bool OmniboxClient::IsNewTabPage(const std::string& url) const {
bool OmniboxClient::IsNewTabPage(const GURL& url) const {
return false;
}
bool OmniboxClient::IsHomePage(const std::string& url) const {
bool OmniboxClient::IsHomePage(const GURL& url) const {
return false;
}
......
......@@ -73,10 +73,10 @@ class OmniboxClient {
virtual bool IsPasteAndGoEnabled() const;
// Returns whether |url| corresponds to the new tab page.
virtual bool IsNewTabPage(const std::string& url) const;
virtual bool IsNewTabPage(const GURL& url) const;
// Returns whether |url| corresponds to the user's home page.
virtual bool IsHomePage(const std::string& url) const;
virtual bool IsHomePage(const GURL& url) const;
// Returns the session ID of the current page.
virtual const SessionID& GetSessionID() const = 0;
......
......@@ -1361,12 +1361,11 @@ OmniboxEventProto::PageClassification OmniboxEditModel::ClassifyPage() const {
const GURL& gurl = client_->GetURL();
if (!gurl.is_valid())
return OmniboxEventProto::INVALID_SPEC;
const std::string& url = gurl.spec();
if (client_->IsNewTabPage(url))
if (client_->IsNewTabPage(gurl))
return OmniboxEventProto::NTP;
if (url == url::kAboutBlankURL)
if (gurl.spec() == url::kAboutBlankURL)
return OmniboxEventProto::BLANK;
if (client_->IsHomePage(url))
if (client_->IsHomePage(gurl))
return OmniboxEventProto::HOME_PAGE;
if (client_->IsSearchResultsPage())
return OmniboxEventProto::SEARCH_RESULT_PAGE_NO_SEARCH_TERM_REPLACEMENT;
......
......@@ -36,8 +36,8 @@ class ChromeOmniboxClientIOS : public OmniboxClient {
bool IsPasteAndGoEnabled() const override;
bool IsInstantNTP() const override;
bool IsSearchResultsPage() const override;
bool IsNewTabPage(const std::string& url) const override;
bool IsHomePage(const std::string& url) const override;
bool IsNewTabPage(const GURL& url) const override;
bool IsHomePage(const GURL& url) const override;
const SessionID& GetSessionID() const override;
bookmarks::BookmarkModel* GetBookmarkModel() override;
TemplateURLService* GetTemplateURLService() override;
......
......@@ -85,11 +85,11 @@ bool ChromeOmniboxClientIOS::IsSearchResultsPage() const {
->IsSearchResultsPageFromDefaultSearchProvider(GetURL());
}
bool ChromeOmniboxClientIOS::IsNewTabPage(const std::string& url) const {
return url == kChromeUINewTabURL;
bool ChromeOmniboxClientIOS::IsNewTabPage(const GURL& url) const {
return url.spec() == kChromeUINewTabURL;
}
bool ChromeOmniboxClientIOS::IsHomePage(const std::string& url) const {
bool ChromeOmniboxClientIOS::IsHomePage(const GURL& url) const {
// iOS does not have a notion of home page.
return false;
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment