Perf metric tracking the wallclock time it takes to show a new tab page.

ios/chrome/browser/ui/browser_view_controller.mm

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "ios/chrome/browser/ui/browser_view_controller.h"
 
 #import <AssetsLibrary/AssetsLibrary.h>
 #import <MobileCoreServices/MobileCoreServices.h>
 #import <PassKit/PassKit.h>
 #import <Photos/Photos.h>
@@ skipping 554 matching lines @@
 @property(nonatomic, assign, getter=isInNewTabAnimation) BOOL inNewTabAnimation;
 // Whether BVC prefers to hide the status bar. This value is used to determine
 // the response from the |prefersStatusBarHidden| method.
 @property(nonatomic, assign) BOOL hideStatusBar;
 // Whether the VoiceSearchBar should be displayed.
 @property(nonatomic, readonly) BOOL shouldShowVoiceSearchBar;
 // Coordinator for displaying a modal overlay with activity indicator to prevent
 // the user from interacting with the browser view.
 @property(nonatomic, strong)
     ActivityOverlayCoordinator* activityOverlayCoordinator;
+// A block to be run when the |tabWasAdded:| method completes the animation
+// for the presentation of a new tab. Can be used to record performance metrics.
+@property(nonatomic, strong, nullable)
+    ProceduralBlock tabWasAddedCompletionBlock;
 
 // The user agent type used to load the currently visible page. User agent type
 // is NONE if there is no visible page or visible page is a native page.
 @property(nonatomic, assign, readonly) web::UserAgentType userAgentType;
 
 // Returns the header views, all the chrome on top of the page, including the
 // ones that cannot be scrolled off screen by full screen.
 @property(nonatomic, strong, readonly) NSArray<HeaderDefinition*>* headerViews;
 
 // BVC initialization:
@@ skipping 340 matching lines @@
 
 @synthesize contentArea = _contentArea;
 @synthesize typingShield = _typingShield;
 @synthesize active = _active;
 @synthesize visible = _visible;
 @synthesize viewVisible = _viewVisible;
 @synthesize dismissingModal = _dismissingModal;
 @synthesize hideStatusBar = _hideStatusBar;
 @synthesize activityOverlayCoordinator = _activityOverlayCoordinator;
 @synthesize presenting = _presenting;
