AppShell support for Mac.

extensions/shell/browser/shell_native_app_window_mac.mm

+// Copyright 2014 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.
+
+#include "extensions/shell/browser/shell_native_app_window_mac.h"

[tapted, 2014/12/17 08:05:16]

nit: import

[Yoyo Zhou, 2014/12/18 02:38:34]

Done.

+
+#import <Cocoa/Cocoa.h>
+
+#include "base/logging.h"
+#include "base/strings/sys_string_conversions.h"
+#include "content/public/browser/web_contents.h"
+
+@implementation ShellNativeAppWindowController
+
+@synthesize appWindow = appWindow_;
+
+- (void)windowWillClose:(NSNotification*)notification {
+  if (appWindow_)
+    appWindow_->WindowWillClose();
+}
+
+@end
+
+// TODO(yoz): Do we need to handle commands (keyboard shortcuts)?
+// Do we need need ChromeEventProcessingWindow or UnderlayOpenGLHostingWindow?
+@interface ShellNSWindow : NSWindow
+@end
+
+@implementation ShellNSWindow
+
+- (BOOL)_isTitleHidden {
+  return YES;
+}
+
+@end
+
+namespace extensions {
+
+ShellNativeAppWindowMac::ShellNativeAppWindowMac(
+    AppWindow* app_window,
+    const AppWindow::CreateParams& params)
+    : ShellNativeAppWindow(app_window, params) {
+  base::scoped_nsobject<NSWindow> shell_window;
+  NSUInteger style_mask = NSTitledWindowMask | NSClosableWindowMask;
+
+  NSRect cocoa_bounds = params.GetInitialWindowBounds(gfx::Insets()).ToCGRect();
+  // TODO(yoz): Flip coordinates here?

[tapted, 2014/12/17 08:05:16]

you could use gfx::ScreenRectToNSRect from #import
"ui/gfx/mac/coordinate_conversion.h"

it might also be good to check for a zero-sized window -- see
http://www.chromium.org/developers/coding-style/cocoa-dos-and-donts#TOC-Zero-...

[Yoyo Zhou, 2014/12/18 02:38:34]

Done.
What should we do if there is a zero-sized window? I'm using
NativeAppWIndowCocoa as a reference implementation, and it doesn't do anything
in that case.

+
+  shell_window.reset(
+      [[ShellNSWindow alloc] initWithContentRect:cocoa_bounds
+                                       styleMask:style_mask
+                                         backing:NSBackingStoreBuffered
+                                           defer:NO]);
+
+  window_controller_.reset([[ShellNativeAppWindowController alloc]
+      initWithWindow:shell_window.release()]);

[tapted, 2014/12/17 08:05:16]

The release() looks a bit like a leak, but it's needed unless you call
[shell_window setReleasedWhenClosed:NO].

Usually I find things easier to follow with setReleasedWhenClosed:NO, then
ShellNativeAppWindowMac could have scoped_nsobjects for both the NSWindow and
the NSWindowDelegate classes, and you get better control over the lifetime.

In ~ShellNativeAppWindowMac(), you would just do
  [window() setDelegate:nil];
  [window() close];

That might untangle the lifetime comments in shell_desktop_controller_mac.mm
too.


Also, since the delegate is owned by |this|, it might be nicer to pass that to a
newly declared intializer. i.e.

  window_controller_.reset(
      [[ShellNativeAppWindowController alloc] initWithAppWindow:app_window]);

then in, initWithAppWindow, create the ShellNSWindow and do

  if ((self = [super initWithWindow:shell_window])) {
    appWindow_ = appWindow;
  }
  return self;
}


(then make the property readonly).

This is fine too, but it might be a good follow-up if the initialization gets
more complicated. 


But if you don't like these ideas, and stick with the default (for NSWindow) of
releasedWhenClosed YES, then I'd just put a comment before this like

// Call release(), since [NSWindow releasedWhenClosed] defaults to YES.

[Yoyo Zhou, 2014/12/18 02:38:34]

This looks more complicated right now; I was hoping to have just a minimal
NSWindowController.
I think this release() and the Cocoa -release are not the same:

// scoped_nsprotocol<>::release() is like scoped_ptr<>::release.  It is NOT a
// wrapper for [object_ release].

So my release() here is so the window doesn't get deallocated (-released) when
the scoped_nsobject goes out of scope. I don't have a strong opinion on
released-when-closed, though, so I'll go with that.

I'm less confident about whether it's now a leak.

[tapted, 2014/12/18 03:30:55]

Corect - the releases are not the same. But NSWindow is weird, since it can
effectively own itself.

e.g. This is a leak:

scoped_nsobject<NSString> foo([[NSString alloc] init]);
scoped_nsobject<NSString> bar([[NSString alloc] initWithString:foo.release()];

but a similar thing with NSWindow is not, so long as the NSWindow is later shown
(and then closed).

To fix the leak in the example above, it should be just

scoped_nsobject<NSString> bar([[NSString alloc] initWithString:foo];

+
+  [[window_controller_ window] setDelegate:window_controller_];
+  [window_controller_ setAppWindow:this];
+
+  NSView* view = app_window->web_contents()->GetNativeView();
+  NSView* frameView = [window() contentView];
+  [view setFrame:[frameView bounds]];
+  [frameView addSubview:view];
+}
+
+ShellNativeAppWindowMac::~ShellNativeAppWindowMac() {
+}

[tapted, 2014/12/17 08:05:16]

this should call [window() setDelegate:nil];  (the window_controller_ data
member is about to go out of scope, but delegates are not retained, and the
window is reference counted, so there could be references to it in event loops
and things that keep it alive)

[Yoyo Zhou, 2014/12/18 02:38:34]

Done.

+
+bool ShellNativeAppWindowMac::IsActive() const {
+  NOTIMPLEMENTED();

[tapted, 2014/12/17 08:05:16]

if you feel like it now.. [window() isKeyWindow] should be enough.

[Yoyo Zhou, 2014/12/18 02:38:34]

Done.

+  return false;
+}
+
+gfx::NativeWindow ShellNativeAppWindowMac::GetNativeWindow() const {
+  return window();
+}
+
+gfx::Rect ShellNativeAppWindowMac::GetBounds() const {
+  // Flip coordinates based on the primary screen.

[tapted, 2014/12/17 08:05:16]

  return gfx::ScreenRectFromNSRect([window() frame]);

[Yoyo Zhou, 2014/12/18 02:38:34]

Done.

+  NSScreen* screen = [[NSScreen screens] objectAtIndex:0];
+  NSRect frame = [[window_controller_ window] frame];
+  return gfx::Rect(frame.origin.x, NSHeight([screen frame]) - NSMaxY(frame),
+                   NSWidth(frame), NSHeight(frame));
+}
+
+void ShellNativeAppWindowMac::Show() {
+  [window_controller_ showWindow:nil];
+}
+
+void ShellNativeAppWindowMac::Hide() {
+  NOTIMPLEMENTED();
+}
+
+void ShellNativeAppWindowMac::Activate() {
+  // TODO(yoz): Activate in front of other applications.
+  [[window_controller_ window] makeKeyAndOrderFront:window_controller_];
+}
+
+void ShellNativeAppWindowMac::Deactivate() {
+  // See crbug.com/51364.
+  NOTIMPLEMENTED();
+}
+
+void ShellNativeAppWindowMac::SetBounds(const gfx::Rect& bounds) {
+  // TODO(yoz): Windows should be fullscreen.
+  NOTIMPLEMENTED();
+}
+
+void ShellNativeAppWindowMac::WindowWillClose() {
+  [window_controller_ setAppWindow:NULL];
+  app_window()->OnNativeWindowChanged();
+  app_window()->OnNativeClose();
+}
+
+ShellNSWindow* ShellNativeAppWindowMac::window() const {
+  NSWindow* window = [window_controller_ window];
+  CHECK(!window || [window isKindOfClass:[ShellNSWindow class]]);

[tapted, 2014/12/17 08:05:16]

maybe, from base/mac/foundation_util.h

  return base::mac::ObjCCastStrict<ShellNSWindow>([window_controller_ window]);

(note that would make it a DCHECK)

[Yoyo Zhou, 2014/12/18 02:38:34]

Done.

+  return static_cast<ShellNSWindow*>(window);
+}
+
+}  // namespace extensions