[Mac] Avoid "adding unknown subview" warning.

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

 // Copyright (c) 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 "chrome/browser/ui/cocoa/tabs/tab_window_controller.h"
 
 #include "base/logging.h"
 #import "base/mac/sdk_forward_declarations.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"
 #import "chrome/browser/ui/cocoa/tabs/tab_strip_background_view.h"
 #import "chrome/browser/ui/cocoa/tabs/tab_strip_view.h"
 #import "chrome/browser/ui/cocoa/themed_window.h"
 #import "ui/base/cocoa/focus_tracker.h"
 #include "ui/base/material_design/material_design_controller.h"
 #include "ui/base/theme_provider.h"
 
 @interface TabWindowController ()
 - (void)setUseOverlay:(BOOL)useOverlay;
 
 // The tab strip background view should always be inserted as the back-most
 // subview of the root view. It cannot be a subview of the contentView, as that
 // would cause it to become layer backed, which would cause it to draw on top
 // of non-layer backed content like the window controls.
 - (void)insertTabStripBackgroundViewIntoWindow:(NSWindow*)window
                                       titleBar:(BOOL)hasTitleBar;
+
+// Called when NSWindowWillEnterFullScreenNotification notification received.
+// Makes visual effects view hidden as it should not be displayed in fullscreen.
+- (void)windowWillEnterFullScreenNotification:(NSNotification*)notification;

[tapted, 2016/10/10 23:38:06]

nit: windowWillEnterFullScreen: (consistent with BrowserWindowController)

[Eugene But (OOO till 7-30), 2016/10/11 00:33:10]

|windowWillEnterFullScreen:| does not follow Coding Guidelines for Cocoa which
say "Make the word before the argument describe the argument.":
https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/Co...

Coding Guidelines for Cocoa is the basis for Google Objective-C Style Guide and
following style guide naming seems to be more important than being consistent
with another file (which does not follow naming rules strictly). WDYT?

+// Called when NSWindowWillExitFullScreenNotification notification received.

[tapted, 2016/10/10 23:38:06]

nit: blank line before [consistency]

[Eugene But (OOO till 7-30), 2016/10/11 00:33:10]

Done.

+// Makes visual effects view visible since it was hidden in fullscreen.
+- (void)windowWillExitFullScreenNotification:(NSNotification*)notification;
+
 @end
 
 @interface TabWindowOverlayWindow : NSWindow
 @end
 
 @implementation TabWindowOverlayWindow
 
 - (const ui::ThemeProvider*)themeProvider {
   return [[self parentWindow] themeProvider];
 }
@@ skipping 44 matching lines @@
         [[FastResizeView alloc] initWithFrame:[chromeContentView_ bounds]]);
     [tabContentArea_ setAutoresizingMask:NSViewWidthSizable |
                                          NSViewHeightSizable];
     [chromeContentView_ addSubview:tabContentArea_];
 
     // tabStripBackgroundView_ draws the theme image behind the tab strip area.
     // When making a tab dragging window (setUseOverlay:), this view stays in
     // the parent window so that it can be translucent, while the tab strip view
     // moves to the child window and stays opaque.
     NSView* windowView = [window contentView];
+    CGFloat paintHeight = [FramedBrowserWindow browserFrameViewPaintHeight];
     tabStripBackgroundView_.reset([[TabStripBackgroundView alloc]
-        initWithFrame:NSMakeRect(0,
-                                 NSMaxY([windowView bounds]) -
-                                     kBrowserFrameViewPaintHeight,
-                                 NSWidth([windowView bounds]),
-                                 kBrowserFrameViewPaintHeight)]);
+        initWithFrame:NSMakeRect(0, NSMaxY([windowView bounds]) - paintHeight,
+                                 NSWidth([windowView bounds]), paintHeight)]);
     [tabStripBackgroundView_
         setAutoresizingMask:NSViewWidthSizable | NSViewMinYMargin];
     [self insertTabStripBackgroundViewIntoWindow:window titleBar:hasTitleBar];
 
     tabStripView_.reset([[TabStripView alloc]
         initWithFrame:NSMakeRect(
                           0, 0, kDefaultWidth, chrome::kTabStripHeight)]);
     [tabStripView_ setAutoresizingMask:NSViewWidthSizable |
                                        NSViewMinYMargin];
     if (hasTabStrip)
       [windowView addSubview:tabStripView_];
+
+    if (chrome::ShouldUseFullSizeContentView()) {
+      [[NSNotificationCenter defaultCenter]
+          addObserver:self
+             selector:@selector(windowWillEnterFullScreenNotification:)
+                 name:NSWindowWillEnterFullScreenNotification
+               object:window];
+      [[NSNotificationCenter defaultCenter]
+          addObserver:self
+             selector:@selector(windowWillExitFullScreenNotification:)
+                 name:NSWindowWillExitFullScreenNotification
+               object:window];
+    }
   }
   return self;
 }
 
