Commit 5fefa0cd authored by Eugene But's avatar Eugene But Committed by Commit Bot

[ios] Log user actions on dismissing settings screens

These actions are helpful for Breadcrumbs feature that attaches steps to
reproduce to crash reports. Hence the code was written to log
UserActions right after the user Tapped Done and before any other code
could run and crash the app.

Bug: 1046231
Change-Id: I5e7e811f27fd5018e7a562330f2841a25272ce7a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2089985
Commit-Queue: Eugene But <eugenebut@chromium.org>
Auto-Submit: Eugene But <eugenebut@chromium.org>
Reviewed-by: default avatarRobert Kaplow <rkaplow@chromium.org>
Reviewed-by: default avatarSergio Collazos <sczs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748286}
parent 1bcea834
......@@ -180,6 +180,7 @@
}
- (void)dismiss {
base::RecordAction(base::UserMetricsAction("MobileClearBrowsingDataClose"));
[self prepareForDismissal];
[self.delegate dismissClearBrowsingData];
}
......@@ -376,6 +377,8 @@
- (void)presentationControllerDidDismiss:
(UIPresentationController*)presentationController {
base::RecordAction(
base::UserMetricsAction("IOSClearBrowsingDataCloseWithSwipe"));
// Call prepareForDismissal to clean up state and stop the Coordinator.
[self prepareForDismissal];
}
......
......@@ -143,6 +143,10 @@ typedef NS_ENUM(NSInteger, ItemType) {
#pragma mark - SettingsControllerProtocol
- (void)reportDismissalUserAction {
base::RecordAction(base::UserMetricsAction("MobileAccountsSettingsClose"));
}
- (void)settingsWillBeDismissed {
[self.signinInteractionCoordinator cancel];
[_alertCoordinator stop];
......@@ -494,4 +498,12 @@ typedef NS_ENUM(NSInteger, ItemType) {
_identityServiceObserver.reset();
}
#pragma mark - UIAdaptivePresentationControllerDelegate
- (void)presentationControllerDidDismiss:
(UIPresentationController*)presentationController {
base::RecordAction(
base::UserMetricsAction("IOSAccountsSettingsCloseWithSwipe"));
}
@end
......@@ -5,6 +5,8 @@
#import "ios/chrome/browser/ui/settings/google_services/google_services_settings_view_controller.h"
#include "base/mac/foundation_util.h"
#include "base/metrics/user_metrics.h"
#include "base/metrics/user_metrics_action.h"
#import "ios/chrome/browser/ui/settings/cells/settings_switch_cell.h"
#import "ios/chrome/browser/ui/settings/cells/sync_switch_item.h"
#import "ios/chrome/browser/ui/settings/google_services/google_services_settings_constants.h"
......@@ -54,6 +56,13 @@
return cell;
}
#pragma mark - SettingsControllerProtocol
- (void)reportDismissalUserAction {
base::RecordAction(
base::UserMetricsAction("MobileGoogleServicesSettingsClose"));
}
#pragma mark - GoogleServicesSettingsConsumer
- (void)insertSections:(NSIndexSet*)sections {
......@@ -125,4 +134,12 @@
[tableView deselectRowAtIndexPath:indexPath animated:YES];
}
#pragma mark - UIAdaptivePresentationControllerDelegate
- (void)presentationControllerDidDismiss:
(UIPresentationController*)presentationController {
base::RecordAction(
base::UserMetricsAction("IOSGoogleServicesSettingsCloseWithSwipe"));
}
@end
......@@ -7,6 +7,8 @@
#include "base/logging.h"
#include "base/mac/foundation_util.h"
#include "base/metrics/histogram_macros.h"
#include "base/metrics/user_metrics.h"
#include "base/metrics/user_metrics_action.h"
#import "ios/chrome/browser/ui/list_model/list_item+Controller.h"
#import "ios/chrome/browser/ui/settings/cells/settings_cells_constants.h"
#import "ios/chrome/browser/ui/settings/cells/settings_switch_cell.h"
......@@ -157,6 +159,11 @@ typedef NS_ENUM(NSInteger, ItemType) {
#pragma mark - SettingsControllerProtocol
- (void)reportDismissalUserAction {
// Language Settings screen does not have Done button.
NOTREACHED();
}
- (void)settingsWillBeDismissed {
[self.dataSource stopObservingModel];
}
......@@ -514,4 +521,12 @@ typedef NS_ENUM(NSInteger, ItemType) {
[self translateEnabled:switchView.isOn];
}
#pragma mark - UIAdaptivePresentationControllerDelegate
- (void)presentationControllerDidDismiss:
(UIPresentationController*)presentationController {
base::RecordAction(
base::UserMetricsAction("IOSLanguagesSettingsCloseWithSwipe"));
}
@end
......@@ -10,6 +10,8 @@
#include "base/logging.h"
#include "base/mac/foundation_util.h"
#include "base/metrics/histogram_macros.h"
#include "base/metrics/user_metrics.h"
#include "base/metrics/user_metrics_action.h"
#include "base/numerics/safe_conversions.h"
#include "base/strings/sys_string_conversions.h"
#include "base/strings/utf_string_conversions.h"
......@@ -406,6 +408,10 @@ std::vector<std::unique_ptr<autofill::PasswordForm>> CopyOf(
#pragma mark - SettingsControllerProtocol
- (void)reportDismissalUserAction {
base::RecordAction(base::UserMetricsAction("MobilePasswordsSettingsClose"));
}
- (void)settingsWillBeDismissed {
// Dismiss the search bar if presented, otherwise the VC will be retained by
// UIKit thus cause a memory leak.
......@@ -1217,4 +1223,12 @@ std::vector<std::unique_ptr<autofill::PasswordForm>> CopyOf(
_identityServiceObserver.reset();
}
#pragma mark - UIAdaptivePresentationControllerDelegate
- (void)presentationControllerDidDismiss:
(UIPresentationController*)presentationController {
base::RecordAction(
base::UserMetricsAction("IOSPasswordsSettingsCloseWithSwipe"));
}
@end
......@@ -10,6 +10,12 @@
// Protocol for settings view controllers.
@protocol SettingsControllerProtocol <NSObject>
@required
// Called when user dismissed settings. View controllers must implement this
// method and report dismissal User Action.
- (void)reportDismissalUserAction;
@optional
// Notifies the controller that the settings screen is being dismissed.
......
......@@ -322,6 +322,12 @@ NSString* const kSettingsDoneButtonId = @"kSettingsDoneButtonId";
}
- (void)closeSettings {
for (UIViewController* controller in [self viewControllers]) {
if ([controller conformsToProtocol:@protocol(SettingsControllerProtocol)]) {
[controller performSelector:@selector(reportDismissalUserAction)];
}
}
[self.settingsNavigationDelegate closeSettings];
}
......
......@@ -9,6 +9,7 @@
#include <memory>
#include "base/bind.h"
#include "base/test/metrics/user_action_tester.h"
#include "components/search_engines/template_url_service.h"
#include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
#include "ios/chrome/browser/browser_state/test_chrome_browser_state_manager.h"
......@@ -152,6 +153,7 @@ TEST_F(SettingsNavigationControllerTest,
// delegate.
TEST_F(SettingsNavigationControllerTest,
CloseSettingsWhenNavigationStackSizeIsOne) {
base::UserActionTester user_action_tester;
@autoreleasepool {
SettingsNavigationController* settingsController =
[SettingsNavigationController
......@@ -159,7 +161,9 @@ TEST_F(SettingsNavigationControllerTest,
delegate:mockDelegate_];
EXPECT_EQ(1U, [[settingsController viewControllers] count]);
[[mockDelegate_ expect] closeSettings];
ASSERT_EQ(0, user_action_tester.GetActionCount("MobileSettingsClose"));
[settingsController popViewControllerOrCloseSettingsAnimated:NO];
EXPECT_EQ(1, user_action_tester.GetActionCount("MobileSettingsClose"));
EXPECT_OCMOCK_VERIFY(mockDelegate_);
[settingsController cleanUpSettings];
}
......
......@@ -8,6 +8,8 @@
#include "base/feature_list.h"
#import "base/mac/foundation_util.h"
#include "base/metrics/user_metrics.h"
#include "base/metrics/user_metrics_action.h"
#include "base/strings/sys_string_conversions.h"
#include "build/branding_buildflags.h"
#include "components/autofill/core/common/autofill_prefs.h"
......@@ -1081,6 +1083,10 @@ NSString* kDevViewSourceKey = @"DevViewSource";
#pragma mark SettingsControllerProtocol
- (void)reportDismissalUserAction {
base::RecordAction(base::UserMetricsAction("MobileSettingsClose"));
}
- (void)settingsWillBeDismissed {
DCHECK(!_settingsHasBeenDismissed);
[_googleServicesSettingsCoordinator stop];
......@@ -1289,4 +1295,11 @@ NSString* kDevViewSourceKey = @"DevViewSource";
[self signinStateDidChange];
}
#pragma mark - UIAdaptivePresentationControllerDelegate
- (void)presentationControllerDidDismiss:
(UIPresentationController*)presentationController {
base::RecordAction(base::UserMetricsAction("IOSSettingsCloseWithSwipe"));
}
@end
......@@ -8,6 +8,8 @@
#include "base/i18n/time_formatting.h"
#include "base/mac/foundation_util.h"
#include "base/metrics/user_metrics.h"
#include "base/metrics/user_metrics_action.h"
#include "base/strings/sys_string_conversions.h"
#include "components/google/core/common/google_util.h"
#import "components/signin/public/identity_manager/objc/identity_manager_observer_bridge.h"
......@@ -506,8 +508,21 @@ const CGFloat kSpinnerButtonPadding = 18;
#pragma mark - SettingsControllerProtocol callbacks
- (void)reportDismissalUserAction {
// Sync Passphrase Settings screen does not have Done button.
NOTREACHED();
}
- (void)settingsWillBeDismissed {
[self stopObserving];
}
#pragma mark - UIAdaptivePresentationControllerDelegate
- (void)presentationControllerDidDismiss:
(UIPresentationController*)presentationController {
base::RecordAction(base::UserMetricsAction(
"IOSSyncEncryptionPassphraseSettingsCloseWithSwipe"));
}
@end
......@@ -8344,6 +8344,15 @@ should be able to be added at any place in this file.
<description>Please enter the description of this user action.</description>
</action>
<action name="IOSAccountsSettingsCloseWithSwipe">
<owner>msarda@chromium.org</owner>
<owner>jlebel@chromium.org</owner>
<description>
Reported when Accounts Settings UI was dismissed was dismissed using swipe
down gesture. iOS only.
</description>
</action>
<action name="IOSBookmarkManagerCloseWithSwipe">
<owner>eugenebut@chromium.com</owner>
<owner>sczs@chromium.org</owner>
......@@ -8352,6 +8361,25 @@ should be able to be added at any place in this file.
</description>
</action>
<action name="IOSClearBrowsingDataCloseWithSwipe">
<owner>edchin@chromium.org</owner>
<owner>gambard@chromium.org</owner>
<owner>sczs@chromium.org</owner>
<description>
Reported when Clear Browsing Data Settings UI was dismissed using swipe down
gesture. iOS only.
</description>
</action>
<action name="IOSGoogleServicesSettingsCloseWithSwipe">
<owner>msarda@chromium.org</owner>
<owner>jlebel@chromium.org</owner>
<description>
Reported when Google Services Settings UI was dismissed using swipe down
gesture. iOS only.
</description>
</action>
<action name="IOSHistoryCloseWithSwipe">
<owner>eugenebut@google.com</owner>
<owner>sczs@chromium.org</owner>
......@@ -8368,6 +8396,15 @@ should be able to be added at any place in this file.
</description>
</action>
<action name="IOSLanguagesSettingsCloseWithSwipe">
<owner>eugenebut@chromium.org</owner>
<owner>pkl@chromium.org</owner>
<description>
Reported when Languages Settings UI was dismissed using swipe down gesture.
iOS only.
</description>
</action>
<action name="IOSLaunchedByOpenInChromeIntent">
<owner>justincohen@chromium.org</owner>
<owner>rohitrao@chromium.org</owner>
......@@ -8381,6 +8418,15 @@ should be able to be added at any place in this file.
</description>
</action>
<action name="IOSPasswordsSettingsCloseWithSwipe">
<owner>djean@chromium.org</owner>
<owner>javierrobles@chromium.com</owner>
<description>
Reported when Passwords Settings UI was dismissed using swipe down gesture.
iOS only.
</description>
</action>
<action name="IOSReadingListCloseWithSwipe">
<owner>eugenebut@google.com</owner>
<owner>olivierrobin@chromium.org</owner>
......@@ -8397,6 +8443,24 @@ should be able to be added at any place in this file.
</description>
</action>
<action name="IOSSettingsCloseWithSwipe">
<owner>edchin@chromium.org</owner>
<owner>gambard@chromium.org</owner>
<owner>sczs@chromium.org</owner>
<description>
Reported when Settings UI was dismissed using swipe down gesture. iOS only.
</description>
</action>
<action name="IOSSyncEncryptionPassphraseSettingsCloseWithSwipe">
<owner>msarda@chromium.org</owner>
<owner>jlebel@chromium.org</owner>
<description>
Reported when Sync Encryption Passphrase Settings UI was dismissed using
swipe down gesture. iOS only.
</description>
</action>
<action name="Italic">
<owner>Please list the metric's owners. Add more owner tags as needed.</owner>
<description>Please enter the description of this user action.</description>
......@@ -11853,6 +11917,14 @@ should be able to be added at any place in this file.
</description>
</action>
<action name="MobileAccountsSettingsClose">
<owner>msarda@chromium.org</owner>
<owner>jlebel@chromium.org</owner>
<description>
Reported when Accounts Settings UI was dismissed. iOS only.
</description>
</action>
<action name="MobileActionBarShown">
<owner>donnd@chromium.org</owner>
<description>
......@@ -12180,6 +12252,15 @@ should be able to be added at any place in this file.
<description>Please enter the description of this user action.</description>
</action>
<action name="MobileClearBrowsingDataClose">
<owner>edchin@chromium.org</owner>
<owner>gambard@chromium.org</owner>
<owner>sczs@chromium.org</owner>
<description>
Reported when Clear Browsing Data Settings UI was dismissed. iOS only.
</description>
</action>
<action name="MobileClearBrowsingDataTriggeredFromLegacyUI">
<owner>sczs@chromium.org</owner>
<description>
......@@ -12549,6 +12630,14 @@ should be able to be added at any place in this file.
</description>
</action>
<action name="MobileGoogleServicesSettingsClose">
<owner>msarda@chromium.org</owner>
<owner>jlebel@chromium.org</owner>
<description>
Reported when Google Services Settings UI was dismissed. iOS only.
</description>
</action>
<action name="MobileGoToBackground">
<owner>tedchoc@chromium.org</owner>
<description>
......@@ -13333,6 +13422,14 @@ should be able to be added at any place in this file.
<description>Please enter the description of this user action.</description>
</action>
<action name="MobilePasswordsSettingsClose">
<owner>djean@chromium.org</owner>
<owner>javierrobles@chromium.com</owner>
<description>
Reported when Passwords Settings UI was dismissed. iOS only.
</description>
</action>
<action name="MobilePopupMenuSwipeToSelect">
<owner>gambard@chromium.org</owner>
<description>
......@@ -13553,6 +13650,13 @@ should be able to be added at any place in this file.
</description>
</action>
<action name="MobileSettingsClose">
<owner>edchin@chromium.org</owner>
<owner>gambard@chromium.org</owner>
<owner>sczs@chromium.org</owner>
<description>Reported when Settings UI was dismissed. iOS only.</description>
</action>
<action name="MobileSettingsStorageClearAll">
<owner>dmurph@chromium.org</owner>
<description>
......
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