Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4009)

Unified Diff: chrome/browser/banners/app_banner_settings_helper.cc

Issue 2178833002: Add new app banner metrics using InstallableStatusCode. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@banner-integrate-checker-no-refptr
Patch Set: Fix histogram name Created 4 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/banners/app_banner_settings_helper.cc
diff --git a/chrome/browser/banners/app_banner_settings_helper.cc b/chrome/browser/banners/app_banner_settings_helper.cc
index ad25487f89ae72c520c9095211d61b3546644127..f384ca2feb27d87b0c714693844b0ddc03036e99 100644
--- a/chrome/browser/banners/app_banner_settings_helper.cc
+++ b/chrome/browser/banners/app_banner_settings_helper.cc
@@ -19,6 +19,7 @@
#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_switches.h"
#include "components/content_settings/core/browser/host_content_settings_map.h"
@@ -374,7 +375,7 @@ void AppBannerSettingsHelper::RecordBannerCouldShowEvent(
std::move(origin_dict));
}
-bool AppBannerSettingsHelper::ShouldShowBanner(
+InstallableStatusCode AppBannerSettingsHelper::ShouldShowBanner(
content::WebContents* web_contents,
const GURL& origin_url,
const std::string& package_name_or_start_url,
@@ -382,12 +383,12 @@ bool AppBannerSettingsHelper::ShouldShowBanner(
// Ignore all checks if the flag to do so is set.
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kBypassAppBannerEngagementChecks)) {
- return true;
+ return NO_ERROR_DETECTED;
}
// Never show a banner when the package name or URL is empty.
if (package_name_or_start_url.empty())
- return false;
+ return PACKAGE_NAME_OR_START_URL_EMPTY;
// Don't show if it has been added to the homescreen.
base::Time added_time =
@@ -395,7 +396,7 @@ bool AppBannerSettingsHelper::ShouldShowBanner(
APP_BANNER_EVENT_DID_ADD_TO_HOMESCREEN);
if (!added_time.is_null()) {
banners::TrackDisplayEvent(banners::DISPLAY_EVENT_INSTALLED_PREVIOUSLY);
- return false;
+ return ALREADY_INSTALLED;
}
base::Time blocked_time =
@@ -407,7 +408,7 @@ bool AppBannerSettingsHelper::ShouldShowBanner(
if (time - blocked_time <
base::TimeDelta::FromDays(kMinimumBannerBlockedToBannerShown)) {
banners::TrackDisplayEvent(banners::DISPLAY_EVENT_BLOCKED_PREVIOUSLY);
- return false;
+ return PREVIOUSLY_BLOCKED;
}
base::Time shown_time =
@@ -416,7 +417,7 @@ bool AppBannerSettingsHelper::ShouldShowBanner(
if (time - shown_time <
base::TimeDelta::FromDays(kMinimumDaysBetweenBannerShows)) {
banners::TrackDisplayEvent(banners::DISPLAY_EVENT_IGNORED_PREVIOUSLY);
- return false;
+ return PREVIOUSLY_IGNORED;
}
// If we have gotten this far and want to use site engagement, the banner flow
@@ -427,7 +428,7 @@ bool AppBannerSettingsHelper::ShouldShowBanner(
// in this method when app banners have fully migrated to using site
// engagement as a trigger condition. See crbug.com/616322.
if (ShouldUseSiteEngagementScore())
- return true;
+ return NO_ERROR_DETECTED;
double total_engagement = 0;
std::vector<BannerEvent> could_show_events = GetCouldShowBannerEvents(
@@ -438,10 +439,10 @@ bool AppBannerSettingsHelper::ShouldShowBanner(
if (!HasSufficientEngagement(total_engagement)) {
banners::TrackDisplayEvent(banners::DISPLAY_EVENT_NOT_VISITED_ENOUGH);
- return false;
+ return INSUFFICIENT_ENGAGEMENT;
}
- return true;
+ return NO_ERROR_DETECTED;
}
std::vector<AppBannerSettingsHelper::BannerEvent>
« no previous file with comments | « chrome/browser/banners/app_banner_settings_helper.h ('k') | chrome/browser/banners/app_banner_settings_helper_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698