+- (void)dealloc {
+  [[NSNotificationCenter defaultCenter] removeObserver:self];
+  [super dealloc];
+}
+
 - (NSView*)tabStripBackgroundView {
   return tabStripBackgroundView_;
 }
 
 - (TabStripView*)tabStripView {
   return tabStripView_;
 }
 
 - (FastResizeView*)tabContentArea {
   return tabContentArea_;
@@ skipping 230 matching lines @@
 
 - (void)insertTabStripBackgroundViewIntoWindow:(NSWindow*)window
                                       titleBar:(BOOL)hasTitleBar {
   DCHECK(tabStripBackgroundView_);
   NSView* rootView = [[window contentView] superview];
 
   // In Material Design on 10.10 and higher, the top portion of the window is
   // blurred using an NSVisualEffectView.
   Class nsVisualEffectViewClass = NSClassFromString(@"NSVisualEffectView");
   if (!nsVisualEffectViewClass) {
     [rootView addSubview:tabStripBackgroundView_

[tapted, 2016/10/10 23:38:06]

before this, perhaps
DCHECK(!chrome::ShouldUseFullSizeContentView())

[Eugene But (OOO till 7-30), 2016/10/11 00:33:10]

Are you sure? Availability of NSVisualEffectView and using it for toolbar effect
seems to be unrelated to view hierarchy fix. Am I missing something? 

[tapted, 2016/10/11 04:21:43]

The problem here is that |rootView| is [contentView superview], which we
shouldn't be calling addSubview on. (ideally we'd move the rootView declaration
into the `else` block below of `if (chrome::ShouldUseFullSizeContentView())` but
it gets used here as well - so DCHECK feels like a nice compromise :)

[Eugene But (OOO till 7-30), 2016/10/11 04:35:46]

Oh, right. Done.

               positioned:NSWindowBelow
               relativeTo:nil];
     return;
   }
 
   [window setTitlebarAppearsTransparent:YES];
 
   // If the window has a normal titlebar, then do not add NSVisualEffectView.
   if (hasTitleBar)
     return;
@@ skipping 13 matching lines @@
   // the start of -[BrowserWindowController initWithBrowser:takeOwnership:],
   // before the |browser_| ivar has been set. Without a browser object we
   // can't check the window's theme. The final setup happens in
   // -[TabStripView setController:], at which point we have access to the theme.
   [visualEffectView setAppearance:
       [NSAppearance appearanceNamed:NSAppearanceNameVibrantLight]];
   [visualEffectView setMaterial:NSVisualEffectMaterialLight];
   [visualEffectView setBlendingMode:NSVisualEffectBlendingModeBehindWindow];
   [visualEffectView setState:NSVisualEffectStateFollowsWindowActiveState];
 
-  [rootView addSubview:visualEffectView
-            positioned:NSWindowBelow
-            relativeTo:nil];
+  if (chrome::ShouldUseFullSizeContentView()) {
+    [[window contentView] addSubview:visualEffectView];
+  } else {
+    [rootView addSubview:visualEffectView
+              positioned:NSWindowBelow
+              relativeTo:nil];
+  }
 
   // Make the |tabStripBackgroundView_| a child of the NSVisualEffectView.
   [tabStripBackgroundView_ setFrame:[visualEffectView bounds]];
   [visualEffectView addSubview:tabStripBackgroundView_];
+  visualEffectView_.reset([visualEffectView retain]);

[tapted, 2016/10/10 23:38:06]

since visualEffectView is also scoped_nsobject, we should just be able to assign
here. Although I'd probably just go with visualEffectView_.reset([[foo alloc]
initWith..]) up on line 399, and add an underscore to the stuff above

[Eugene But (OOO till 7-30), 2016/10/11 00:33:10]

Done.

 }
 
 // Called when the size of the window content area has changed. Override to
 // position specific views. Base class implementation does nothing.
 - (void)layoutSubviews {
   NOTIMPLEMENTED();
 }
 
+- (void)windowWillEnterFullScreenNotification:(NSNotification*)notification {
+  [[visualEffectView_ animator] setAlphaValue:0.0];

[tapted, 2016/10/10 23:38:06]

I wonder if there are implications for text rendering / supbixel AA if we change
the opacity of this. E.g. do tab titles and the profile selector still look
right. (although at a glance, the profile selector is doing something to force
subpixel AA and it's getting the backgroudn colour wrong anyway)

[Eugene But (OOO till 7-30), 2016/10/11 00:33:10]

Visually looks the same before and after the changes.

+}
+
+- (void)windowWillExitFullScreenNotification:(NSNotification*)notification {
+  [[visualEffectView_ animator] setAlphaValue:1.0];

[tapted, 2016/10/10 23:38:06]

Maybe worth a note for these - visualEffectView_ will be nil for popups (i.e.
when hasTitleBar is false in insertTabStripBackgroundViewIntoWindow:) - the code
works, but it could be a trap for the unwary later

[Eugene But (OOO till 7-30), 2016/10/11 00:33:10]

Done. Added comments to the header.

+}
+
 @end