Add Startup.BrowserView.FirstPaint / .CompositingEnded histograms.

chrome/browser/ui/cocoa/tabs/tab_window_controller.mm

Index: chrome/browser/ui/cocoa/tabs/tab_window_controller.mm
diff --git a/chrome/browser/ui/cocoa/tabs/tab_window_controller.mm b/chrome/browser/ui/cocoa/tabs/tab_window_controller.mm
index 716cb70c975cf3dc0740bb3958365f211f1b17d3..b81b80f40565f32ff0be9e093de162652b86bcf0 100644
--- a/chrome/browser/ui/cocoa/tabs/tab_window_controller.mm
+++ b/chrome/browser/ui/cocoa/tabs/tab_window_controller.mm
@@ -6,6 +6,8 @@
 
 #include "base/logging.h"
 #import "base/mac/sdk_forward_declarations.h"
+#include "base/memory/ptr_util.h"
+#include "chrome/browser/ui/browser_view_histogram_helper.h"
 #import "chrome/browser/ui/cocoa/browser_window_layout.h"
 #import "chrome/browser/ui/cocoa/fast_resize_view.h"
 #import "chrome/browser/ui/cocoa/framed_browser_window.h"
@@ -64,13 +66,23 @@
 @end
 
 // Subview of the window's contentView, contains everything but the tab strip.
-@interface ChromeContentView : NSView
+@interface ChromeContentView : NSView {
+ @private
+  std::unique_ptr<BrowserViewHistogramHelper> histogram_helper_;

[tapted, 2017/04/07 00:49:34]

histogramHelper_  (since this is declared between @foo/@end). Also does this
need to be unique_ptr? (i.e. why not just `BrowserViewHistogramHelper
histogramHelper_;`). Then you don't need an initializer.

[themblsha, 2017/04/10 17:28:18]

I'm using unique_ptr in browser_view.h so I can forward-declare the class
without adding it as a dependency for all the units. Here however it's a good
concern :)

Decided to use it without an instance variable in BrowserWindowCocoa::Show().

+}
 @end
 
 @implementation ChromeContentView
 
 // NSView overrides.
 
+- (instancetype)initWithFrame:(NSRect)frameRect {
+  if ((self = [super initWithFrame:frameRect])) {
+    histogram_helper_ = base::MakeUnique<BrowserViewHistogramHelper>();
+  }
+  return self;
+}
+
 // Since Auto Layout and frame-based layout behave differently in small but
 // important ways (e.g. Auto Layout can restrict window resizing, frame-based
 // layout doesn't log a warning when a view's autoresizing mask can't be
@@ -79,6 +91,13 @@
   return YES;
 }
 
+- (void)drawRect:(NSRect)dirtyRect {
+  // -drawRect: is called from -drawLayer:inContext:, which is called before all
+  // ChromeContentView's got a chance to paint. But there seems to be no
+  // non-hacky way of getting that notification.

[tapted, 2017/04/07 00:49:34]

Does void BrowserWindowCocoa::Show() already record the value you want for
Cocoa?

Cocoa drawing is typically quite happy just to block the UI thread until
essential drawing is done. In BrowserWindowCocoa::Show() we make the assumption
that all drawing occurs under that `[window()
makeKeyAndOrderFront:controller_];` call. When it returns, drawing is "done".

(in MacViews we can probably cheat a little and fire off a compositing task
before calling makeKeyAndOrderFront: so drawing is parallelised with the
WindowServer IPC)

I'd need to do some more research to see if there's an alternative hook that
captures when drawing is finished versus when the window is mapped to the
screen, but it's probably not worth collecting UMA separately since the user
needs both to happen. (we may still want to capture it for analysis though).

[themblsha, 2017/04/10 17:28:18]

Thanks for the tip! Moved the code there.

The BrowserView solution with ui::Compositor already compiles and works on
MacViews. I've traced the BrowserView::Show() (around the frame_->Show(), which
calls makeKeyAndOrderFront: inside), and frame_->Show() call by itself took 56ms
on Debug build on my i7, Startup.BrowserView.FirstPaint was reported 420ms
later, and compositing took 164ms according to my new histograms. 

+  histogram_helper_->OnDidPaintChildren(nullptr);
+}
+
 @end
 
 @implementation TabWindowController