mash: improves browser frame decorations

ash/common/frame/custom_frame_view_ash.h

 // Copyright (c) 2012 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.
 
 #ifndef ASH_COMMON_FRAME_CUSTOM_FRAME_VIEW_ASH_H_
 #define ASH_COMMON_FRAME_CUSTOM_FRAME_VIEW_ASH_H_
 
 #include <memory>
 
 #include "ash/ash_export.h"
+#include "ash/public/interfaces/window_style.mojom.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/views/window/non_client_view.h"
 
 namespace views {
 class Widget;
 }
 
 namespace ash {
 
 class FrameCaptionButtonContainerView;
@@ skipping 14 matching lines @@
 
   // |enable_immersive| controls whether ImmersiveFullscreenController is
   // created for the CustomFrameViewAsh; if true and a WindowStateDelegate has
   // not been set on the WindowState associated with |frame|, then an
   // ImmersiveFullscreenController is created.
   // If ImmersiveFullscreenControllerDelegate is not supplied, HeaderView is
   // used as the ImmersiveFullscreenControllerDelegate.
   explicit CustomFrameViewAsh(
       views::Widget* frame,
       ImmersiveFullscreenControllerDelegate* immersive_delegate = nullptr,
-      bool enable_immersive = true);
+      bool enable_immersive = true,
+      mojom::WindowStyle window_style = mojom::WindowStyle::DEFAULT);
   ~CustomFrameViewAsh() override;
 
   // Inits |immersive_fullscreen_controller| so that the controller reveals
   // and hides |header_view_| in immersive fullscreen.
   // CustomFrameViewAsh does not take ownership of
   // |immersive_fullscreen_controller|.
   void InitImmersiveFullscreenControllerForView(
       ImmersiveFullscreenController* immersive_fullscreen_controller);
 
   // Sets the active and inactive frame colors. Note the inactive frame color
   // will have some transparency added when the frame is drawn.
   void SetFrameColors(SkColor active_frame_color, SkColor inactive_frame_color);
 
+  // Sets the height of the header. If |height| has no value (the default), the
+  // preferred height is used.
+  void SetHeaderHeight(base::Optional<int> height);

[James Cook, 2017/03/02 21:00:00]

This is a little confusing to me. It seems like this should take a non-optional
int and only use the preferred size if this method is never called.

It seems weird to me that someone would call SetHeaderHeight(base::nullopt_t) or
other null-int.

[sky, 2017/03/02 21:28:42]

If you go with the int, you have no way to unset. base::optional is designed for
this sort of thing. Otherwise you need an explicit ClearHeaderHeight(), which is
equally ick. We have started to use this pattern in other places, e.g.
https://chromium.googlesource.com/chromium/src/+/master/ui/views/controls/but...
.

[James Cook, 2017/03/02 21:44:24]

Yes, but I don't see you calling SetHeaderHeight() anywhere that unsets the
value.

+
   // views::NonClientFrameView:
   gfx::Rect GetBoundsForClientView() const override;
   gfx::Rect GetWindowBoundsForClientBounds(
       const gfx::Rect& client_bounds) const override;
   int NonClientHitTest(const gfx::Point& point) override;
   void GetWindowMask(const gfx::Size& size, gfx::Path* window_mask) override;
   void ResetWindowControls() override;
   void UpdateWindowIcon() override;
   void UpdateWindowTitle() override;
   void SizeConstraintsChanged() override;
@@ skipping 27 matching lines @@
 
   // Height from top of window to top of client area.
   int NonClientTopBorderHeight() const;
 
   // Not owned.
   views::Widget* frame_;
 
   // View which contains the title and window controls.
   HeaderView* header_view_;
 
+  OverlayView* overlay_view_;
+
   ImmersiveFullscreenControllerDelegate* immersive_delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(CustomFrameViewAsh);
 };
 
 }  // namespace ash
 
 #endif  // ASH_COMMON_FRAME_CUSTOM_FRAME_VIEW_ASH_H_