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

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 356 matching lines @@
   [[NSNotificationCenter defaultCenter] removeObserver:window_delegate_];
   native_widget_mac_->OnWindowWillClose();
 }
 
 void BridgedNativeWidget::OnFullscreenTransitionStart(
     bool target_fullscreen_state) {
   // Note: This can fail for fullscreen changes started externally, but a user
   // 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;
 
   // If going into fullscreen, store an answer for GetRestoredBounds().
   if (target_fullscreen_state)
     bounds_before_fullscreen_ = gfx::ScreenRectFromNSRect([window_ frame]);
 }
 
 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;
 
   // First update to reflect reality so that OnTargetFullscreenStateChanged()
   // expects the change.
   target_fullscreen_state_ = actual_fullscreen_state;
   ToggleDesiredFullscreenState();
 
   // Usually ToggleDesiredFullscreenState() sets |in_fullscreen_transition_| via
   // OnFullscreenTransitionStart(). When it does not, it means Cocoa ignored the
@@ skipping 24 matching lines @@
   // of relying on AppKit to do it, and not worry that OnVisibilityChanged()
   // won't be called for externally triggered fullscreen requests.
   if (!window_visible_)
     SetVisibilityState(SHOW_INACTIVE);
 
   if (base::mac::IsOSSnowLeopard()) {
     NOTIMPLEMENTED();
     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() {
   gfx::Size new_size = GetClientAreaSize();
   native_widget_mac_->GetWidget()->OnNativeWidgetSizeChanged(new_size);
   if (layer())
     UpdateLayerProperties();
 }
 
 void BridgedNativeWidget::OnVisibilityChanged() {
@@ skipping 65 matching lines @@
       widget->GetFocusManager()->RestoreFocusedView();
     } else {
       widget->OnNativeBlur();
       widget->GetFocusManager()->StoreFocusedView(true);
     }
   }
 }
 
 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();
   bool is_resizable = widget->widget_delegate()->CanResize();
   bool shows_resize_controls =
       is_resizable && (min_size.IsEmpty() || min_size != max_size);
   bool shows_fullscreen_controls =
       is_resizable && widget->widget_delegate()->CanMaximize();
 
   gfx::ApplyNSWindowSizeConstraints(window, min_size, max_size,
@@ skipping 297 matching lines @@
       window_, &kWindowPropertiesKey);
   if (!properties) {
     properties = [NSMutableDictionary dictionary];
     objc_setAssociatedObject(window_, &kWindowPropertiesKey,
                              properties, OBJC_ASSOCIATION_RETAIN);
   }
   return properties;
 }
 
 }  // namespace views