Commit 52dad56b authored by Michael McGreevy's avatar Michael McGreevy Committed by Commit Bot

Log the source when installing a PWA on desktop.

Bug: 785661
Change-Id: Ibeef9470ce0224869e4d398115d211a123b82417
Reviewed-on: https://chromium-review.googlesource.com/804907
Commit-Queue: Michael McGreevy <mcgreevy@chromium.org>
Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Reviewed-by: default avatarBen Wells <benwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525265}
parent 25bbd5e7
...@@ -36,6 +36,8 @@ void TrackInstallSource(WebAppInstallSource event) { ...@@ -36,6 +36,8 @@ void TrackInstallSource(WebAppInstallSource event) {
break; break;
case WebAppInstallSource::MENU: case WebAppInstallSource::MENU:
source = InstallSource::INSTALL_SOURCE_MENU; source = InstallSource::INSTALL_SOURCE_MENU;
case WebAppInstallSource::MANAGEMENT_API:
// MANAGEMENT_API is not reported. Fallthrough to NOTREACHED().
case WebAppInstallSource::COUNT: case WebAppInstallSource::COUNT:
NOTREACHED(); NOTREACHED();
return; return;
......
...@@ -83,16 +83,14 @@ bool AppBannerManagerDesktop::IsWebAppConsideredInstalled( ...@@ -83,16 +83,14 @@ bool AppBannerManagerDesktop::IsWebAppConsideredInstalled(
} }
void AppBannerManagerDesktop::ShowBannerUi(WebAppInstallSource install_source) { void AppBannerManagerDesktop::ShowBannerUi(WebAppInstallSource install_source) {
// TODO(mcgreevy): log install_source to Webapp.Install.InstallSource
// histogram.
content::WebContents* contents = web_contents(); content::WebContents* contents = web_contents();
DCHECK(contents && !manifest_.IsEmpty()); DCHECK(contents && !manifest_.IsEmpty());
Profile* profile = Profile::FromBrowserContext(contents->GetBrowserContext()); Profile* profile = Profile::FromBrowserContext(contents->GetBrowserContext());
WebApplicationInfo web_app_info; WebApplicationInfo web_app_info;
bookmark_app_helper_.reset( bookmark_app_helper_.reset(new extensions::BookmarkAppHelper(
new extensions::BookmarkAppHelper(profile, web_app_info, contents)); profile, web_app_info, contents, install_source));
if (IsExperimentalAppBannersEnabled()) { if (IsExperimentalAppBannersEnabled()) {
RecordDidShowBanner("AppBanner.WebApp.Shown"); RecordDidShowBanner("AppBanner.WebApp.Shown");
......
...@@ -149,7 +149,8 @@ class ChromeAppForLinkDelegate : public extensions::AppForLinkDelegate { ...@@ -149,7 +149,8 @@ class ChromeAppForLinkDelegate : public extensions::AppForLinkDelegate {
} }
bookmark_app_helper_.reset(new extensions::BookmarkAppHelper( bookmark_app_helper_.reset(new extensions::BookmarkAppHelper(
Profile::FromBrowserContext(context), web_app, NULL)); Profile::FromBrowserContext(context), web_app, nullptr,
WebAppInstallSource::MANAGEMENT_API));
bookmark_app_helper_->Create( bookmark_app_helper_->Create(
base::Bind(&extensions::ManagementGenerateAppForLinkFunction:: base::Bind(&extensions::ManagementGenerateAppForLinkFunction::
FinishCreateBookmarkApp, FinishCreateBookmarkApp,
......
...@@ -531,12 +531,14 @@ BookmarkAppHelper::BitmapAndSource::~BitmapAndSource() { ...@@ -531,12 +531,14 @@ BookmarkAppHelper::BitmapAndSource::~BitmapAndSource() {
BookmarkAppHelper::BookmarkAppHelper(Profile* profile, BookmarkAppHelper::BookmarkAppHelper(Profile* profile,
WebApplicationInfo web_app_info, WebApplicationInfo web_app_info,
content::WebContents* contents) content::WebContents* contents,
WebAppInstallSource install_source)
: profile_(profile), : profile_(profile),
contents_(contents), contents_(contents),
web_app_info_(web_app_info), web_app_info_(web_app_info),
crx_installer_(extensions::CrxInstaller::CreateSilent( crx_installer_(extensions::CrxInstaller::CreateSilent(
ExtensionSystem::Get(profile)->extension_service())), ExtensionSystem::Get(profile)->extension_service())),
install_source_(install_source),
weak_factory_(this) { weak_factory_(this) {
if (contents) if (contents)
installable_manager_ = InstallableManager::FromWebContents(contents); installable_manager_ = InstallableManager::FromWebContents(contents);
...@@ -729,6 +731,11 @@ void BookmarkAppHelper::OnBubbleCompleted( ...@@ -729,6 +731,11 @@ void BookmarkAppHelper::OnBubbleCompleted(
if (user_accepted) { if (user_accepted) {
web_app_info_ = web_app_info; web_app_info_ = web_app_info;
crx_installer_->InstallWebApp(web_app_info_); crx_installer_->InstallWebApp(web_app_info_);
if (InstallableMetrics::IsReportableInstallSource(install_source_) &&
installable_ == INSTALLABLE_YES) {
InstallableMetrics::TrackInstallSource(install_source_);
}
} else { } else {
callback_.Run(nullptr, web_app_info_); callback_.Run(nullptr, web_app_info_);
} }
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "base/callback.h" #include "base/callback.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "chrome/browser/installable/installable_metrics.h"
#include "chrome/common/web_application_info.h" #include "chrome/common/web_application_info.h"
#include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h" #include "content/public/browser/notification_registrar.h"
...@@ -55,9 +56,11 @@ class BookmarkAppHelper : public content::NotificationObserver { ...@@ -55,9 +56,11 @@ class BookmarkAppHelper : public content::NotificationObserver {
// All existing icons from WebApplicationInfo will also be used. The user // All existing icons from WebApplicationInfo will also be used. The user
// will then be prompted to edit the creation information via a bubble and // will then be prompted to edit the creation information via a bubble and
// will have a chance to cancel the operation. // will have a chance to cancel the operation.
// |install_source| indicates how the installation was triggered.
BookmarkAppHelper(Profile* profile, BookmarkAppHelper(Profile* profile,
WebApplicationInfo web_app_info, WebApplicationInfo web_app_info,
content::WebContents* contents); content::WebContents* contents,
WebAppInstallSource install_source);
~BookmarkAppHelper() override; ~BookmarkAppHelper() override;
// Update the given WebApplicationInfo with information from the manifest. // Update the given WebApplicationInfo with information from the manifest.
...@@ -165,6 +168,9 @@ class BookmarkAppHelper : public content::NotificationObserver { ...@@ -165,6 +168,9 @@ class BookmarkAppHelper : public content::NotificationObserver {
Installable installable_ = INSTALLABLE_UNKNOWN; Installable installable_ = INSTALLABLE_UNKNOWN;
// The mechanism via which the app creation was triggered.
WebAppInstallSource install_source_;
// With fast tab unloading enabled, shutting down can cause BookmarkAppHelper // With fast tab unloading enabled, shutting down can cause BookmarkAppHelper
// to be destroyed before the bookmark creation bubble. Use weak pointers to // to be destroyed before the bookmark creation bubble. Use weak pointers to
// prevent a heap-use-after free in this instance (https://crbug.com/534994). // prevent a heap-use-after free in this instance (https://crbug.com/534994).
......
...@@ -31,8 +31,9 @@ class TestBookmarkAppHelper : public BookmarkAppHelper { ...@@ -31,8 +31,9 @@ class TestBookmarkAppHelper : public BookmarkAppHelper {
TestBookmarkAppHelper(Profile* profile, TestBookmarkAppHelper(Profile* profile,
WebApplicationInfo web_app_info, WebApplicationInfo web_app_info,
content::WebContents* contents, content::WebContents* contents,
base::Closure on_icons_downloaded_closure) base::Closure on_icons_downloaded_closure,
: BookmarkAppHelper(profile, web_app_info, contents), WebAppInstallSource install_source)
: BookmarkAppHelper(profile, web_app_info, contents, install_source),
on_icons_downloaded_closure_(on_icons_downloaded_closure) {} on_icons_downloaded_closure_(on_icons_downloaded_closure) {}
// TestBookmarkAppHelper: // TestBookmarkAppHelper:
...@@ -73,7 +74,8 @@ class BookmarkAppHelperTest : public DialogBrowserTest { ...@@ -73,7 +74,8 @@ class BookmarkAppHelperTest : public DialogBrowserTest {
info.title = base::UTF8ToUTF16(info.app_url.spec()); info.title = base::UTF8ToUTF16(info.app_url.spec());
bookmark_app_helper_ = base::MakeUnique<TestBookmarkAppHelper>( bookmark_app_helper_ = base::MakeUnique<TestBookmarkAppHelper>(
browser()->profile(), info, web_contents(), quit_closure_); browser()->profile(), info, web_contents(), quit_closure_,
WebAppInstallSource::MENU);
bookmark_app_helper_->Create( bookmark_app_helper_->Create(
base::Bind(&BookmarkAppHelperTest::FinishCreateBookmarkApp, base::Bind(&BookmarkAppHelperTest::FinishCreateBookmarkApp,
base::Unretained(this))); base::Unretained(this)));
......
...@@ -283,7 +283,10 @@ class TestBookmarkAppHelper : public BookmarkAppHelper { ...@@ -283,7 +283,10 @@ class TestBookmarkAppHelper : public BookmarkAppHelper {
TestBookmarkAppHelper(ExtensionService* service, TestBookmarkAppHelper(ExtensionService* service,
WebApplicationInfo web_app_info, WebApplicationInfo web_app_info,
content::WebContents* contents) content::WebContents* contents)
: BookmarkAppHelper(service->profile(), web_app_info, contents), : BookmarkAppHelper(service->profile(),
web_app_info,
contents,
WebAppInstallSource::MENU),
bitmap_(CreateSquareBitmapWithColor(32, SK_ColorRED)) {} bitmap_(CreateSquareBitmapWithColor(32, SK_ColorRED)) {}
~TestBookmarkAppHelper() override {} ~TestBookmarkAppHelper() override {}
......
...@@ -24,6 +24,7 @@ ...@@ -24,6 +24,7 @@
#include "chrome/browser/extensions/install_tracker_factory.h" #include "chrome/browser/extensions/install_tracker_factory.h"
#include "chrome/browser/extensions/webstore_inline_installer.h" #include "chrome/browser/extensions/webstore_inline_installer.h"
#include "chrome/browser/extensions/webstore_inline_installer_factory.h" #include "chrome/browser/extensions/webstore_inline_installer_factory.h"
#include "chrome/browser/installable/installable_metrics.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/sessions/session_tab_helper.h" #include "chrome/browser/sessions/session_tab_helper.h"
#include "chrome/browser/shell_integration.h" #include "chrome/browser/shell_integration.h"
...@@ -365,8 +366,8 @@ void TabHelper::OnDidGetWebApplicationInfo( ...@@ -365,8 +366,8 @@ void TabHelper::OnDidGetWebApplicationInfo(
if (web_app_info_.title.empty()) if (web_app_info_.title.empty())
web_app_info_.title = base::UTF8ToUTF16(web_app_info_.app_url.spec()); web_app_info_.title = base::UTF8ToUTF16(web_app_info_.app_url.spec());
bookmark_app_helper_.reset( bookmark_app_helper_.reset(new BookmarkAppHelper(
new BookmarkAppHelper(profile_, web_app_info_, web_contents())); profile_, web_app_info_, web_contents(), WebAppInstallSource::MENU));
bookmark_app_helper_->Create(base::Bind( bookmark_app_helper_->Create(base::Bind(
&TabHelper::FinishCreateBookmarkApp, weak_ptr_factory_.GetWeakPtr())); &TabHelper::FinishCreateBookmarkApp, weak_ptr_factory_.GetWeakPtr()));
break; break;
......
...@@ -9,10 +9,18 @@ ...@@ -9,10 +9,18 @@
// static // static
void InstallableMetrics::TrackInstallSource(WebAppInstallSource source) { void InstallableMetrics::TrackInstallSource(WebAppInstallSource source) {
DCHECK(IsReportableInstallSource(source));
UMA_HISTOGRAM_ENUMERATION("Webapp.Install.InstallSource", source, UMA_HISTOGRAM_ENUMERATION("Webapp.Install.InstallSource", source,
WebAppInstallSource::COUNT); WebAppInstallSource::COUNT);
} }
// static
bool InstallableMetrics::IsReportableInstallSource(WebAppInstallSource source) {
return source == WebAppInstallSource::AUTOMATIC_PROMPT ||
source == WebAppInstallSource::MENU ||
source == WebAppInstallSource::API;
}
namespace { namespace {
void WriteMenuOpenHistogram(InstallabilityCheckStatus status, int count) { void WriteMenuOpenHistogram(InstallabilityCheckStatus status, int count) {
......
...@@ -32,11 +32,14 @@ enum class AddToHomescreenTimeoutStatus { ...@@ -32,11 +32,14 @@ enum class AddToHomescreenTimeoutStatus {
}; };
// The ways that an app install can be triggered. // The ways that an app install can be triggered.
// NOTE: each enum entry which is reportable must be added to
// InstallableMetrics::IsReportableInstallSource().
// This enum backs a UMA histogram and must be treated as append-only. // This enum backs a UMA histogram and must be treated as append-only.
enum class WebAppInstallSource { enum class WebAppInstallSource {
AUTOMATIC_PROMPT = 0, // Automatic prompt e.g. install banner. AUTOMATIC_PROMPT = 0, // Automatic prompt e.g. install banner.
MENU = 1, // Chrome menu MENU = 1, // Chrome menu
API = 2, // BeforeInstallPrompt.prompt(). API = 2, // BeforeInstallPrompt.prompt().
MANAGEMENT_API = 3, // Extensions management API (not reported).
COUNT, COUNT,
}; };
...@@ -59,8 +62,14 @@ class InstallableMetrics { ...@@ -59,8 +62,14 @@ class InstallableMetrics {
InstallableMetrics(); InstallableMetrics();
~InstallableMetrics(); ~InstallableMetrics();
// Records |source| in the Webapp.Install.InstallSource histogram.
// IsReportableInstallSource(|source|) must be true.
static void TrackInstallSource(WebAppInstallSource source); static void TrackInstallSource(WebAppInstallSource source);
// Returns whether |source| is a value that may be passed to
// TrackInstallSource.
static bool IsReportableInstallSource(WebAppInstallSource source);
// This records the state of the installability check when the Android menu is // This records the state of the installability check when the Android menu is
// opened. // opened.
void RecordMenuOpen(); void RecordMenuOpen();
......
...@@ -43928,6 +43928,7 @@ Called by update_traffic_annotation_histograms.py.--> ...@@ -43928,6 +43928,7 @@ Called by update_traffic_annotation_histograms.py.-->
<int value="0" label="Installed via the automatically-triggered prompt"/> <int value="0" label="Installed via the automatically-triggered prompt"/>
<int value="1" label="Installed via the menu"/> <int value="1" label="Installed via the menu"/>
<int value="2" label="Installed via a developer-triggered API call"/> <int value="2" label="Installed via a developer-triggered API call"/>
<int value="3" label="Installed via the management API (not reported)"/>
</enum> </enum>
<enum name="WebAudioAutoplayStatus"> <enum name="WebAudioAutoplayStatus">
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