Introduce new WebApp header style for hosted apps and fizzy apps on ash.

chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc

Index: chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
diff --git a/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc b/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
index 885453f40e8fddfdb77c52d1c8043d4f083f113a..8f09addad77f3a93a5b6a29ae2ada5271eee12fe 100644
--- a/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
+++ b/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
@@ -7,14 +7,17 @@
 #include <algorithm>
 
 #include "ash/ash_switches.h"
+#include "ash/frame/caption_buttons/frame_caption_button.h"
 #include "ash/frame/caption_buttons/frame_caption_button_container_view.h"
 #include "ash/frame/default_header_painter.h"
 #include "ash/frame/frame_border_hit_test_controller.h"
 #include "ash/frame/header_painter_util.h"
 #include "ash/shell.h"
 #include "base/command_line.h"
+#include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/themes/theme_properties.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/views/frame/browser_frame.h"
 #include "chrome/browser/ui/views/frame/browser_header_painter_ash.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
@@ -69,6 +72,21 @@ const int kTabstripTopSpacingShort = 0;
 // to hit easily.
 const int kTabShadowHeight = 4;
 
+// Combines View::ConvertPointToTarget() and View::HitTest() for a given
+// |point|.
+// Converts |point| from |src| to |dst| and hit tests it against |dst|.

[James Cook, 2014/08/15 00:00:47]

wrap comment together

[benwells, 2014/08/18 05:59:13]

Done.

+bool ConvertedHitTest(views::View* src,
+                      views::View* dst,
+                      const gfx::Point& point) {
+  if (!dst)
+    return false;
+
+  DCHECK(src);
+  gfx::Point converted_point(point);
+  views::View::ConvertPointToTarget(src, dst, &converted_point);
+  return dst->HitTestPoint(converted_point);
+}
+
 }  // namespace
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -79,9 +97,11 @@ const char BrowserNonClientFrameViewAsh::kViewClassName[] =
     "BrowserNonClientFrameViewAsh";
 
 BrowserNonClientFrameViewAsh::BrowserNonClientFrameViewAsh(
-    BrowserFrame* frame, BrowserView* browser_view)
+    BrowserFrame* frame,
+    BrowserView* browser_view)
     : BrowserNonClientFrameView(frame, browser_view),
       caption_button_container_(NULL),
