Mac: Clean up outdated use of NSThemeFrame

chrome/browser/ui/cocoa/framed_browser_window.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/framed_browser_window.h"
 
 #include "base/logging.h"
 #include "base/mac/sdk_forward_declarations.h"
 #include "chrome/browser/global_keyboard_shortcuts_mac.h"
 #include "chrome/browser/profiles/profile_avatar_icon_util.h"
@@ skipping 23 matching lines @@
 
 }
 
 @interface FramedBrowserWindow (Private)
 
 - (void)adjustCloseButton:(NSNotification*)notification;
 - (void)adjustMiniaturizeButton:(NSNotification*)notification;
 - (void)adjustZoomButton:(NSNotification*)notification;
 - (void)adjustButton:(NSButton*)button
               ofKind:(NSWindowButton)kind;
-- (NSView*)frameView;
 
 @end
 
 // Undocumented APIs. They are really on NSGrayFrame rather than NSView. Take
 // care to only call them on the NSView passed into
 // -[NSWindow drawCustomRect:forView:].
 @interface NSView (UndocumentedAPI)
 
 - (float)roundedCornerRadius;
 - (CGRect)_titlebarTitleRect;
@@ skipping 49 matching lines @@
                    name:NSViewFrameDidChangeNotification
                  object:closeButton_];
     [center addObserver:self
                selector:@selector(adjustMiniaturizeButton:)
                    name:NSViewFrameDidChangeNotification
                  object:miniaturizeButton_];
     [center addObserver:self
                selector:@selector(adjustZoomButton:)
                    name:NSViewFrameDidChangeNotification
                  object:zoomButton_];
