MacViews: Fix positioning of top-level Widgets with parents

ui/views/cocoa/bridged_native_widget.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.
 
 #import "ui/views/cocoa/bridged_native_widget.h"
 
 #import <objc/runtime.h>
 
 #include "base/logging.h"
 #include "base/mac/mac_util.h"
@@ skipping 20 matching lines @@
 
 int kWindowPropertiesKey;
 
 float GetDeviceScaleFactorFromView(NSView* view) {
   gfx::Display display =
       gfx::Screen::GetScreenFor(view)->GetDisplayNearestWindow(view);
   DCHECK(display.is_valid());
   return display.device_scale_factor();
 }
 
+// Returns true if bounds passed to window in SetBounds should be treated as
+// though they are in screen coordinates.
+bool PositionWindowInScreenCoordinates(views::Widget* widget,
+                                       views::Widget::InitParams::Type type) {
+  // Replicate the logic in desktop_aura/desktop_screen_position_client.cc.
+  if (type == views::Widget::InitParams::TYPE_POPUP)

[Andre, 2015/02/23 18:27:08]

The aura version of this checks for ui::wm::WINDOW_TYPE_POPUP which seems to
have a different meaning from InitParams::TYPE_POPUP.
GetAuraWindowTypeForWidgetType() maps several InitParams types to
WINDOW_TYPE_POPUP:

https://code.google.com/p/chromium/codesearch#chromium/src/ui/views/widget/wi...
  

[tapted, 2015/02/24 00:14:32]

ooh - good catch! Fixed.

used GetAuraWindowTypeForWidgetType(..) directly.. it introduces a dependency on
an enum defined in Aura, but not code. I think that's marginally better than
duplicating the logic somewhere, and the long-term goal is aura, so I don't
think we need to move the enum unless there's an issue. Widget::InitParams
already does a similar thing with aura::WindowLayerType.

+    return true;
+
+  return widget && widget->is_top_level();
+}
+
 }  // namespace
 
 namespace views {
 
 BridgedNativeWidget::BridgedNativeWidget(NativeWidgetMac* parent)
     : native_widget_mac_(parent),
-      focus_manager_(NULL),
+      focus_manager_(nullptr),
+      widget_type_(Widget::InitParams::TYPE_WINDOW),  // Updated in Init().
       parent_(nullptr),
       target_fullscreen_state_(false),
       in_fullscreen_transition_(false),
       window_visible_(false),
       wants_to_be_visible_(false) {
   DCHECK(parent);
   window_delegate_.reset(
       [[ViewsNSWindowDelegate alloc] initWithBridgedNativeWidget:this]);
 }
 
 BridgedNativeWidget::~BridgedNativeWidget() {
   RemoveOrDestroyChildren();
   DCHECK(child_windows_.empty());
   SetFocusManager(NULL);
   SetRootView(NULL);
   DestroyCompositor();
   if ([window_ delegate]) {
     // If the delegate is still set, it means OnWindowWillClose has not been
     // called and the window is still open. Calling -[NSWindow close] will
     // synchronously call OnWindowWillClose and notify NativeWidgetMac.
     [window_ close];
   }
   DCHECK(![window_ delegate]);
 }
 
 void BridgedNativeWidget::Init(base::scoped_nsobject<NSWindow> window,
                                const Widget::InitParams& params) {
+  widget_type_ = params.type;
+
   DCHECK(!window_);
   window_.swap(window);
   [window_ setDelegate:window_delegate_];
 
   // Register for application hide notifications so that visibility can be
   // properly tracked. This is not done in the delegate so that the lifetime is
   // tied to the C++ object, rather than the delegate (which may be reference
   // counted). This is required since the application hides do not send an
   // orderOut: to individual windows. Unhide, however, does send an order
   // message.
@@ skipping 14 matching lines @@
     BridgedNativeWidget* parent =
         NativeWidgetMac::GetBridgeForNativeWindow([params.parent window]);
     // The parent could be an NSWindow without an associated Widget. That could
     // work by observing NSWindowWillCloseNotification, but for now it's not
     // supported, and there might not be a use-case for that.
     CHECK(parent);
     parent_ = parent;
     parent->child_windows_.push_back(this);
   }
 
+  // Set a meaningful initial bounds. Note that except for frameless widgets
+  // with no WidgetDelegate, the bounds will be set again by Widget after
+  // initializing the non-client view. In the former case, if bounds were not
+  // set at all, the creator of the Widget is expected to call SetBounds()
+  // before calling Widget::Show() to avoid a kWindowSizeDeterminedLater-sized
+  // (i.e. 1x1) window appearing.
+  if (!params.bounds.IsEmpty()) {
+    SetBounds(params.bounds);
+  } else {
+    // If a position is set, but no size, complain. Otherwise, a 1x1 window
+    // would appear there, which might be unexpected.
+    DCHECK(params.bounds.origin().IsOrigin())
+        << "Zero-sized windows not supported on Mac.";
+  }
+
   // Widgets for UI controls (usually layered above web contents) start visible.
   if (params.type == Widget::InitParams::TYPE_CONTROL)
     SetVisibilityState(SHOW_INACTIVE);
 }
 
 void BridgedNativeWidget::SetFocusManager(FocusManager* focus_manager) {
   if (focus_manager_ == focus_manager)
     return;
 
   if (focus_manager_)
     focus_manager_->RemoveFocusChangeListener(this);
 
   if (focus_manager)
     focus_manager->AddFocusChangeListener(this);
 
   focus_manager_ = focus_manager;
 }
 
 void BridgedNativeWidget::SetBounds(const gfx::Rect& new_bounds) {
+  // A contentRect with zero width or height is a banned practice in ChromeMac,
+  // due to unpredictable OSX treatment.
+  DCHECK(!new_bounds.IsEmpty()) << "Zero-sized windows not supported on Mac";
+
   gfx::Rect actual_new_bounds(new_bounds);
-  if (parent_)
+
+  if (parent_ &&
+      !PositionWindowInScreenCoordinates(native_widget_mac_->GetWidget(),
+                                         widget_type_))
     actual_new_bounds.Offset(parent_->GetRestoredBounds().OffsetFromOrigin());
 
   [window_ setFrame:gfx::ScreenRectToNSRect(actual_new_bounds)
             display:YES
             animate:NO];
 }
 
 void BridgedNativeWidget::SetRootView(views::View* view) {
   if (view == [bridged_view_ hostedView])
     return;
@@ skipping 555 matching lines @@
       window_, &kWindowPropertiesKey);
   if (!properties) {
     properties = [NSMutableDictionary dictionary];
     objc_setAssociatedObject(window_, &kWindowPropertiesKey,
                              properties, OBJC_ASSOCIATION_RETAIN);
   }
   return properties;
 }
 
 }  // namespace views