Commit 18d5a836 authored by Gauthier Ambard's avatar Gauthier Ambard Committed by Commit Bot

Add metric for the opening of popup menu

This CL adds a metrics for the time elapsed between the time the user
taps on a button and the popup menu is presented.

Bug: 829343
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Id978bd57ffa93adc0827a7810ca546c8272aded7
Reviewed-on: https://chromium-review.googlesource.com/999476
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: default avatarMark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548763}
parent 2896ec3f
...@@ -271,6 +271,10 @@ class UnopenedDownloadsTracker : public web::DownloadTaskObserver, ...@@ -271,6 +271,10 @@ class UnopenedDownloadsTracker : public web::DownloadTaskObserver,
#pragma mark - ContainedPresenterDelegate #pragma mark - ContainedPresenterDelegate
- (void)containedPresenterDidPresent:(id<ContainedPresenter>)presenter {
DCHECK(presenter == self.presenter);
}
- (void)containedPresenterDidDismiss:(id<ContainedPresenter>)presenter { - (void)containedPresenterDidDismiss:(id<ContainedPresenter>)presenter {
DCHECK(presenter == self.presenter); DCHECK(presenter == self.presenter);
} }
......
...@@ -24,6 +24,7 @@ source_set("popup_menu") { ...@@ -24,6 +24,7 @@ source_set("popup_menu") {
"//ios/chrome/browser/ui/commands", "//ios/chrome/browser/ui/commands",
"//ios/chrome/browser/ui/coordinators:chrome_coordinators", "//ios/chrome/browser/ui/coordinators:chrome_coordinators",
"//ios/chrome/browser/ui/popup_menu/cells", "//ios/chrome/browser/ui/popup_menu/cells",
"//ios/chrome/browser/ui/presenters",
"//ios/chrome/browser/ui/reading_list", "//ios/chrome/browser/ui/reading_list",
"//ios/chrome/browser/ui/tools_menu/public", "//ios/chrome/browser/ui/tools_menu/public",
"//ios/chrome/browser/ui/util", "//ios/chrome/browser/ui/util",
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#import "ios/chrome/browser/ui/popup_menu/popup_menu_coordinator.h" #import "ios/chrome/browser/ui/popup_menu/popup_menu_coordinator.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/metrics/histogram_macros.h"
#include "base/metrics/user_metrics.h" #include "base/metrics/user_metrics.h"
#include "base/metrics/user_metrics_action.h" #include "base/metrics/user_metrics_action.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h" #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
...@@ -16,6 +17,7 @@ ...@@ -16,6 +17,7 @@
#import "ios/chrome/browser/ui/popup_menu/popup_menu_mediator.h" #import "ios/chrome/browser/ui/popup_menu/popup_menu_mediator.h"
#import "ios/chrome/browser/ui/popup_menu/popup_menu_presenter.h" #import "ios/chrome/browser/ui/popup_menu/popup_menu_presenter.h"
#import "ios/chrome/browser/ui/popup_menu/popup_menu_table_view_controller.h" #import "ios/chrome/browser/ui/popup_menu/popup_menu_table_view_controller.h"
#import "ios/chrome/browser/ui/presenters/contained_presenter_delegate.h"
#import "ios/chrome/browser/ui/util/layout_guide_names.h" #import "ios/chrome/browser/ui/util/layout_guide_names.h"
#if !defined(__has_feature) || !__has_feature(objc_arc) #if !defined(__has_feature) || !__has_feature(objc_arc)
...@@ -31,12 +33,15 @@ PopupMenuCommandType CommandTypeFromPopupType(PopupMenuType type) { ...@@ -31,12 +33,15 @@ PopupMenuCommandType CommandTypeFromPopupType(PopupMenuType type) {
} }
} // namespace } // namespace
@interface PopupMenuCoordinator ()<PopupMenuCommands> @interface PopupMenuCoordinator ()<ContainedPresenterDelegate,
PopupMenuCommands>
// Presenter for the popup menu, managing the animations. // Presenter for the popup menu, managing the animations.
@property(nonatomic, strong) PopupMenuPresenter* presenter; @property(nonatomic, strong) PopupMenuPresenter* presenter;
// Mediator for the popup menu. // Mediator for the popup menu.
@property(nonatomic, strong) PopupMenuMediator* mediator; @property(nonatomic, strong) PopupMenuMediator* mediator;
// Time when the presentation of the popup menu is requested.
@property(nonatomic, assign) NSTimeInterval requestStartTime;
@end @end
...@@ -45,6 +50,7 @@ PopupMenuCommandType CommandTypeFromPopupType(PopupMenuType type) { ...@@ -45,6 +50,7 @@ PopupMenuCommandType CommandTypeFromPopupType(PopupMenuType type) {
@synthesize dispatcher = _dispatcher; @synthesize dispatcher = _dispatcher;
@synthesize mediator = _mediator; @synthesize mediator = _mediator;
@synthesize presenter = _presenter; @synthesize presenter = _presenter;
@synthesize requestStartTime = _requestStartTime;
@synthesize webStateList = _webStateList; @synthesize webStateList = _webStateList;
#pragma mark - ChromeCoordinator #pragma mark - ChromeCoordinator
...@@ -111,6 +117,24 @@ PopupMenuCommandType CommandTypeFromPopupType(PopupMenuType type) { ...@@ -111,6 +117,24 @@ PopupMenuCommandType CommandTypeFromPopupType(PopupMenuType type) {
self.mediator = nil; self.mediator = nil;
} }
#pragma mark - ContainedPresenterDelegate
- (void)containedPresenterDidPresent:(id<ContainedPresenter>)presenter {
DCHECK(presenter == self.presenter);
if (self.requestStartTime != 0) {
base::TimeDelta elapsed = base::TimeDelta::FromSecondsD(
[NSDate timeIntervalSinceReferenceDate] - self.requestStartTime);
UMA_HISTOGRAM_TIMES("Toolbar.ShowToolsMenuResponsiveness", elapsed);
// Reset the start time to ensure that whatever happens, we only record
// this once.
self.requestStartTime = 0;
}
}
- (void)containedPresenterDidDismiss:(id<ContainedPresenter>)presenter {
DCHECK(presenter == self.presenter);
}
#pragma mark - Notification callback #pragma mark - Notification callback
- (void)applicationDidEnterBackground:(NSNotification*)note { - (void)applicationDidEnterBackground:(NSNotification*)note {
...@@ -129,6 +153,8 @@ PopupMenuCommandType CommandTypeFromPopupType(PopupMenuType type) { ...@@ -129,6 +153,8 @@ PopupMenuCommandType CommandTypeFromPopupType(PopupMenuType type) {
[callableDispatcher [callableDispatcher
prepareForPopupMenuPresentation:CommandTypeFromPopupType(type)]; prepareForPopupMenuPresentation:CommandTypeFromPopupType(type)];
self.requestStartTime = [NSDate timeIntervalSinceReferenceDate];
PopupMenuTableViewController* tableViewController = PopupMenuTableViewController* tableViewController =
[[PopupMenuTableViewController alloc] [[PopupMenuTableViewController alloc]
initWithStyle:UITableViewStyleGrouped]; initWithStyle:UITableViewStyleGrouped];
...@@ -152,6 +178,7 @@ PopupMenuCommandType CommandTypeFromPopupType(PopupMenuType type) { ...@@ -152,6 +178,7 @@ PopupMenuCommandType CommandTypeFromPopupType(PopupMenuType type) {
self.presenter.commandHandler = self; self.presenter.commandHandler = self;
self.presenter.presentedViewController = tableViewController; self.presenter.presentedViewController = tableViewController;
self.presenter.guideName = guideName; self.presenter.guideName = guideName;
self.presenter.delegate = self;
[self.presenter prepareForPresentation]; [self.presenter prepareForPresentation];
[self.presenter presentAnimated:YES]; [self.presenter presentAnimated:YES];
......
...@@ -133,7 +133,9 @@ const CGFloat kDamping = 0.85; ...@@ -133,7 +133,9 @@ const CGFloat kDamping = 0.85;
self.popupViewController.contentContainer.transform = self.popupViewController.contentContainer.transform =
CGAffineTransformIdentity; CGAffineTransformIdentity;
} }
withCompletion:nil]; withCompletion:^(BOOL finished) {
[self.delegate containedPresenterDidPresent:self];
}];
} }
- (void)dismissAnimated:(BOOL)animated { - (void)dismissAnimated:(BOOL)animated {
......
...@@ -13,6 +13,8 @@ ...@@ -13,6 +13,8 @@
// and which is informed about dismissal events. // and which is informed about dismissal events.
@protocol ContainedPresenterDelegate @protocol ContainedPresenterDelegate
- (void)containedPresenterDidPresent:(id<ContainedPresenter>)presenter;
// Tells the delegate that |presenter| has finished dismissing. // Tells the delegate that |presenter| has finished dismissing.
- (void)containedPresenterDidDismiss:(id<ContainedPresenter>)presenter; - (void)containedPresenterDidDismiss:(id<ContainedPresenter>)presenter;
......
...@@ -81,14 +81,23 @@ NSTimeInterval kAnimationDuration = 0.2; ...@@ -81,14 +81,23 @@ NSTimeInterval kAnimationDuration = 0.2;
if (self.presentedConstraints[0].active) if (self.presentedConstraints[0].active)
return; return;
[UIView animateWithDuration:animated ? kAnimationDuration : 0.0 auto animations = ^{
animations:^{ [NSLayoutConstraint deactivateConstraints:self.dismissedConstraints];
[NSLayoutConstraint [NSLayoutConstraint activateConstraints:self.presentedConstraints];
deactivateConstraints:self.dismissedConstraints]; [self.baseViewController.view layoutIfNeeded];
[NSLayoutConstraint };
activateConstraints:self.presentedConstraints]; auto completion = ^(BOOL finished) {
[self.baseViewController.view layoutIfNeeded]; [self.delegate containedPresenterDidPresent:self];
}]; };
if (animated) {
[UIView animateWithDuration:kAnimationDuration
animations:animations
completion:completion];
} else {
animations();
completion(YES);
}
} }
- (void)dismissAnimated:(BOOL)animated { - (void)dismissAnimated:(BOOL)animated {
...@@ -100,12 +109,12 @@ NSTimeInterval kAnimationDuration = 0.2; ...@@ -100,12 +109,12 @@ NSTimeInterval kAnimationDuration = 0.2;
if (self.dismissedConstraints[0].active) if (self.dismissedConstraints[0].active)
return; return;
void (^animations)() = ^{ auto animations = ^{
[NSLayoutConstraint deactivateConstraints:self.presentedConstraints]; [NSLayoutConstraint deactivateConstraints:self.presentedConstraints];
[NSLayoutConstraint activateConstraints:self.dismissedConstraints]; [NSLayoutConstraint activateConstraints:self.dismissedConstraints];
[self.baseViewController.view layoutIfNeeded]; [self.baseViewController.view layoutIfNeeded];
}; };
void (^completion)(BOOL) = ^(BOOL finished) { auto completion = ^(BOOL finished) {
[self cleanUpAfterDismissal]; [self cleanUpAfterDismissal];
[self.delegate containedPresenterDidDismiss:self]; [self.delegate containedPresenterDidDismiss:self];
}; };
......
...@@ -13,14 +13,21 @@ ...@@ -13,14 +13,21 @@
#error "This file requires ARC support." #error "This file requires ARC support."
#endif #endif
// Test delegate helper; the delegate callback sets the |dismissed| property. // Test delegate helper; the delegate callback sets the |presented| and
// |dismissed| property.
@interface TestContainedPresenterDelegate : NSObject<ContainedPresenterDelegate> @interface TestContainedPresenterDelegate : NSObject<ContainedPresenterDelegate>
@property(nonatomic) BOOL presented;
@property(nonatomic) BOOL dismissed; @property(nonatomic) BOOL dismissed;
@end @end
@implementation TestContainedPresenterDelegate @implementation TestContainedPresenterDelegate
@synthesize presented = _presented;
@synthesize dismissed = _dismissed; @synthesize dismissed = _dismissed;
- (void)containedPresenterDidPresent:(id<ContainedPresenter>)presenter {
self.presented = YES;
}
- (void)containedPresenterDidDismiss:(id<ContainedPresenter>)presenter { - (void)containedPresenterDidDismiss:(id<ContainedPresenter>)presenter {
self.dismissed = YES; self.dismissed = YES;
} }
...@@ -65,6 +72,9 @@ TEST_F(VerticalAnimationContainerTest, TestPreparation) { ...@@ -65,6 +72,9 @@ TEST_F(VerticalAnimationContainerTest, TestPreparation) {
EXPECT_EQ(base_view_width, CGRectGetWidth(presented_.view.bounds)); EXPECT_EQ(base_view_width, CGRectGetWidth(presented_.view.bounds));
EXPECT_EQ(presented_.view.frame.origin.x, 0); EXPECT_EQ(presented_.view.frame.origin.x, 0);
EXPECT_GE(presented_.view.frame.origin.y, base_.view.bounds.size.height); EXPECT_GE(presented_.view.frame.origin.y, base_.view.bounds.size.height);
// The presentation did not finish yet.
EXPECT_FALSE(delegate_.presented);
} }
TEST_F(VerticalAnimationContainerTest, TestPresentation) { TEST_F(VerticalAnimationContainerTest, TestPresentation) {
...@@ -76,6 +86,7 @@ TEST_F(VerticalAnimationContainerTest, TestPresentation) { ...@@ -76,6 +86,7 @@ TEST_F(VerticalAnimationContainerTest, TestPresentation) {
EXPECT_TRUE(CGRectContainsRect(base_.view.bounds, presented_.view.frame)); EXPECT_TRUE(CGRectContainsRect(base_.view.bounds, presented_.view.frame));
EXPECT_EQ(CGRectGetMaxY(base_.view.bounds), EXPECT_EQ(CGRectGetMaxY(base_.view.bounds),
CGRectGetMaxY(presented_.view.frame)); CGRectGetMaxY(presented_.view.frame));
EXPECT_TRUE(delegate_.presented);
// The delegate method should not be called here. // The delegate method should not be called here.
EXPECT_FALSE(delegate_.dismissed); EXPECT_FALSE(delegate_.dismissed);
} }
......
...@@ -125,6 +125,10 @@ ...@@ -125,6 +125,10 @@
#pragma mark - ContainedPresenterDelegate #pragma mark - ContainedPresenterDelegate
- (void)containedPresenterDidPresent:(id<ContainedPresenter>)presenter {
DCHECK(presenter == self.presenter);
}
- (void)containedPresenterDidDismiss:(id<ContainedPresenter>)presenter { - (void)containedPresenterDidDismiss:(id<ContainedPresenter>)presenter {
DCHECK(presenter == self.presenter); DCHECK(presenter == self.presenter);
[self stop]; [self stop];
......
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