+      web_app_back_button_(NULL),
       window_icon_(NULL),
       frame_border_hit_test_controller_(
           new ash::FrameBorderHitTestController(frame)) {
@@ -90,6 +110,7 @@ BrowserNonClientFrameViewAsh::BrowserNonClientFrameViewAsh(
 
 BrowserNonClientFrameViewAsh::~BrowserNonClientFrameViewAsh() {
   ash::Shell::GetInstance()->RemoveShellObserver(this);
+  chrome::RemoveCommandObserver(browser_view()->browser(), IDC_BACK, this);

[James Cook, 2014/08/15 00:00:47]

You might comment here on the memory ownership (why browser_view() outlives
|this|).  I had to add logging to my build to satisfy myself that the lifetimes
were correct.

[benwells, 2014/08/18 05:59:13]

Done.

 }
 
 void BrowserNonClientFrameViewAsh::Init() {
@@ -113,8 +134,28 @@ void BrowserNonClientFrameViewAsh::Init() {
   if (UsePackagedAppHeaderStyle()) {
     ash::DefaultHeaderPainter* header_painter = new ash::DefaultHeaderPainter;
     header_painter_.reset(header_painter);
-    header_painter->Init(frame(), this, window_icon_,
-        caption_button_container_);
+    header_painter->Init(frame(), this, caption_button_container_);
+    if (window_icon_) {
+      header_painter->UpdateLeftHeaderView(window_icon_);
+    }
+  } else if (UseWebAppHeaderStyle()) {
+    web_app_back_button_ =
+        new ash::FrameCaptionButton(this, ash::CAPTION_BUTTON_ICON_BACK);
+    web_app_back_button_->SetImages(ash::CAPTION_BUTTON_ICON_BACK,
+                                    ash::FrameCaptionButton::ANIMATE_NO,
+                                    IDR_AURA_WINDOW_CONTROL_ICON_BACK,
+                                    IDR_AURA_WINDOW_CONTROL_ICON_BACK_I,
+                                    IDR_AURA_WINDOW_CONTROL_BACKGROUND_H,
+                                    IDR_AURA_WINDOW_CONTROL_BACKGROUND_P);
+
+    UpdateBackButtonState(true);
+    chrome::AddCommandObserver(browser_view()->browser(), IDC_BACK, this);
+    AddChildView(web_app_back_button_);
+
+    ash::DefaultHeaderPainter* header_painter = new ash::DefaultHeaderPainter;
+    header_painter_.reset(header_painter);

[James Cook, 2014/08/15 00:00:47]

Collapse this with the line above and use header_painter_->foo() below.  Feel
free to do the same below.

[benwells, 2014/08/18 05:59:13]

Init and UpdateLeftHeaderView are not on the HeaderPainter interface, which is
why these variables are typed. Note the signatures of DefaultHeaderPainter and
BrowserHeaderPainterAsh are also different (3 args versus 5).

+    header_painter->Init(frame(), this, caption_button_container_);
+    header_painter->UpdateLeftHeaderView(web_app_back_button_);
   } else {
     BrowserHeaderPainterAsh* header_painter = new BrowserHeaderPainterAsh;
     header_painter_.reset(header_painter);
@@ -155,7 +196,7 @@ int BrowserNonClientFrameViewAsh::GetTopInset() const {
       return kTabstripTopSpacingTall;
   }
 
-  if (UsePackagedAppHeaderStyle())
+  if (UsePackagedAppHeaderStyle() || UseWebAppHeaderStyle())
     return header_painter_->GetHeaderHeightForPainting();
 
   int caption_buttons_bottom = caption_button_container_->bounds().bottom();
@@ -199,11 +240,12 @@ int BrowserNonClientFrameViewAsh::NonClientHitTest(const gfx::Point& point) {
       caption_button_container_, point);
 
   // See if the point is actually within the avatar menu button or within
-  // the avatar label.
-  if (hit_test == HTCAPTION && ((avatar_button() &&
-       avatar_button()->GetMirroredBounds().Contains(point)) ||
-      (avatar_label() && avatar_label()->GetMirroredBounds().Contains(point))))
-      return HTCLIENT;
+  // the web app back button.
+  if (hit_test == HTCAPTION &&
+      (ConvertedHitTest(this, avatar_button(), point) ||
+       ConvertedHitTest(this, web_app_back_button_, point))) {

[James Cook, 2014/08/15 00:00:47]

I would prefer to see this unrolled as two hit tests, skipping the second one if
the web app back button doesn't exist.  Otherwise I have to know that it's OK to
hit test against NULL.

This would also let you DCHECK(src), DCHECK(dst) in the ConvertedHitTest()
function.

[benwells, 2014/08/18 05:59:13]

Done.

+    return HTCLIENT;
+  }
 
   // When the window is restored we want a large click target above the tabs
   // to drag the window, so redirect clicks in the tab's shadow to caption.
@@ -258,13 +300,20 @@ void BrowserNonClientFrameViewAsh::OnPaint(gfx::Canvas* canvas) {
   }
 
   caption_button_container_->SetPaintAsActive(ShouldPaintAsActive());
+  if (web_app_back_button_) {
+    // TODO(benwells): Check that the disabled and inactive states should be
+    // drawn in the same way.
+    web_app_back_button_->set_paint_as_active(
+        ShouldPaintAsActive() &&
+        chrome::IsCommandEnabled(browser_view()->browser(), IDC_BACK));
+  }
 
   ash::HeaderPainter::Mode header_mode = ShouldPaintAsActive() ?
       ash::HeaderPainter::MODE_ACTIVE : ash::HeaderPainter::MODE_INACTIVE;
   header_painter_->PaintHeader(canvas, header_mode);
   if (browser_view()->IsToolbarVisible())
     PaintToolbarBackground(canvas);
-  else if (!UsePackagedAppHeaderStyle())
+  else if (!UsePackagedAppHeaderStyle() && !UseWebAppHeaderStyle())
     PaintContentEdge(canvas);
 }
 
@@ -286,8 +335,13 @@ void BrowserNonClientFrameViewAsh::Layout() {
     painted_height = GetTopInset();
   }
   header_painter_->SetHeaderHeightForPainting(painted_height);
-  if (avatar_button())
+  if (avatar_button()) {
     LayoutAvatar();
+    header_painter_->UpdateLeftViewXInset(avatar_button()->bounds().right());
+  } else {
+    header_painter_->UpdateLeftViewXInset(
+        ash::HeaderPainterUtil::GetDefaultLeftViewXInset());
+  }
   BrowserNonClientFrameView::Layout();
 }
 
@@ -362,6 +416,24 @@ gfx::ImageSkia BrowserNonClientFrameViewAsh::GetFaviconForTabIconView() {
 }
 
 ///////////////////////////////////////////////////////////////////////////////
+// CommandObserver:
+
+void BrowserNonClientFrameViewAsh::EnabledStateChangedForCommand(int id,
+                                                                 bool enabled) {
+  DCHECK_EQ(IDC_BACK, id);
+  UpdateBackButtonState(enabled);
+}
+
+///////////////////////////////////////////////////////////////////////////////
+// views::ButtonListener:
+
+void BrowserNonClientFrameViewAsh::ButtonPressed(views::Button* sender,
+                                                 const ui::Event& event) {
+  DCHECK_EQ(sender, web_app_back_button_);
+  chrome::ExecuteCommand(browser_view()->browser(), IDC_BACK);
+}
+
+///////////////////////////////////////////////////////////////////////////////
 // BrowserNonClientFrameViewAsh, private:
 
 // views::NonClientFrameView:
@@ -414,12 +486,17 @@ bool BrowserNonClientFrameViewAsh::UseImmersiveLightbarHeaderStyle() const {
 }
 
 bool BrowserNonClientFrameViewAsh::UsePackagedAppHeaderStyle() const {
-  // Non streamlined hosted apps do not have a toolbar or tabstrip. Their header
-  // should look the same as the header for packaged apps. Streamlined hosted
-  // apps have a toolbar so should use the browser header style.
+  // Use the packaged app style for apps that aren't using the newer WebApp
+  // style.
+  return browser_view()->browser()->is_app() && !UseWebAppHeaderStyle();
+}
+
+bool BrowserNonClientFrameViewAsh::UseWebAppHeaderStyle() const {
+  // Use of the experimental WebApp header style is guarded with the
+  // streamlined hosted app style.
   return browser_view()->browser()->is_app() &&
-      !CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableStreamlinedHostedApps);
+         CommandLine::ForCurrentProcess()->HasSwitch(
+             switches::kEnableStreamlinedHostedApps);
 }
 
 void BrowserNonClientFrameViewAsh::LayoutAvatar() {
@@ -549,9 +626,14 @@ void BrowserNonClientFrameViewAsh::PaintToolbarBackground(gfx::Canvas* canvas) {
 }
 
 void BrowserNonClientFrameViewAsh::PaintContentEdge(gfx::Canvas* canvas) {
-  DCHECK(!UsePackagedAppHeaderStyle());
+  DCHECK(!UsePackagedAppHeaderStyle() && !UseWebAppHeaderStyle());
   canvas->FillRect(gfx::Rect(0, caption_button_container_->bounds().bottom(),
                              width(), kClientEdgeThickness),
                    ThemeProperties::GetDefaultColor(
                        ThemeProperties::COLOR_TOOLBAR_SEPARATOR));
 }
+
+void BrowserNonClientFrameViewAsh::UpdateBackButtonState(bool enabled) {
+  web_app_back_button_->SetState(enabled ? views::Button::STATE_NORMAL
+                                         : views::Button::STATE_DISABLED);
+}