[M140] [iOS][safari-data-import] Track Safari import usages with FET
Original change's description:
> [iOS][safari-data-import] Track Safari import usages with FET
>
> Fixing a bug that the Safari import might be displayed before it was
> expected. This was because despite that in the code review response for
> crrev.com/c/6614357, I promised that I would call `tracker->NotifyUsed
> Event(feature)`, but never wrote a comment to remind myself to do so.
>
> Fixed: 437429581
> Change-Id: Iab6f531192800f942aad6afb118ea0da743cf12b
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6829450
> Reviewed-by: Robbie Gibson <rkgibson@google.com>
> Auto-Submit: Ginny Huang <ginnyhuang@chromium.org>
> Commit-Queue: Ginny Huang <ginnyhuang@chromium.org>
> Commit-Queue: Robbie Gibson <rkgibson@google.com>
> Cr-Commit-Position: refs/heads/main@{#1499650}
Bug: 438215231,437429581
Change-Id: Iab6f531192800f942aad6afb118ea0da743cf12b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6842540
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Chrome Cherry Picker <chrome-cherry-picker@chops-service-accounts.iam.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/7339@{#455}
Cr-Branched-From: 27be8b77710f4405fdfeb4ee946fcabb0f6c92b2-refs/heads/main@{#1496484}
diff --git a/ios/chrome/browser/safari_data_import/coordinator/BUILD.gn b/ios/chrome/browser/safari_data_import/coordinator/BUILD.gn
index 22af4515..f9c528e 100644
--- a/ios/chrome/browser/safari_data_import/coordinator/BUILD.gn
+++ b/ios/chrome/browser/safari_data_import/coordinator/BUILD.gn
@@ -15,6 +15,8 @@
":coordinator_delegate",
":export_coordinator",
"//base",
+ "//components/feature_engagement/public",
+ "//ios/chrome/browser/feature_engagement/model",
"//ios/chrome/browser/passwords/model:features",
"//ios/chrome/browser/promos_manager/model",
"//ios/chrome/browser/promos_manager/model:factory",
@@ -116,10 +118,15 @@
sources = [ "safari_data_import_entry_point_mediator_unittest.mm" ]
deps = [
":coordinator",
+ "//base",
+ "//components/feature_engagement/public",
+ "//components/feature_engagement/test:test_support",
"//ios/chrome/app/profile",
+ "//ios/chrome/browser/feature_engagement/model",
"//ios/chrome/browser/promos_manager/model",
"//ios/chrome/browser/promos_manager/model:test_support",
"//ios/chrome/browser/shared/coordinator/scene:scene_state_header",
+ "//ios/web/public/test",
"//testing/gtest",
"//third_party/ocmock",
]
diff --git a/ios/chrome/browser/safari_data_import/coordinator/safari_data_import_entry_point_mediator.h b/ios/chrome/browser/safari_data_import/coordinator/safari_data_import_entry_point_mediator.h
index c9912dc..01c8f768 100644
--- a/ios/chrome/browser/safari_data_import/coordinator/safari_data_import_entry_point_mediator.h
+++ b/ios/chrome/browser/safari_data_import/coordinator/safari_data_import_entry_point_mediator.h
@@ -7,6 +7,9 @@
#import <Foundation/Foundation.h>
+namespace feature_engagement {
+class Tracker;
+}
class PromosManager;
@protocol UIBlockerTarget;
@@ -16,12 +19,16 @@
/// Initializer.
- (instancetype)initWithUIBlockerTarget:(id<UIBlockerTarget>)target
promosManager:(PromosManager*)promosManager
+ featureEngagementTracker:(feature_engagement::Tracker*)tracker
NS_DESIGNATED_INITIALIZER;
- (instancetype)init NS_UNAVAILABLE;
/// Displays the entry point again in a few days.
- (void)registerReminder;
+/// Mark Safari data import workflow as used or dismissed by the user.
+- (void)notifyUsedOrDismissed;
+
/// Disconnects mediator dependencies; should be called when stopping the
/// coordinator.
- (void)disconnect;
diff --git a/ios/chrome/browser/safari_data_import/coordinator/safari_data_import_entry_point_mediator.mm b/ios/chrome/browser/safari_data_import/coordinator/safari_data_import_entry_point_mediator.mm
index cfc9c58..2087ecd8 100644
--- a/ios/chrome/browser/safari_data_import/coordinator/safari_data_import_entry_point_mediator.mm
+++ b/ios/chrome/browser/safari_data_import/coordinator/safari_data_import_entry_point_mediator.mm
@@ -6,6 +6,9 @@
#import "base/check.h"
#import "base/memory/raw_ptr.h"
+#import "components/feature_engagement/public/event_constants.h"
+#import "components/feature_engagement/public/feature_constants.h"
+#import "components/feature_engagement/public/tracker.h"
#import "ios/chrome/browser/promos_manager/model/constants.h"
#import "ios/chrome/browser/promos_manager/model/promos_manager.h"
#import "ios/chrome/browser/scoped_ui_blocker/ui_bundled/scoped_ui_blocker.h"
@@ -16,10 +19,13 @@
std::unique_ptr<ScopedUIBlocker> _UIBlocker;
/// Promos manager that is used to register the reminder.
raw_ptr<PromosManager> _promosManager;
+ /// Feature engagement tracker used for the reminder.
+ raw_ptr<feature_engagement::Tracker> _tracker;
}
- (instancetype)initWithUIBlockerTarget:(id<UIBlockerTarget>)target
- promosManager:(PromosManager*)promosManager {
+ promosManager:(PromosManager*)promosManager
+ featureEngagementTracker:(feature_engagement::Tracker*)tracker {
self = [super init];
if (self) {
CHECK(target);
@@ -27,6 +33,7 @@
_UIBlocker =
std::make_unique<ScopedUIBlocker>(target, UIBlockerExtent::kProfile);
_promosManager = promosManager;
+ _tracker = tracker;
}
return self;
}
@@ -34,6 +41,12 @@
- (void)registerReminder {
_promosManager->RegisterPromoForSingleDisplay(
promos_manager::Promo::SafariImportRemindMeLater);
+ _tracker->NotifyEvent(
+ feature_engagement::events::kIOSSafariImportRemindMeLater);
+}
+
+- (void)notifyUsedOrDismissed {
+ _tracker->NotifyUsedEvent(feature_engagement::kIPHiOSSafariImportFeature);
}
- (void)disconnect {
diff --git a/ios/chrome/browser/safari_data_import/coordinator/safari_data_import_entry_point_mediator_unittest.mm b/ios/chrome/browser/safari_data_import/coordinator/safari_data_import_entry_point_mediator_unittest.mm
index 3112de0..6daba00 100644
--- a/ios/chrome/browser/safari_data_import/coordinator/safari_data_import_entry_point_mediator_unittest.mm
+++ b/ios/chrome/browser/safari_data_import/coordinator/safari_data_import_entry_point_mediator_unittest.mm
@@ -6,10 +6,18 @@
#import <memory>
+#import "base/run_loop.h"
+#import "components/feature_engagement/public/event_constants.h"
+#import "components/feature_engagement/public/feature_list.h"
+#import "components/feature_engagement/public/tracker.h"
+#import "components/feature_engagement/test/scoped_iph_feature_list.h"
+#import "components/feature_engagement/test/test_tracker.h"
#import "ios/chrome/app/profile/profile_state.h"
+#import "ios/chrome/browser/feature_engagement/model/tracker_factory.h"
#import "ios/chrome/browser/promos_manager/model/constants.h"
#import "ios/chrome/browser/promos_manager/model/mock_promos_manager.h"
#import "ios/chrome/browser/shared/coordinator/scene/scene_state.h"
+#import "ios/web/public/test/web_task_environment.h"
#import "testing/gmock/include/gmock/gmock.h"
#import "testing/platform_test.h"
@@ -17,19 +25,35 @@
class SafariDataImportEntryPointMediatorTest : public PlatformTest {
public:
SafariDataImportEntryPointMediatorTest() : PlatformTest() {
+ feature_engagement::test::ScopedIphFeatureList list;
+ list.InitAndEnableFeatures(
+ {feature_engagement::kIPHiOSSafariImportFeature});
+
ProfileState* profile_state = [[ProfileState alloc] initWithAppState:nil];
scene_state_ = [[SceneState alloc] initWithAppState:nil];
scene_state_.profileState = profile_state;
promos_manager_ = std::make_unique<MockPromosManager>();
+ tracker_ = feature_engagement::CreateTestTracker();
+ tracker_->AddOnInitializedCallback(BoolArgumentQuitClosure());
+
mediator_ = [[SafariDataImportEntryPointMediator alloc]
- initWithUIBlockerTarget:scene_state_
- promosManager:promos_manager_.get()];
+ initWithUIBlockerTarget:scene_state_
+ promosManager:promos_manager_.get()
+ featureEngagementTracker:tracker_.get()];
+ }
+
+ base::RepeatingCallback<void(bool)> BoolArgumentQuitClosure() {
+ return base::IgnoreArgs<bool>(run_loop_.QuitClosure());
}
protected:
+ web::WebTaskEnvironment task_environment_{
+ base::test::TaskEnvironment::TimeSource::MOCK_TIME};
SceneState* scene_state_;
std::unique_ptr<MockPromosManager> promos_manager_;
+ std::unique_ptr<feature_engagement::Tracker> tracker_;
SafariDataImportEntryPointMediator* mediator_;
+ base::RunLoop run_loop_;
};
// Tests that the Safari import reminder is registered on request.
@@ -39,4 +63,24 @@
RegisterPromoForSingleDisplay(
promos_manager::Promo::SafariImportRemindMeLater));
[mediator_ registerReminder];
+ // Tests that reminder is displayed two days later.
+ task_environment_.FastForwardBy(base::Days(1.1));
+ EXPECT_FALSE(tracker_->ShouldTriggerHelpUI(
+ feature_engagement::kIPHiOSSafariImportFeature));
+ task_environment_.FastForwardBy(base::Days(1.1));
+ EXPECT_TRUE(tracker_->ShouldTriggerHelpUI(
+ feature_engagement::kIPHiOSSafariImportFeature));
+}
+
+// Tests that the reminder would not be displayed once the mediator marks Safari
+// import as used or dismissed.
+TEST_F(SafariDataImportEntryPointMediatorTest,
+ TestNoReminderAfterUsedOrDismissed) {
+ [mediator_ notifyUsedOrDismissed];
+ // Register reminder and test.
+ tracker_->NotifyEvent(
+ feature_engagement::events::kIOSSafariImportRemindMeLater);
+ task_environment_.FastForwardBy(base::Days(2.1));
+ EXPECT_FALSE(tracker_->ShouldTriggerHelpUI(
+ feature_engagement::kIPHiOSSafariImportFeature));
}
diff --git a/ios/chrome/browser/safari_data_import/coordinator/safari_data_import_main_coordinator.mm b/ios/chrome/browser/safari_data_import/coordinator/safari_data_import_main_coordinator.mm
index f82959dd..19749ec8 100644
--- a/ios/chrome/browser/safari_data_import/coordinator/safari_data_import_main_coordinator.mm
+++ b/ios/chrome/browser/safari_data_import/coordinator/safari_data_import_main_coordinator.mm
@@ -8,6 +8,7 @@
#import "base/check.h"
#import "base/feature_list.h"
+#import "ios/chrome/browser/feature_engagement/model/tracker_factory.h"
#import "ios/chrome/browser/passwords/model/features.h"
#import "ios/chrome/browser/promos_manager/model/promos_manager.h"
#import "ios/chrome/browser/promos_manager/model/promos_manager_factory.h"
@@ -57,9 +58,12 @@
_viewController.actionHandler = self;
PromosManager* promosManager =
PromosManagerFactory::GetForProfile(self.profile);
+ feature_engagement::Tracker* tracker =
+ feature_engagement::TrackerFactory::GetForProfile(self.profile);
_mediator = [[SafariDataImportEntryPointMediator alloc]
- initWithUIBlockerTarget:self.browser->GetSceneState()
- promosManager:promosManager];
+ initWithUIBlockerTarget:self.browser->GetSceneState()
+ promosManager:promosManager
+ featureEngagementTracker:tracker];
[self.baseViewController presentViewController:_viewController
animated:YES
completion:nil];
@@ -85,6 +89,7 @@
RecordSafariImportActionOnEntryPoint(
SafariDataImportEntryPointAction::kImport, _entryPoint);
CHECK(!_exportCoordinator);
+ [_mediator notifyUsedOrDismissed];
_exportCoordinator = [[SafariDataImportExportCoordinator alloc]
initWithBaseViewController:_viewController
browser:self.browser];
@@ -102,6 +107,7 @@
- (void)confirmationAlertDismissAction {
RecordSafariImportActionOnEntryPoint(
SafariDataImportEntryPointAction::kDismiss, _entryPoint);
+ [_mediator notifyUsedOrDismissed];
[self.delegate safariImportWorkflowDidEndForCoordinator:self];
}
