Commit 4e8ea8d3 authored by phillis's avatar phillis Committed by Commit Bot

DPWA: Add create shortcut as an install source.

Make a separate install source for installing from "Create Shortcut"
instead of sharing the same install source with install from menu.
Also record the Webapp.Install.InstallEvent for this source.

Bug: 1131210
Change-Id: Id0531e6d283603da01c02ba3c94cfa44138daacc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2439425
Commit-Queue: Phillis Tang <phillis@chromium.org>
Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Reviewed-by: default avatarAlexey Baskakov <loyso@chromium.org>
Reviewed-by: default avatarDaniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812594}
parent 1cec715d
......@@ -36,7 +36,8 @@ bool InstallableMetrics::IsReportableInstallSource(WebappInstallSource source) {
source == WebappInstallSource::EXTERNAL_DEFAULT ||
source == WebappInstallSource::EXTERNAL_POLICY ||
source == WebappInstallSource::SYSTEM_DEFAULT ||
source == WebappInstallSource::OMNIBOX_INSTALL_ICON;
source == WebappInstallSource::OMNIBOX_INSTALL_ICON ||
source == WebappInstallSource::MENU_CREATE_SHORTCUT;
}
// static
......@@ -63,6 +64,11 @@ WebappInstallSource InstallableMetrics::GetInstallSource(
case InstallTrigger::MENU:
return is_custom_tab ? WebappInstallSource::MENU_CUSTOM_TAB
: WebappInstallSource::MENU_BROWSER_TAB;
// Create shortcut does not exist on Android, so it doesn't apply to custom
// tab.
case InstallTrigger::CREATE_SHORTCUT:
DCHECK(!is_custom_tab);
return WebappInstallSource::MENU_CREATE_SHORTCUT;
}
NOTREACHED();
return WebappInstallSource::COUNT;
......
......@@ -18,6 +18,7 @@ enum class InstallTrigger {
API,
AUTOMATIC_PROMPT,
MENU,
CREATE_SHORTCUT,
};
// Sources for triggering webapp installation.
......@@ -77,6 +78,9 @@ enum class WebappInstallSource {
// Installed from sync (not reported by |TrackInstallEvent|).
SYNC = 16,
// Create shortcut item in menu
MENU_CREATE_SHORTCUT = 17,
// Add any new values above this one.
COUNT,
};
......
......@@ -3,9 +3,11 @@
// found in the LICENSE file.
#include "base/run_loop.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/metrics/user_action_tester.h"
#include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/banners/test_app_banner_manager_desktop.h"
#include "chrome/browser/installable/installable_metrics.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_commands.h"
#include "chrome/browser/ui/browser_dialogs.h"
......@@ -16,6 +18,7 @@
#include "chrome/browser/web_applications/components/app_registry_controller.h"
#include "chrome/browser/web_applications/components/web_app_constants.h"
#include "chrome/browser/web_applications/components/web_app_id.h"
#include "chrome/browser/web_applications/components/web_app_prefs_utils.h"
#include "chrome/browser/web_applications/components/web_app_provider_base.h"
#include "chrome/browser/web_applications/test/web_app_install_observer.h"
#include "chrome/browser/web_applications/test/web_app_test.h"
......@@ -63,6 +66,29 @@ IN_PROC_BROWSER_TEST_P(CreateShortcutBrowserTest,
EXPECT_EQ(1, user_action_tester.GetActionCount("CreateShortcut"));
}
IN_PROC_BROWSER_TEST_P(CreateShortcutBrowserTest, InstallSourceRecorded) {
ASSERT_TRUE(embedded_test_server()->Start());
// LatestWebAppInstallSource should be correctly set and reported to UMA for
// both installable and non-installable sites.
for (const GURL& url : {GetInstallableAppURL(),
embedded_test_server()->GetURL(
"/web_apps/theme_color_only_manifest.html")}) {
base::HistogramTester histogram_tester;
NavigateToURLAndWait(browser(), url);
AppId app_id = InstallShortcutAppForCurrentUrl();
base::Optional<int> install_source = GetIntWebAppPref(
profile()->GetPrefs(), app_id, kLatestWebAppInstallSource);
EXPECT_TRUE(install_source.has_value());
EXPECT_EQ(static_cast<WebappInstallSource>(*install_source),
WebappInstallSource::MENU_CREATE_SHORTCUT);
histogram_tester.ExpectUniqueSample(
"Webapp.Install.InstallEvent",
static_cast<int>(WebappInstallSource::MENU_CREATE_SHORTCUT), 1);
}
}
IN_PROC_BROWSER_TEST_P(CreateShortcutBrowserTest,
CanInstallOverTabShortcutApp) {
NavigateToURLAndWait(browser(), GetInstallableAppURL());
......
......@@ -108,8 +108,9 @@ void CreateWebAppFromCurrentWebContents(Browser* browser,
auto* provider = WebAppProvider::GetForWebContents(web_contents);
DCHECK(provider);
WebappInstallSource install_source =
InstallableMetrics::GetInstallSource(web_contents, InstallTrigger::MENU);
WebappInstallSource install_source = InstallableMetrics::GetInstallSource(
web_contents, force_shortcut_app ? InstallTrigger::CREATE_SHORTCUT
: InstallTrigger::MENU);
WebAppInstalledCallback callback = base::DoNothing();
......
......@@ -57,6 +57,7 @@ Source::Type InferSourceFromMetricsInstallSource(
case WebappInstallSource::AMBIENT_BADGE_CUSTOM_TAB:
case WebappInstallSource::OMNIBOX_INSTALL_ICON:
case WebappInstallSource::SYNC:
case WebappInstallSource::MENU_CREATE_SHORTCUT:
return Source::kSync;
case WebappInstallSource::INTERNAL_DEFAULT:
......
......@@ -202,7 +202,7 @@ void WebAppInstallTask::InstallWebAppFromInfo(
install_source_ = install_source;
background_installation_ = true;
RecordInstallEvent(for_installable_site);
RecordInstallEvent();
InstallFinalizer::FinalizeOptions options;
options.install_source = install_source;
......@@ -314,12 +314,10 @@ void WebAppInstallTask::CheckInstallPreconditions() {
initiated_ = true;
}
void WebAppInstallTask::RecordInstallEvent(
ForInstallableSite for_installable_site) {
void WebAppInstallTask::RecordInstallEvent() {
DCHECK(install_source_ != kNoInstallSource);
if (InstallableMetrics::IsReportableInstallSource(install_source_) &&
for_installable_site == ForInstallableSite::kYes) {
if (InstallableMetrics::IsReportableInstallSource(install_source_)) {
InstallableMetrics::TrackInstallEvent(install_source_);
}
}
......@@ -717,7 +715,7 @@ void WebAppInstallTask::OnDialogCompleted(
WebApplicationInfo web_app_info_copy = *web_app_info;
// This metric is recorded regardless of the installation result.
RecordInstallEvent(for_installable_site);
RecordInstallEvent();
InstallFinalizer::FinalizeOptions finalize_options;
finalize_options.install_source = install_source_;
......
......@@ -158,7 +158,7 @@ class WebAppInstallTask : content::WebContentsObserver {
private:
void CheckInstallPreconditions();
void RecordInstallEvent(ForInstallableSite for_installable_site);
void RecordInstallEvent();
// Calling the callback may destroy |this| task. Callers shouldn't work with
// any |this| class members after calling it.
......
......@@ -74711,6 +74711,7 @@ Full version information for the fingerprint enum values:
<int value="14" label="System default app on Chrome OS"/>
<int value="15" label="Install icon in the Omnibox"/>
<int value="16" label="Installed from sync"/>
<int value="17" label="Create shortcut from menu on desktop"/>
</enum>
<enum name="WebAppLauncherLaunchResult">
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