Skip to content

Commit

Permalink
Remove app banner RAPPOR metrics
Browse files Browse the repository at this point in the history
RAPPOR is deprecated in favor of UKM. As part of BMO we plan on revisiting
our metrics as a whole so for now these RAPPOR metrics can be removed.

Bug: 1016906
Change-Id: If2de040847161c7d6f4d980fc8e99d480399a61b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1890377
Auto-Submit: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712040}
  • Loading branch information
alancutter authored and Commit Bot committed Nov 4, 2019
1 parent af69db6 commit 961c02c
Show file tree
Hide file tree
Showing 9 changed files with 18 additions and 201 deletions.
38 changes: 0 additions & 38 deletions chrome/browser/android/metrics/launch_metrics.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,9 @@
#include "chrome/android/chrome_jni_headers/LaunchMetrics_jni.h"
#include "chrome/browser/android/shortcut_info.h"
#include "chrome/browser/banners/app_banner_settings_helper.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/engagement/site_engagement_service.h"
#include "chrome/browser/prefs/pref_metrics_service.h"
#include "chrome/browser/profiles/profile.h"
#include "components/rappor/public/rappor_utils.h"
#include "components/rappor/rappor_service_impl.h"
#include "content/public/browser/web_contents.h"
#include "third_party/blink/public/mojom/manifest/display_mode.mojom.h"
#include "url/gurl.h"
Expand Down Expand Up @@ -65,32 +62,6 @@ static void JNI_LaunchMetrics_RecordLaunch(
service->SetLastShortcutLaunchTime(web_contents, url);
}

std::string rappor_metric_source;
switch (histogram_source) {
case ShortcutInfo::SOURCE_ADD_TO_HOMESCREEN_DEPRECATED:
case ShortcutInfo::SOURCE_ADD_TO_HOMESCREEN_PWA:
case ShortcutInfo::SOURCE_ADD_TO_HOMESCREEN_STANDALONE:
case ShortcutInfo::SOURCE_ADD_TO_HOMESCREEN_SHORTCUT:
rappor_metric_source = "Launch.HomeScreenSource.AddToHomeScreen";
break;
case ShortcutInfo::SOURCE_APP_BANNER:
rappor_metric_source = "Launch.HomeScreenSource.AppBanner";
break;
case ShortcutInfo::SOURCE_BOOKMARK_NAVIGATOR_WIDGET:
rappor_metric_source = "Launch.HomeScreenSource.BookmarkNavigatorWidget";
break;
case ShortcutInfo::SOURCE_BOOKMARK_SHORTCUT_WIDGET:
rappor_metric_source = "Launch.HomeScreenSource.BookmarkShortcutWidget";
break;
case ShortcutInfo::SOURCE_NOTIFICATION:
rappor_metric_source = "Launch.HomeScreenSource.Notification";
break;
case ShortcutInfo::SOURCE_UNKNOWN:
case ShortcutInfo::SOURCE_COUNT:
rappor_metric_source = "Launch.HomeScreenSource.Unknown";
break;
}

UMA_HISTOGRAM_ENUMERATION("Launch.HomeScreenSource",
static_cast<ShortcutInfo::Source>(histogram_source),
ShortcutInfo::SOURCE_COUNT);
Expand All @@ -101,20 +72,11 @@ static void JNI_LaunchMetrics_RecordLaunch(
static_cast<blink::mojom::DisplayMode>(display_mode));
}

rappor::SampleDomainAndRegistryFromGURL(g_browser_process->rappor_service(),
rappor_metric_source, url);

HomeScreenLaunchType action = is_shortcut ? HomeScreenLaunchType::SHORTCUT
: HomeScreenLaunchType::STANDALONE;
std::string rappor_metric_action = is_shortcut
? "Launch.HomeScreen.Shortcut"
: "Launch.HomeScreen.Standalone";

UMA_HISTOGRAM_ENUMERATION("Launch.HomeScreen", action,
HomeScreenLaunchType::COUNT);

rappor::SampleDomainAndRegistryFromGURL(g_browser_process->rappor_service(),
rappor_metric_action, url);
}

