Skip to content

Commit

Permalink
[iOS] Convert promos CHECK to a DWC and add necessary data as crash key
Browse files Browse the repository at this point in the history
This includes which promo failed to be dismissed as crash data, to help
fixes.

(cherry picked from commit 8f84dd1)

Bug: 1443982
Change-Id: Ica32e3154f7fbddaf9c67308ec8e787caabddc41
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4572516
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Commit-Queue: Robbie Gibson <rkgibson@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1154898}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4605926
Reviewed-by: Benjamin Williams <bwwilliams@google.com>
Auto-Submit: Robbie Gibson <rkgibson@google.com>
Cr-Commit-Position: refs/branch-heads/5790@{#701}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
rkgibson2 authored and Chromium LUCI CQ committed Jun 13, 2023
1 parent 96d8cbe commit 7eff173
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 14 deletions.
21 changes: 13 additions & 8 deletions ios/chrome/browser/promos_manager/constants.cc
Expand Up @@ -5,6 +5,7 @@
#include "ios/chrome/browser/promos_manager/constants.h"

#include "base/notreached.h"
#include "base/strings/strcat.h"
#import "third_party/abseil-cpp/absl/types/optional.h"

namespace promos_manager {
Expand Down Expand Up @@ -42,22 +43,26 @@ absl::optional<Promo> PromoForName(base::StringPiece promo) {
return absl::nullopt;
}

base::StringPiece NameForPromo(Promo promo) {
std::string NameForPromo(Promo promo) {
return base::StrCat({"promos_manager::Promo::", ShortNameForPromo(promo)});
}

base::StringPiece ShortNameForPromo(Promo promo) {
switch (promo) {
case promos_manager::Promo::Test:
return "promos_manager::Promo::Test";
return "Test";
case promos_manager::Promo::DefaultBrowser:
return "promos_manager::Promo::DefaultBrowser";
return "DefaultBrowser";
case promos_manager::Promo::AppStoreRating:
return "promos_manager::Promo::AppStoreRating";
return "AppStoreRating";
case promos_manager::Promo::CredentialProviderExtension:
return "promos_manager::Promo::CredentialProviderExtension";
return "CredentialProviderExtension";
case promos_manager::Promo::PostRestoreSignInFullscreen:
return "promos_manager::Promo::PostRestoreSignInFullscreen";
return "PostRestoreSignInFullscreen";
case promos_manager::Promo::PostRestoreSignInAlert:
return "promos_manager::Promo::PostRestoreSignInAlert";
return "PostRestoreSignInAlert";
case promos_manager::Promo::WhatsNew:
return "promos_manager::Promo::WhatsNew";
return "WhatsNew";
}
}

Expand Down
5 changes: 4 additions & 1 deletion ios/chrome/browser/promos_manager/constants.h
Expand Up @@ -71,7 +71,10 @@ struct Impression {
};

// Returns string representation of promos_manager::Promo `promo`.
base::StringPiece NameForPromo(Promo promo);
std::string NameForPromo(Promo promo);

// Returns a string representation of the short name for the provided `promo`.
base::StringPiece ShortNameForPromo(Promo promo);

// Returns promos_manager::Promo for string `promo`.
absl::optional<Promo> PromoForName(base::StringPiece promo);
Expand Down
6 changes: 3 additions & 3 deletions ios/chrome/browser/promos_manager/promos_manager_impl.mm
Expand Up @@ -52,7 +52,7 @@ void ConditionallyAppendPromoToPrefList(promos_manager::Promo promo,

ScopedListPrefUpdate update(local_state, pref_path);

base::StringPiece promo_name = promos_manager::NameForPromo(promo);
std::string promo_name = promos_manager::NameForPromo(promo);

// Erase `promo_name` if it already exists in `active_promos`; avoid polluting
// `active_promos` with duplicate `promo_name` entries.
Expand Down Expand Up @@ -170,7 +170,7 @@ ScopedListPrefUpdate update(local_state_,
// update the pending promos saved in pref.
ScopedDictPrefUpdate pending_promos_update(
local_state_, prefs::kIosPromosManagerSingleDisplayPendingPromos);
base::StringPiece promo_name = promos_manager::NameForPromo(promo);
std::string promo_name = promos_manager::NameForPromo(promo);
base::Time becomes_active_time = clock_->Now() + becomes_active_after_period;
pending_promos_update->Set(promo_name,
base::TimeToValue(becomes_active_time));
Expand All @@ -190,7 +190,7 @@ ScopedListPrefUpdate single_display_promos_update(
ScopedDictPrefUpdate pending_promos_update(
local_state_, prefs::kIosPromosManagerSingleDisplayPendingPromos);

base::StringPiece promo_name = promos_manager::NameForPromo(promo);
std::string promo_name = promos_manager::NameForPromo(promo);

// Erase `promo_name` from the single-display and continuous-display active
// promos lists.
Expand Down
1 change: 1 addition & 0 deletions ios/chrome/browser/ui/promos_manager/BUILD.gn
Expand Up @@ -65,6 +65,7 @@ source_set("promos_manager") {
]
deps = [
":promos_manager_scene_agent",
"//components/crash/core/common:crash_key",
"//ios/chrome/app/strings",
"//ios/chrome/browser/credential_provider_promo:features",
"//ios/chrome/browser/default_browser:utils",
Expand Down
14 changes: 12 additions & 2 deletions ios/chrome/browser/ui/promos_manager/promos_manager_coordinator.mm
Expand Up @@ -9,9 +9,11 @@

#import "base/check.h"
#import "base/containers/small_map.h"
#import "base/debug/dump_without_crashing.h"
#import "base/metrics/histogram_functions.h"
#import "base/notreached.h"
#import "base/strings/sys_string_conversions.h"
#import "components/crash/core/common/crash_key.h"
#import "components/feature_engagement/public/tracker.h"
#import "ios/chrome/app/tests_hook.h"
#import "ios/chrome/browser/credential_provider_promo/features.h"
Expand Down Expand Up @@ -207,8 +209,16 @@ - (void)displayPromo:(promos_manager::Promo)promo {
return;
}

CHECK(!current_promo.has_value()) << "Current promo is already set: "
<< NameForPromo(current_promo.value());
// Trying to display a promo while the previous dismissal was not communicated
// back to the promos manager.
// TODO(crbug.com/1452233): Remove once all promos dismiss themselves.
if (current_promo.has_value()) {
static crash_reporter::CrashKeyString<40> key("current-promo");
crash_reporter::ScopedCrashKeyString crashKey(
&key, ShortNameForPromo(current_promo.value()));
base::debug::DumpWithoutCrashing();
}

current_promo = promo;

auto handler_it = _displayHandlerPromos.find(promo);
Expand Down

0 comments on commit 7eff173

Please sign in to comment.