Mac: Clean up outdated use of NSThemeFrame

Mac: Clean up outdated use of NSThemeFrame

We have now moved to a better place where we no longer swizzle
NSThemeFrame's drawRect, and use a subview instead
(TabStripBackgroundView).

We also no longer add subviews to NSThemeFrame (with the
exception of TabStripBackgroundView), and use a full-size
content view instead.

This is a clean-up of code that relies on outdated assumptions.
For example, FramedBrowserWindow was unnecessarily calling
setNeedsDisplay and setNeedsDisplayInRect.

BUG=453996
Committed: https://crrev.com/51fe497c74cf1378f40159250fa1d627e893cf01
Cr-Commit-Position: refs/heads/master@{#314845}

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

andresantoso@chromium.org changed reviewers:
+ erikchen@chromium.org, rsesek@chromium.org

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

Robert, Erik, please review.

https://codereview.chromium.org/871423004/diff/1/chrome/browser/ui/cocoa/brow...
File chrome/browser/ui/cocoa/browser_window_controller.h (right):

https://codereview.chromium.org/871423004/diff/1/chrome/browser/ui/cocoa/brow...
chrome/browser/ui/cocoa/browser_window_controller.h:356: // Returns the position
in window coordinates that the top left of a theme
They are the same coordinates, but window coordinates is the much more common
term.

https://codereview.chromium.org/871423004/diff/1/chrome/browser/ui/cocoa/fram...
File chrome/browser/ui/cocoa/framed_browser_window.mm (left):

https://codereview.chromium.org/871423004/diff/1/chrome/browser/ui/cocoa/fram...
chrome/browser/ui/cocoa/framed_browser_window.mm:232: // the window to the
foreground.
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.

https://codereview.chromium.org/871423004/diff/1/chrome/browser/ui/cocoa/fram...
chrome/browser/ui/cocoa/framed_browser_window.mm:374: position = [frameView
convertPointToBase:position];
This is deprecated in 10.7, and is not the right thing to do for layer backed
views.

https://codereview.chromium.org/871423004/diff/1/chrome/browser/ui/cocoa/tabs...
File chrome/browser/ui/cocoa/tabs/tab_window_controller.h (left):

https://codereview.chromium.org/871423004/diff/1/chrome/browser/ui/cocoa/tabs...
chrome/browser/ui/cocoa/tabs/tab_window_controller.h:164: -
(void)insertTabStripBackgroundViewIntoWindow:(NSWindow*)window;
This is also declared in the .mm file, so remove this duplicate.

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

https://codereview.chromium.org/871423004/diff/1/chrome/browser/ui/cocoa/fram...
File chrome/browser/ui/cocoa/framed_browser_window.mm (left):

https://codereview.chromium.org/871423004/diff/1/chrome/browser/ui/cocoa/fram...
chrome/browser/ui/cocoa/framed_browser_window.mm:222: [[self frameView]
setNeedsDisplay:YES];
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?

https://codereview.chromium.org/871423004/diff/1/chrome/browser/ui/cocoa/fram...
chrome/browser/ui/cocoa/framed_browser_window.mm:374: position = [frameView
convertPointToBase:position];
On 2015/02/04 23:03:23, Andre wrote:
> This is deprecated in 10.7, and is not the right thing to do for layer backed
> views.

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

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

https://codereview.chromium.org/871423004/diff/1/chrome/browser/ui/cocoa/fram...
File chrome/browser/ui/cocoa/framed_browser_window.mm (left):

https://codereview.chromium.org/871423004/diff/1/chrome/browser/ui/cocoa/fram...
chrome/browser/ui/cocoa/framed_browser_window.mm:222: [[self frameView]
setNeedsDisplay:YES];
On 2015/02/04 23:53:55, erikchen wrote:
> 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?

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.

https://codereview.chromium.org/871423004/diff/1/chrome/browser/ui/cocoa/fram...
chrome/browser/ui/cocoa/framed_browser_window.mm:374: position = [frameView
convertPointToBase:position];
On 2015/02/04 23:53:55, erikchen wrote:
> On 2015/02/04 23:03:23, Andre wrote:
> > This is deprecated in 10.7, and is not the right thing to do for layer
backed
> > views.
> 
> Shouldn't we have some sort of logic for 10.6? 

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.

[erikchen, 2015-02-05 01:26:04]

lgtm

[Robert Sesek, 2015-02-05 19:01:09]

LGTM

[Andre, 2015-02-05 19:03:25]

The CQ bit was checked by andresantoso@chromium.org

[commit-bot: I haz the power, 2015-02-05 19:04:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/871423004/20001

[commit-bot: I haz the power, 2015-02-05 19:09:05]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-02-05 19:10:06]

Patchset 2 (id:??) landed as
https://crrev.com/51fe497c74cf1378f40159250fa1d627e893cf01
Cr-Commit-Position: refs/heads/master@{#314845}