mash: improves browser frame decorations

ash/common/frame/custom_frame_view_ash.h

Index: ash/common/frame/custom_frame_view_ash.h
diff --git a/ash/common/frame/custom_frame_view_ash.h b/ash/common/frame/custom_frame_view_ash.h
index b2e50bedffb1845f9b4109c5762acb19816ec546..4a4136efdf34ceaef80933de44c69573deab78c7 100644
--- a/ash/common/frame/custom_frame_view_ash.h
+++ b/ash/common/frame/custom_frame_view_ash.h
@@ -8,7 +8,9 @@
 #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"
 
@@ -43,7 +45,8 @@ class ASH_EXPORT CustomFrameViewAsh : public views::NonClientFrameView {
   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
@@ -57,6 +60,10 @@ class ASH_EXPORT CustomFrameViewAsh : public views::NonClientFrameView {
   // 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(
@@ -104,6 +111,8 @@ class ASH_EXPORT CustomFrameViewAsh : public views::NonClientFrameView {
   // View which contains the title and window controls.
   HeaderView* header_view_;
 
+  OverlayView* overlay_view_;
+
   ImmersiveFullscreenControllerDelegate* immersive_delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(CustomFrameViewAsh);