MacViews: Fix positioning of top-level Widgets with parents

ui/views/cocoa/bridged_native_widget.mm

Index: ui/views/cocoa/bridged_native_widget.mm
diff --git a/ui/views/cocoa/bridged_native_widget.mm b/ui/views/cocoa/bridged_native_widget.mm
index 0d397bd8de76be2ebf0eba52d59785bbf89e4834..b0de1547d42a5fc5e777286a1af66c14a8814e5f 100644
--- a/ui/views/cocoa/bridged_native_widget.mm
+++ b/ui/views/cocoa/bridged_native_widget.mm
@@ -38,13 +38,25 @@ float GetDeviceScaleFactorFromView(NSView* view) {
   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),
@@ -72,6 +84,8 @@ BridgedNativeWidget::~BridgedNativeWidget() {
 
 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_];
@@ -106,6 +120,21 @@ void BridgedNativeWidget::Init(base::scoped_nsobject<NSWindow> window,
     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);
@@ -125,8 +154,15 @@ void BridgedNativeWidget::SetFocusManager(FocusManager* 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)