[MacViews] Fix behavior of non-resizable windows in fullscreen.

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 24275787c04e43ac1ab87b032e13ff5f4665461c..ad0c44a7501cd0cdccb05ab4b152e62b3cde1a6a 100644
--- a/ui/views/cocoa/bridged_native_widget.mm
+++ b/ui/views/cocoa/bridged_native_widget.mm
@@ -374,7 +374,6 @@ void BridgedNativeWidget::OnFullscreenTransitionStart(
   // shouldn't be able to do that if the window is invisible to begin with.
   DCHECK(window_visible_);
 
-  DCHECK_NE(target_fullscreen_state, target_fullscreen_state_);

[tapted, 2015/05/13 06:12:42]

I think we can keep this.... see below

[jackhou1, 2015/05/13 06:53:10]

Done.

   target_fullscreen_state_ = target_fullscreen_state;
   in_fullscreen_transition_ = true;
 
@@ -386,6 +385,12 @@ void BridgedNativeWidget::OnFullscreenTransitionStart(
 void BridgedNativeWidget::OnFullscreenTransitionComplete(
     bool actual_fullscreen_state) {
   in_fullscreen_transition_ = false;
+
+  // If window did leave fullscreen, reset the size constraints and collection
+  // behavior.
+  if (!actual_fullscreen_state)
+    OnSizeConstraintsChanged();

[tapted, 2015/05/13 06:12:42]

This should only happen if we're staying in this state -- i.e. within the `if`
block just below, before the early return.

Otherwise, we just undo this step in ToggleDesiredFullscreenState


(and with my other suggestions, the other check is redundant. i.e. you could do
just (below)

  if (target_fullscreen_state_ == actual_fullscreen_state) {
    // Ensure constraints are re-applied when completing a transition.
    OnSizeContraintsChanged();
    return;
  }

[jackhou1, 2015/05/13 06:53:10]

Done.

+
   if (target_fullscreen_state_ == actual_fullscreen_state)
     return;
 
@@ -430,13 +435,18 @@ void BridgedNativeWidget::ToggleDesiredFullscreenState() {
     return;  // TODO(tapted): Implement this for Snow Leopard.
   }
 
-  // Since fullscreen requests are ignored if the collection behavior does not
-  // allow it, save the collection behavior and restore it after.
-  NSWindowCollectionBehavior behavior = [window_ collectionBehavior];
-  [window_ setCollectionBehavior:behavior |
-                                 NSWindowCollectionBehaviorFullScreenPrimary];
+  // Before entering fullscreen, and during fullscreen:
+  // 1: Size constraints must be removed so that the window takes up the entire
+  //    screen.
+  // 2: Fullscreen collection behavior must be enabled because:
+  //   a: -[NSWindow toggleFullscreen:] would otherwise be ignored,
+  //   b: it enables the fullscreen button so the user can leave fullscreen.
+  // These will be reset after leaving fullscreen.
+  if (!([ns_window() styleMask] & NSFullScreenWindowMask)) {
+    target_fullscreen_state_ = true;

[tapted, 2015/05/13 06:12:42]

it feels fragile to update target_fullscreen_state_ here as well as
OnFullscreenTransitionStart -- OnFullscreenTransitionStart/Complete is the
source of truth.

Also, I don't think it's nice to check the current styleMask -- Cocoa updates it
at weird times (I think the current code guarantees that if
target_fullscreen_state_ is false, we are trying to enter fullscreen, but I
don't think we need to check).

Here (instead of the if), I'd suggest just doing something like

  // Clear any constraints and enable fullscreen before starting any transition
  // (they will be re-applied when a transition out of fullscreen completes).
  ApplyNSWindowSizeConstraints(window_, gfx::Size(), gfx::Size(), true, true);


But, I think this will leave the problem that changing the style mask will
trigger a call to setFrame -> OnRootViewLayout -> OnSizeConstraintsChanged -- I
think we need to avoid that, but in a more obvious way.

- is the frame size actually changing? (can we early exit in [BridgedContentView
setFrameSize:]. There's an early exit in View::SetBoundsRect when needs_layout
is false -- why isn't that coming into play?

Can we make the setStyleMask in ApplyNSWindowSizeConstraints a no-op?
e.g., here, do

  /* style_mask &= ~NSResizableWindowMask; */
  ApplyNSWindowSizeConstraints(window_, gfx::Size(), gfx::Size(), true, true);

That's still hacky (we don't want that useless layout call before going
fullscreen), so would need a good comment.


Otherwise.... I'd be looking to nerf the hook in [BridgedContentView
setFrameSize:]. E.g.

// To make a non-user-resizable window properly fullscreen, it must have have
// NSResizableWindowMask set. Changing this flag triggers a resize, but it won't
// be the resize that takes the window into or out of fullscreen. To avoid a
// useless layout, temporarily suppress frame change propagations, and wait for
// the one that is triggered from [somewhere].
[bridged_view_ setIgnoreFrameChanges:YES];
ApplyNSWindowSizeConstraints(window_, gfx::Size(), gfx::Size(), true, true);
[bridged_view_ setIgnoreFrameChanges:NO];



OR!

If we can do (here)

// Ensure a setStyleMask call doesn't trigger a useless layout. [more words]
bool can_resize = style_mask() & NSResizableWindowMask;
ApplyNSWindowSizeConstraints(window_, gfx::Size(), gfx::Size(), can_resize,
true);

and combine that with the 

willUseFullScreenContentSize:

trick in views_nswindow_delegate.mm

then maybe that's a nice way to do it.

But I'd bring back willUseFullScreenContentSize with a comment like ~"Without
this, windows with the NSResizableWindowMask cleared from their styleMask will
only enter fullscreen at their
current size rather than resizing to fill the screen."

[jackhou1, 2015/05/13 06:53:10]

By bringing back the willUseFullScreenContentSize, we no longer need to set the
style mask or size constraints here. So this just enables fullscreen collection
behavior.

+    OnSizeConstraintsChanged();
+  }
   [window_ toggleFullScreen:nil];
-  [window_ setCollectionBehavior:behavior];
 }
 
 void BridgedNativeWidget::OnSizeChanged() {
@@ -522,6 +532,15 @@ void BridgedNativeWidget::OnWindowKeyStatusChangedTo(bool is_key) {
 
 void BridgedNativeWidget::OnSizeConstraintsChanged() {
   NSWindow* window = ns_window();
+
+  // If entering or in fullscreen, remove size constraints so that the window
+  // fills the screen, and show fullscreen controls so the user can leave.
+  if (target_fullscreen_state_) {

[tapted, 2015/05/13 06:12:42]

I think just (at the start, with an appropriate comment):

if (target_fullscreen_state_ || in_fullscreen_transition)
  return;

[jackhou1, 2015/05/13 06:53:10]

Done.

+    gfx::ApplyNSWindowSizeConstraints(window, gfx::Size(), gfx::Size(), true,
+                                      true);
+    return;
+  }
+
   Widget* widget = native_widget_mac()->GetWidget();
   gfx::Size min_size = widget->GetMinimumSize();
   gfx::Size max_size = widget->GetMaximumSize();