-    [center addObserver:self
-               selector:@selector(themeDidChangeNotification:)
-                   name:kBrowserThemeDidChangeNotification
-                 object:nil];
   }
 
   return self;
 }
 
 - (void)dealloc {
   [[NSNotificationCenter defaultCenter] removeObserver:self];
   [super dealloc];
 }
 
 - (void)adjustCloseButton:(NSNotification*)notification {
   [self adjustButton:[notification object]
               ofKind:NSWindowCloseButton];
 }
 
 - (void)adjustMiniaturizeButton:(NSNotification*)notification {
   [self adjustButton:[notification object]
               ofKind:NSWindowMiniaturizeButton];
 }
 
 - (void)adjustZoomButton:(NSNotification*)notification {
   [self adjustButton:[notification object]
               ofKind:NSWindowZoomButton];
 }
 
 - (void)adjustButton:(NSButton*)button
               ofKind:(NSWindowButton)kind {
   NSRect buttonFrame = [button frame];
-  NSRect frameViewBounds = [[self frameView] bounds];
 
   CGFloat xOffset = hasTabStrip_
       ? kFramedWindowButtonsWithTabStripOffsetFromLeft
       : kFramedWindowButtonsWithoutTabStripOffsetFromLeft;
   CGFloat yOffset = hasTabStrip_
       ? kFramedWindowButtonsWithTabStripOffsetFromTop
       : kFramedWindowButtonsWithoutTabStripOffsetFromTop;
   buttonFrame.origin =
-      NSMakePoint(xOffset, (NSHeight(frameViewBounds) -
+      NSMakePoint(xOffset, (NSHeight([self frame]) -
                             NSHeight(buttonFrame) - yOffset));
 
   switch (kind) {
     case NSWindowZoomButton:
       buttonFrame.origin.x += NSWidth([miniaturizeButton_ frame]);
       buttonFrame.origin.x += windowButtonsInterButtonSpacing_;
       // fallthrough
     case NSWindowMiniaturizeButton:
       buttonFrame.origin.x += NSWidth([closeButton_ frame]);
       buttonFrame.origin.x += windowButtonsInterButtonSpacing_;
       // fallthrough
     default:
       break;
   }
 
   BOOL didPost = [button postsBoundsChangedNotifications];
   [button setPostsFrameChangedNotifications:NO];
   [button setFrame:buttonFrame];
   [button setPostsFrameChangedNotifications:didPost];
 }
 
-- (NSView*)frameView {
-  return [[self contentView] superview];
-}
-
 // The tab strip view covers our window buttons. So we add hit testing here
 // to find them properly and return them to the accessibility system.
 - (id)accessibilityHitTest:(NSPoint)point {
   NSPoint windowPoint = [self convertScreenToBase:point];
   NSControl* controls[] = { closeButton_, zoomButton_, miniaturizeButton_ };
   id value = nil;
   for (size_t i = 0; i < sizeof(controls) / sizeof(controls[0]); ++i) {
     if (NSPointInRect(windowPoint, [controls[i] frame])) {
       value = [controls[i] accessibilityHitTest:point];
       break;
     }
   }
   if (!value) {
     value = [super accessibilityHitTest:point];
   }
   return value;
 }
 
-- (void)windowMainStatusChanged {
-  NSView* frameView = [self frameView];
-  NSView* contentView = [self contentView];
-  NSRect updateRect = [frameView frame];
-  NSRect contentRect = [contentView frame];
-  CGFloat tabStripHeight = [TabStripController defaultTabHeight];
-  updateRect.size.height -= NSHeight(contentRect) - tabStripHeight;
-  updateRect.origin.y = NSMaxY(contentRect) - tabStripHeight;
-  [[self frameView] setNeedsDisplayInRect:updateRect];
-}
-
-- (void)becomeMainWindow {
-  [self windowMainStatusChanged];
-  [super becomeMainWindow];
-}
-
-- (void)resignMainWindow {
-  [self windowMainStatusChanged];
-  [super resignMainWindow];
-}
-
-// Called after the current theme has changed.
-- (void)themeDidChangeNotification:(NSNotification*)aNotification {
-  [[self frameView] setNeedsDisplay:YES];

[erikchen, 2015/02/04 23:53:55]

It seems like this class was a generic catch-all to redraw the entire window
when the theme changes. I haven't looked at all places where we do theme redraw,
but it looks like tab_strip_background_view is getting redrawn only because of
this code here?

[Andre, 2015/02/05 01:04:26]

This does not trigger redraw of the entire window, only NSThemeFrame.
TabStripBackgroundView currently gets dirtied by
-[BrowserWindowController userChangedTheme].
Just yesterday, I introduced ThemedWindowDrawing and make
TabStripBackgroundView adopt it, see
-[TabStripBackgroundView windowDidChangeTheme].
Eventually, I hope to delete the setNeedsDisplay call in
-userChangedTheme.

-}
-
-- (void)sendEvent:(NSEvent*)event {
-  // For Cocoa windows, clicking on the close and the miniaturize buttons (but
-  // not the zoom button) while a window is in the background does NOT bring
-  // that window to the front. We don't get that behavior for free (probably
-  // because the tab strip view covers those buttons), so we handle it here.
-  // Zoom buttons do bring the window to the front. Note that Finder windows (in
-  // Leopard) behave differently in this regard in that zoom buttons don't bring
-  // the window to the foreground.

[Andre, 2015/02/04 23:03:23]

This looks like it's no longer a problem.
There was a fix sometime ago to ensure that the buttons are never covered by the
tab strip view.
Let me know if you can think of something that this change might break.

-  BOOL eventHandled = NO;
-  if (![self isMainWindow]) {
-    if ([event type] == NSLeftMouseDown) {
-      NSView* frameView = [self frameView];
-      NSPoint mouse = [frameView convertPoint:[event locationInWindow]
-                                     fromView:nil];
-      if (NSPointInRect(mouse, [closeButton_ frame])) {
-        [closeButton_ mouseDown:event];
-        eventHandled = YES;
-      } else if (NSPointInRect(mouse, [miniaturizeButton_ frame])) {
-        [miniaturizeButton_ mouseDown:event];
-        eventHandled = YES;
-      }
-    }
-  }
-  if (!eventHandled) {
-    [super sendEvent:event];
-  }
-}
-
 - (void)setShouldHideTitle:(BOOL)flag {
   shouldHideTitle_ = flag;
 }
 
 - (BOOL)_isTitleHidden {
   return shouldHideTitle_;
 }
 
 - (CGFloat)windowButtonsInterButtonSpacing {
   return windowButtonsInterButtonSpacing_;
@@ skipping 98 matching lines @@
     NSCompositingOperation operation = NSCompositeCopy;
     if (forceBlackBackground) {
       [[NSColor blackColor] set];
       NSRectFill(dirtyRect);
       operation = NSCompositeSourceOver;
     }
 
     NSPoint position = [[view window] themeImagePositionForAlignment:
         THEME_IMAGE_ALIGN_WITH_FRAME];
 
-    // Align the phase to physical pixels so resizing the window under HiDPI
-    // doesn't cause wiggling of the theme.
-    NSView* frameView = [[[view window] contentView] superview];
-    position = [frameView convertPointToBase:position];

[Andre, 2015/02/04 23:03:23]

This is deprecated in 10.7, and is not the right thing to do for layer backed
views.

[erikchen, 2015/02/04 23:53:55]

Shouldn't we have some sort of logic for 10.6? 

[Andre, 2015/02/05 01:04:26]

Good point.
Took a deeper look at this, I believe this code does not really do anything.
It basically wants to floor |position| which is in window coordinates to a
multiple of 1/backingScaleFactor.
But looking at +[BrowserWindowUtils
themeImagePositionInTabStripCoords:alignment],
it should always return a round number since [windowView bounds] and
[tabStripView bounds] are round numbers.

I could not reproduce the theme wiggling on 1x or 2x, and the value of
|position|
before and after this code is always the same.

I removed this code in patchset 2.

-    position.x = floor(position.x);
-    position.y = floor(position.y);
-    position = [frameView convertPointFromBase:position];
+    if ([view respondsToSelector:@selector(convertPointToBacking:)]) {
+      // Align the phase to physical pixels so resizing the window under HiDPI
+      // doesn't cause wiggling of the theme.
+      position = [view convertPointToBacking:position];
+      position.x = floor(position.x);
+      position.y = floor(position.y);
+      position = [view convertPointFromBacking:position];
+    }
+
     [[NSGraphicsContext currentContext] cr_setPatternPhase:position
                                                    forView:view];
 
     [themeImageColor set];
     NSRectFillUsingOperation(dirtyRect, operation);
     themed = YES;
   } else if (gradient) {
     NSPoint startPoint = NSMakePoint(NSMinX(bounds), NSMaxY(bounds));
     NSPoint endPoint = startPoint;
     endPoint.y -= kBrowserFrameViewPaintHeight;
     [gradient drawFromPoint:startPoint toPoint:endPoint options:0];
     themed = YES;
   }
 
   // Check to see if we have an overlay image.
   NSImage* overlayImage = nil;
   if (themeProvider->HasCustomImage(IDR_THEME_FRAME_OVERLAY) && !incognito &&
       !popup) {
     overlayImage = themeProvider->
         GetNSImageNamed(active ? IDR_THEME_FRAME_OVERLAY :
                                  IDR_THEME_FRAME_OVERLAY_INACTIVE);
   }
 
   if (overlayImage) {
     // Anchor to top-left and don't scale.
-    NSView* frameView = [[[view window] contentView] superview];
     NSPoint position = [[view window] themeImagePositionForAlignment:
         THEME_IMAGE_ALIGN_WITH_FRAME];
-    position = [view convertPoint:position fromView:frameView];
+    position = [view convertPoint:position fromView:nil];
     NSSize overlaySize = [overlayImage size];
     NSRect imageFrame = NSMakeRect(0, 0, overlaySize.width, overlaySize.height);
     [overlayImage drawAtPoint:NSMakePoint(position.x,
                                           position.y - overlaySize.height)
                      fromRect:imageFrame
                     operation:NSCompositeSourceOver
                      fraction:1.0];
   }
 
   return themed;
 }
 
 - (NSColor*)titleColor {
   ui::ThemeProvider* themeProvider = [self themeProvider];
   if (!themeProvider)
     return [NSColor windowFrameTextColor];
 
   ThemedWindowStyle windowStyle = [self themedWindowStyle];
   BOOL incognito = windowStyle & THEMED_INCOGNITO;
 
   if (incognito)
     return [NSColor whiteColor];
   else
     return [NSColor windowFrameTextColor];
 }
 
 @end