+@synthesize tabWasAddedCompletionBlock = _tabWasAddedCompletionBlock;
 
 #pragma mark - Object lifecycle
 
 - (instancetype)initWithTabModel:(TabModel*)model
                     browserState:(ios::ChromeBrowserState*)browserState
                dependencyFactory:
                    (BrowserViewControllerDependencyFactory*)factory {
   self = [super initWithNibName:nil bundle:base::mac::FrameworkBundle()];
   if (self) {
     DCHECK(factory);
@@ skipping 246 matching lines @@
   [self setNeedsStatusBarAppearanceUpdate];
 }
 
 #pragma mark - IBActions
 
 - (void)shieldWasTapped:(id)sender {
   [_toolbarController cancelOmniboxEdit];
 }
 
 - (void)newTab:(id)sender {
+  // Observe the timing of the new tab creation, both MainController
+  // and BrowserViewController call into this method on the correct BVC to
+  // create new tabs making it preferable to doing this in
+  // |chromeExecuteCommand:|.
+  NSTimeInterval startTime = [NSDate timeIntervalSinceReferenceDate];
+  __weak __typeof(self) weakSelf = self;

[rohitrao (ping after 24h), 2017/05/22 12:55:05]

Could we capture the value of isOffTheRecord instead of capturing self?

[PL, 2017/05/23 22:37:55]

Absolutely! Done!

+  self.tabWasAddedCompletionBlock = ^{
+    UMA_HISTOGRAM_TIMES(
+        weakSelf.isOffTheRecord
+            ? "Toolbar.Menu.NewIncognitoTabPresentationDuration"
+            : "Toolbar.Menu.NewTabPresentationDuration",

[Ilya Sherman, 2017/05/22 20:45:45]

Names passed to UMA_HISTOGRAM macros must be runtime constants.  Please either
move the if-stmt out of the macro, or use the non-macro version,
base::UmaHistogramTimes().

[PL, 2017/05/23 22:37:55]

Ah thanks! I've broken these out into two separate calls now.

+        base::TimeDelta::FromSecondsD([NSDate timeIntervalSinceReferenceDate] -
+                                      startTime));
+  };
+
   [self setLastTapPoint:sender];
   DCHECK(self.visible || self.dismissingModal);
   Tab* currentTab = [_model currentTab];
   if (currentTab) {
     [currentTab updateSnapshotWithOverlay:YES visibleFrameOnly:YES];
   }
   [self addSelectedTabWithURL:GURL(kChromeUINewTabURL)
                    transition:ui::PAGE_TRANSITION_TYPED];
 }
 
@@ skipping 373 matching lines @@
   // Block that starts voice search at the end of new Tab animation if
   // necessary.
   ProceduralBlock startVoiceSearchIfNecessaryBlock = ^void() {
     if (_startVoiceSearchAfterNewTabAnimation) {
       _startVoiceSearchAfterNewTabAnimation = NO;
       [self startVoiceSearch];
     }
   };
 
   self.inNewTabAnimation = YES;
   if (!inBackground) {

[rohitrao (ping after 24h), 2017/05/22 12:55:05]

What happens when we open a new tab in the background?  Is it possible for
tabWasAddedCompletionBlock to be non-nil when a tab is opened in the background?
 Currently we only run the block for new foreground tabs.

[PL, 2017/05/23 22:37:55]

This is a good question. 

The answer is (I believe): nothing.

The newTab: method appears to only be able to create foreground tabs. So we
shouldn't be in a position where we set this block and then somehow get on
screen without running it. Also, if that did happen somehow, the block would
just be overwritten by the next call to newTab without it being run in any
erroneous state.

I do think you're right picking up on this issue though, it could be clearer
when this block will run, so I've renamed it, and just to be super safe, it will
now get reset whether the presented tab is foreground or  background.

Let me know what you think!

     UIView* animationParentView = _contentArea;
     // Create the new page image, and load with the new tab page snapshot.
     CGFloat newPageOffset = 0;
     UIImageView* newPage;
     if (tab.url == GURL(kChromeUINewTabURL) && !_isOffTheRecord &&
         !IsIPadIdiom()) {
       animationParentView = self.view;
       newPage = [self pageFullScreenOpenCloseAnimationView];
     } else {
       newPage = [self pageOpenCloseAnimationView];
@@ skipping 13 matching lines @@
           // Use the model's currentTab here because it is possible that it can
           // be reset to a new value before the new Tab animation finished (e.g.
           // if another Tab shows a dialog via |dialogPresenter|). However, that
           // tab's view hasn't been displayed yet because it was in a new tab
           // animation.
           Tab* currentTab = [_model currentTab];
           if (currentTab) {
             [self tabSelected:currentTab];
           }
           startVoiceSearchIfNecessaryBlock();
+
+          if (self.tabWasAddedCompletionBlock) {
+            self.tabWasAddedCompletionBlock();
+            self.tabWasAddedCompletionBlock = nil;
+          }
         });
   } else {
     // -updateSnapshotWithOverlay will force a screen redraw, so take the
     // snapshot before adding the views needed for the background animation.
     Tab* topTab = [_model currentTab];
     UIImage* image = [topTab updateSnapshotWithOverlay:YES
                                       visibleFrameOnly:self.isToolbarOnScreen];
     // Add three layers in order on top of the contentArea for the animation:
     // 1. The black "background" screen.
     UIView* background = [[UIView alloc] initWithFrame:[_contentArea bounds]];
@@ skipping 3531 matching lines @@
 
 - (UIView*)voiceSearchButton {
   return _voiceSearchButton;
 }
 
 - (id<LogoAnimationControllerOwner>)logoAnimationControllerOwner {
   return [self currentLogoAnimationControllerOwner];
 }
 
 @end