Mac: Clean up outdated use of NSThemeFrame

chrome/browser/ui/cocoa/framed_browser_window.mm

Index: chrome/browser/ui/cocoa/framed_browser_window.mm
diff --git a/chrome/browser/ui/cocoa/framed_browser_window.mm b/chrome/browser/ui/cocoa/framed_browser_window.mm
index c290f70605fa91b6821b014f42a2d70f81ad4542..533fae676b34b25549f6bbd60e24a6ac528d0cdb 100644
--- a/chrome/browser/ui/cocoa/framed_browser_window.mm
+++ b/chrome/browser/ui/cocoa/framed_browser_window.mm
@@ -41,7 +41,6 @@ const CGFloat kWindowGradientHeight = 24.0;
 - (void)adjustZoomButton:(NSNotification*)notification;
 - (void)adjustButton:(NSButton*)button
               ofKind:(NSWindowButton)kind;
-- (NSView*)frameView;
 
 @end
 
@@ -111,10 +110,6 @@ const CGFloat kWindowGradientHeight = 24.0;
                selector:@selector(adjustZoomButton:)
                    name:NSViewFrameDidChangeNotification
                  object:zoomButton_];
-    [center addObserver:self
-               selector:@selector(themeDidChangeNotification:)
-                   name:kBrowserThemeDidChangeNotification
-                 object:nil];
   }
 
   return self;
@@ -143,7 +138,6 @@ const CGFloat kWindowGradientHeight = 24.0;
 - (void)adjustButton:(NSButton*)button
               ofKind:(NSWindowButton)kind {
   NSRect buttonFrame = [button frame];
-  NSRect frameViewBounds = [[self frameView] bounds];
 
   CGFloat xOffset = hasTabStrip_
       ? kFramedWindowButtonsWithTabStripOffsetFromLeft
@@ -152,7 +146,7 @@ const CGFloat kWindowGradientHeight = 24.0;
       ? kFramedWindowButtonsWithTabStripOffsetFromTop
       : kFramedWindowButtonsWithoutTabStripOffsetFromTop;
   buttonFrame.origin =
-      NSMakePoint(xOffset, (NSHeight(frameViewBounds) -
+      NSMakePoint(xOffset, (NSHeight([self frame]) -
                             NSHeight(buttonFrame) - yOffset));
 
   switch (kind) {
@@ -174,10 +168,6 @@ const CGFloat kWindowGradientHeight = 24.0;
   [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 {
@@ -196,60 +186,6 @@ const CGFloat kWindowGradientHeight = 24.0;
   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;
 }
@@ -368,13 +304,15 @@ const CGFloat kWindowGradientHeight = 24.0;
     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];
 
@@ -400,10 +338,9 @@ const CGFloat kWindowGradientHeight = 24.0;
 
   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,