static void JNI_LaunchMetrics_RecordHomePageLaunchMetrics(
Expand Down
8 changes: 1 addition & 7 deletions chrome/browser/banners/app_banner_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,11 @@
#include "base/time/time.h"
#include "chrome/browser/banners/app_banner_metrics.h"
#include "chrome/browser/banners/app_banner_settings_helper.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/engagement/site_engagement_service.h"
#include "chrome/browser/installable/installable_metrics.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/common/chrome_features.h"
#include "chrome/common/chrome_switches.h"
#include "components/rappor/public/rappor_utils.h"
#include "components/rappor/rappor_service_impl.h"
#include "content/public/browser/back_forward_cache.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/render_frame_host.h"
Expand Down Expand Up @@ -399,17 +396,14 @@ void AppBannerManager::OnDidPerformInstallableWebAppCheck(
SendBannerPromptRequest();
}

void AppBannerManager::RecordDidShowBanner(const std::string& event_name) {
void AppBannerManager::RecordDidShowBanner() {
content::WebContents* contents = web_contents();
DCHECK(contents);

AppBannerSettingsHelper::RecordBannerEvent(
contents, validated_url_, GetAppIdentifier(),
AppBannerSettingsHelper::APP_BANNER_EVENT_DID_SHOW,
GetCurrentTime());
rappor::SampleDomainAndRegistryFromGURL(g_browser_process->rappor_service(),
event_name,
contents->GetLastCommittedURL());
}

void AppBannerManager::ReportStatus(InstallableStatusCode code) {
Expand Down
5 changes: 2 additions & 3 deletions chrome/browser/banners/app_banner_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -241,9 +241,8 @@ class AppBannerManager : public content::WebContentsObserver,
virtual void OnDidPerformInstallableWebAppCheck(
const InstallableData& result);

// Records that a banner was shown. The |event_name| corresponds to the RAPPOR
// metric being recorded.
void RecordDidShowBanner(const std::string& event_name);
// Records that a banner was shown.
void RecordDidShowBanner();

// Reports |code| via a UMA histogram or logs it to the console.
void ReportStatus(InstallableStatusCode code);
Expand Down
13 changes: 5 additions & 8 deletions chrome/browser/banners/app_banner_manager_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -267,8 +267,7 @@ void AppBannerManagerAndroid::RecordEventForAppBanner(
case AddToHomescreenParams::AppType::SHORTCUT:
TrackUserResponse(USER_RESPONSE_WEB_APP_ACCEPTED);
AppBannerSettingsHelper::RecordBannerInstallEvent(
web_contents(), a2hs_params.shortcut_info->url.spec(),
AppBannerSettingsHelper::WEB);
web_contents(), a2hs_params.shortcut_info->url.spec());
break;
default:
NOTREACHED();
Expand Down Expand Up @@ -303,10 +302,10 @@ void AppBannerManagerAndroid::RecordEventForAppBanner(

case AddToHomescreenInstaller::Event::UI_SHOWN:
if (a2hs_params.app_type == AddToHomescreenParams::AppType::NATIVE) {
RecordDidShowBanner("AppBanner.NativeApp.Shown");
RecordDidShowBanner();
TrackDisplayEvent(DISPLAY_EVENT_NATIVE_APP_BANNER_CREATED);
} else {
RecordDidShowBanner("AppBanner.WebApp.Shown");
RecordDidShowBanner();
TrackDisplayEvent(DISPLAY_EVENT_WEB_APP_BANNER_CREATED);
}
break;
Expand All @@ -319,16 +318,14 @@ void AppBannerManagerAndroid::RecordEventForAppBanner(
DCHECK(!a2hs_params.native_app_package_name.empty());
TrackUserResponse(USER_RESPONSE_NATIVE_APP_DISMISSED);
AppBannerSettingsHelper::RecordBannerDismissEvent(
web_contents(), a2hs_params.native_app_package_name,
AppBannerSettingsHelper::NATIVE);
web_contents(), a2hs_params.native_app_package_name);
} else {
if (a2hs_params.app_type == AddToHomescreenParams::AppType::WEBAPK)
webapk::TrackInstallEvent(
webapk::ADD_TO_HOMESCREEN_DIALOG_DISMISSED_BEFORE_INSTALLATION);
TrackUserResponse(USER_RESPONSE_WEB_APP_DISMISSED);
AppBannerSettingsHelper::RecordBannerDismissEvent(
web_contents(), a2hs_params.shortcut_info->url.spec(),
AppBannerSettingsHelper::WEB);
web_contents(), a2hs_params.shortcut_info->url.spec());
}
break;
}
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/banners/app_banner_manager_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ class AppBannerManagerTest : public AppBannerManager {
// Fake the call to ReportStatus here - this is usually called in
// platform-specific code which is not exposed here.
ReportStatus(SHOWING_WEB_APP_BANNER);
RecordDidShowBanner("AppBanner.WebApp.Shown");
RecordDidShowBanner();

ASSERT_FALSE(banner_shown_.get());
banner_shown_.reset(new bool(true));
Expand Down
10 changes: 5 additions & 5 deletions chrome/browser/banners/app_banner_manager_desktop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ bool AppBannerManagerDesktop::ShouldAllowWebAppReplacementInstall() {
}

void AppBannerManagerDesktop::ShowBannerUi(WebappInstallSource install_source) {
RecordDidShowBanner("AppBanner.WebApp.Shown");
RecordDidShowBanner();
TrackDisplayEvent(DISPLAY_EVENT_WEB_APP_BANNER_CREATED);
ReportStatus(SHOWING_APP_INSTALLATION_DIALOG);
CreateWebApp(install_source);
Expand Down Expand Up @@ -199,13 +199,13 @@ void AppBannerManagerDesktop::DidFinishCreatingWebApp(
if (code == web_app::InstallResultCode::kSuccessNewInstall) {
SendBannerAccepted();
TrackUserResponse(USER_RESPONSE_WEB_APP_ACCEPTED);
AppBannerSettingsHelper::RecordBannerInstallEvent(
contents, GetAppIdentifier(), AppBannerSettingsHelper::WEB);
AppBannerSettingsHelper::RecordBannerInstallEvent(contents,
GetAppIdentifier());
} else if (code == web_app::InstallResultCode::kUserInstallDeclined) {
SendBannerDismissed();
TrackUserResponse(USER_RESPONSE_WEB_APP_DISMISSED);
AppBannerSettingsHelper::RecordBannerDismissEvent(
contents, GetAppIdentifier(), AppBannerSettingsHelper::WEB);
AppBannerSettingsHelper::RecordBannerDismissEvent(contents,
GetAppIdentifier());
}
}

Expand Down
21 changes: 2 additions & 19 deletions chrome/browser/banners/app_banner_settings_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,13 @@
#include "base/util/values/values_util.h"
#include "chrome/browser/banners/app_banner_manager.h"
#include "chrome/browser/banners/app_banner_metrics.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/installable/installable_logging.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/common/chrome_features.h"
#include "chrome/common/chrome_switches.h"
#include "components/content_settings/core/browser/host_content_settings_map.h"
#include "components/content_settings/core/common/content_settings_pattern.h"
#include "components/rappor/public/rappor_utils.h"
#include "components/rappor/rappor_service_impl.h"
#include "components/variations/variations_associated_data.h"
#include "content/public/browser/web_contents.h"
#include "url/gurl.h"
Expand Down Expand Up @@ -262,40 +259,26 @@ void AppBannerSettingsHelper::ClearHistoryForURLs(

void AppBannerSettingsHelper::RecordBannerInstallEvent(
content::WebContents* web_contents,
const std::string& package_name_or_start_url,
AppBannerRapporMetric rappor_metric) {
const std::string& package_name_or_start_url) {
banners::TrackInstallEvent(banners::INSTALL_EVENT_WEB_APP_INSTALLED);

AppBannerSettingsHelper::RecordBannerEvent(
web_contents, web_contents->GetLastCommittedURL(),
package_name_or_start_url,
AppBannerSettingsHelper::APP_BANNER_EVENT_DID_ADD_TO_HOMESCREEN,
banners::AppBannerManager::GetCurrentTime());

rappor::SampleDomainAndRegistryFromGURL(
g_browser_process->rappor_service(),
(rappor_metric == WEB ? "AppBanner.WebApp.Installed"
: "AppBanner.NativeApp.Installed"),
web_contents->GetLastCommittedURL());
}

void AppBannerSettingsHelper::RecordBannerDismissEvent(
content::WebContents* web_contents,
const std::string& package_name_or_start_url,
AppBannerRapporMetric rappor_metric) {
const std::string& package_name_or_start_url) {
banners::TrackDismissEvent(banners::DISMISS_EVENT_CLOSE_BUTTON);

AppBannerSettingsHelper::RecordBannerEvent(
web_contents, web_contents->GetLastCommittedURL(),
package_name_or_start_url,
AppBannerSettingsHelper::APP_BANNER_EVENT_DID_BLOCK,
banners::AppBannerManager::GetCurrentTime());

rappor::SampleDomainAndRegistryFromGURL(
g_browser_process->rappor_service(),
(rappor_metric == WEB ? "AppBanner.WebApp.Dismissed"
: "AppBanner.NativeApp.Dismissed"),
web_contents->GetLastCommittedURL());
}

void AppBannerSettingsHelper::RecordBannerEvent(
Expand Down
11 changes: 2 additions & 9 deletions chrome/browser/banners/app_banner_settings_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,6 @@ class AppBannerSettingsHelper {
APP_BANNER_EVENT_NUM_EVENTS,
};

enum AppBannerRapporMetric {
WEB,
NATIVE,
};

static const char kInstantAppsKey[];

// The content setting basically records a simplified subset of history.
Expand All @@ -90,14 +85,12 @@ class AppBannerSettingsHelper {
// Record a banner installation event, for either a WEB or NATIVE app.
static void RecordBannerInstallEvent(
content::WebContents* web_contents,
const std::string& package_name_or_start_url,
AppBannerRapporMetric rappor_metric);
const std::string& package_name_or_start_url);

// Record a banner dismissal event, for either a WEB or NATIVE app.
static void RecordBannerDismissEvent(
content::WebContents* web_contents,
const std::string& package_name_or_start_url,
AppBannerRapporMetric rappor_metric);
const std::string& package_name_or_start_url);

// Record a banner event specified by |event|.
static void RecordBannerEvent(content::WebContents* web_contents,
Expand Down

0 comments on commit 961c02c

Please sign in to comment.