OopAsh: Hide caption buttons for browser windows in tablet mode.

Port a couple related browser tests to ash unit tests.

Change-Id: Id1fcf3b823b3f470dc93d651c681738053cc3383
Reviewed-on: https://chromium-review.googlesource.com/1173152Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583061}

ash/frame/header_view.cc

@@ -240,6 +240,7 @@ void HeaderView::ChildPreferredSizeChanged(views::View* child) {
 }
 
 void HeaderView::OnTabletModeStarted() {
+  UpdateCaptionButtonsVisibility();
   caption_button_container_->UpdateCaptionButtonState(true /*=animate*/);
   parent()->Layout();
   if (target_widget_ &&
@@ -250,6 +251,7 @@ void HeaderView::OnTabletModeStarted() {
 }
 
 void HeaderView::OnTabletModeEnded() {
+  UpdateCaptionButtonsVisibility();
   caption_button_container_->UpdateCaptionButtonState(true /*=animate*/);
   parent()->Layout();
   if (target_widget_)
@@ -297,7 +299,7 @@ void HeaderView::SetShouldPaintHeader(bool paint) {
     return;
 
   should_paint_ = paint;
-  caption_button_container_->SetVisible(should_paint_);
+  UpdateCaptionButtonsVisibility();
   SchedulePaint();
 }
 
@@ -389,4 +391,16 @@ void HeaderView::UpdateBackButton() {
   }
 }
 
+void HeaderView::UpdateCaptionButtonsVisibility() {
+  if (!target_widget_)
+    return;
+
+  caption_button_container_->SetVisible(
+      should_paint_ && !(Shell::Get()
+                             ->tablet_mode_controller()
+                             ->IsTabletModeWindowManagerEnabled() &&
+                         target_widget_->GetNativeWindow()->GetProperty(
+                             ash::kHideCaptionButtonsInTabletModeKey)));
+}
+
 }  // namespace ash

ash/frame/header_view.h

@@ -126,6 +126,7 @@ class ASH_EXPORT HeaderView : public views::View,
   void PaintHeaderContent(gfx::Canvas* canvas);
 
   void UpdateBackButton();
+  void UpdateCaptionButtonsVisibility();
 
   // The widget that the caption buttons act on.
   views::Widget* target_widget_;

ash/frame/non_client_frame_view_ash_unittest.cc

@@ -12,7 +12,9 @@
 #include "ash/frame/default_frame_header.h"
 #include "ash/frame/header_view.h"
 #include "ash/frame/wide_frame_view.h"
+#include "ash/public/cpp/app_list/app_list_features.h"
 #include "ash/public/cpp/ash_layout_constants.h"
+#include "ash/public/cpp/ash_switches.h"
 #include "ash/public/cpp/immersive/immersive_fullscreen_controller.h"
 #include "ash/public/cpp/immersive/immersive_fullscreen_controller_test_api.h"
 #include "ash/public/cpp/vector_icons/vector_icons.h"
@@ -25,7 +27,9 @@
 #include "ash/wm/window_state.h"
 #include "ash/wm/window_state_delegate.h"
 #include "ash/wm/wm_event.h"
+#include "base/command_line.h"
 #include "base/containers/flat_set.h"
+#include "base/test/scoped_feature_list.h"
 #include "services/ui/public/interfaces/window_tree_constants.mojom.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/aura/window.h"
@@ -948,4 +952,81 @@ TEST_P(NonClientFrameViewAshFrameColorTest, WideFrameInitialColor) {
 // Run frame color tests with and without custom wm::WindowStateDelegate.
 INSTANTIATE_TEST_CASE_P(, NonClientFrameViewAshFrameColorTest, testing::Bool());
 
+class HomeLauncherNonClientFrameViewAshTest : public AshTestBase {
+ public:
+  HomeLauncherNonClientFrameViewAshTest() = default;
+  ~HomeLauncherNonClientFrameViewAshTest() override = default;
+
+  void SetUp() override {
+    scoped_feature_list_.InitAndEnableFeature(
+        app_list::features::kEnableHomeLauncher);
+    AshTestBase::SetUp();
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+
+  DISALLOW_COPY_AND_ASSIGN(HomeLauncherNonClientFrameViewAshTest);
+};
+
+// Tests the visibility of the caption button container when
+// kHideCaptionButtonsInTabletModeKey is set.
+TEST_F(HomeLauncherNonClientFrameViewAshTest,
+       TabletModeBrowserCaptionButtonVisibility) {
+  auto* delegate = new NonClientFrameViewAshTestWidgetDelegate();
+  std::unique_ptr<views::Widget> widget = CreateTestWidget(
+      delegate, kShellWindowId_DefaultContainer, gfx::Rect(100, 0, 400, 500));
+  widget->GetNativeWindow()->SetProperty(kHideCaptionButtonsInTabletModeKey,
+                                         true);
+
+  FrameCaptionButtonContainerView* caption_buttons =
+      delegate->non_client_frame_view()
+          ->GetHeaderView()
+          ->caption_button_container();
+
+  EXPECT_TRUE(caption_buttons->visible());
+  ash::Shell* shell = ash::Shell::Get();
+  ash::TabletModeController* tablet_mode_controller =
+      shell->tablet_mode_controller();
+  tablet_mode_controller->EnableTabletModeWindowManager(true);
+  EXPECT_FALSE(caption_buttons->visible());
+
+  shell->window_selector_controller()->ToggleOverview();
+  EXPECT_FALSE(caption_buttons->visible());
+  shell->window_selector_controller()->ToggleOverview();
+  EXPECT_FALSE(caption_buttons->visible());
+
+  tablet_mode_controller->EnableTabletModeWindowManager(false);
+  EXPECT_TRUE(caption_buttons->visible());
+}
+
+// Tests the visibility of the caption button container when
+// kHideCaptionButtonsInTabletModeKey is not set.
+TEST_F(HomeLauncherNonClientFrameViewAshTest,
+       TabletModeAppCaptionButtonVisibility) {
+  auto* delegate = new NonClientFrameViewAshTestWidgetDelegate();
+  std::unique_ptr<views::Widget> widget = CreateTestWidget(
+      delegate, kShellWindowId_DefaultContainer, gfx::Rect(100, 0, 400, 500));
+
+  FrameCaptionButtonContainerView* caption_buttons =
+      delegate->non_client_frame_view()
+          ->GetHeaderView()
+          ->caption_button_container();
+
+  EXPECT_TRUE(caption_buttons->visible());
+  ash::Shell* shell = ash::Shell::Get();
+  ash::TabletModeController* tablet_mode_controller =
+      shell->tablet_mode_controller();
+  tablet_mode_controller->EnableTabletModeWindowManager(true);
+  EXPECT_TRUE(caption_buttons->visible());
+
+  shell->window_selector_controller()->ToggleOverview();
+  EXPECT_FALSE(caption_buttons->visible());
+  shell->window_selector_controller()->ToggleOverview();
+  EXPECT_TRUE(caption_buttons->visible());
+
+  tablet_mode_controller->EnableTabletModeWindowManager(false);
+  EXPECT_TRUE(caption_buttons->visible());
+}
+
 }  // namespace ash

ash/public/cpp/mus_property_mirror_ash.cc

@@ -96,6 +96,10 @@ void MusPropertyMirrorAsh::MirrorPropertyFromWidgetWindowToRootWindow(
   } else if (key == kFrameTextColorKey) {
     root_window->SetProperty(kFrameTextColorKey,
                              window->GetProperty(kFrameTextColorKey));
+  } else if (key == kHideCaptionButtonsInTabletModeKey) {
+    root_window->SetProperty(
+        kHideCaptionButtonsInTabletModeKey,
+        window->GetProperty(kHideCaptionButtonsInTabletModeKey));
   }
 }
 

ash/public/cpp/window_properties.cc

@@ -60,6 +60,10 @@ void RegisterWindowProperties(aura::PropertyConverter* property_converter) {
   property_converter->RegisterPrimitiveProperty(
       kFrameImageYInsetKey, mojom::kFrameImageYInset_Property,
       aura::PropertyConverter::CreateAcceptAnyValueCallback());
+  property_converter->RegisterPrimitiveProperty(
+      kHideCaptionButtonsInTabletModeKey,
+      mojom::kHideCaptionButtonsInTabletMode_Property,
+      aura::PropertyConverter::CreateAcceptAnyValueCallback());
   property_converter->RegisterPrimitiveProperty(
       kFrameInactiveColorKey,
       ui::mojom::WindowManager::kFrameInactiveColor_Property,
@@ -129,6 +133,7 @@ DEFINE_OWNED_UI_CLASS_PROPERTY_KEY(base::UnguessableToken,
                                    kFrameImageOverlayInactiveKey,
                                    nullptr);
 DEFINE_UI_CLASS_PROPERTY_KEY(int, kFrameImageYInsetKey, 0);
+DEFINE_UI_CLASS_PROPERTY_KEY(bool, kHideCaptionButtonsInTabletModeKey, false);
 DEFINE_UI_CLASS_PROPERTY_KEY(bool, kHideInOverviewKey, false);
 DEFINE_UI_CLASS_PROPERTY_KEY(bool, kHideShelfWhenFullscreenKey, true);
 DEFINE_UI_CLASS_PROPERTY_KEY(bool,

ash/public/cpp/window_properties.h

@@ -100,6 +100,11 @@ ASH_PUBLIC_EXPORT extern const aura::WindowProperty<
 ASH_PUBLIC_EXPORT extern const aura::WindowProperty<int>* const
     kFrameImageYInsetKey;
 
+// A property to control the visibility of the frame captions buttons when in
+// tablet mode (when not in tablet mode, this property is ignored).
+ASH_PUBLIC_EXPORT extern const aura::WindowProperty<bool>* const
+    kHideCaptionButtonsInTabletModeKey;
+
 // A property key to indicate whether we should hide this window in overview
 // mode and Alt + Tab.
 ASH_PUBLIC_EXPORT extern const aura::WindowProperty<bool>* const

ash/public/interfaces/window_properties.mojom

@@ -42,6 +42,10 @@ const string kFrameIsThemedByHostedApp_Property =
 // for tabless browser windows.
 const string kFrameTextColor_Property = "ash:frame-text-color";
 
+// See ash::kHideCaptionButtonsInTabletModeKey.
+const string kHideCaptionButtonsInTabletMode_Property =
+    "ash:hide-caption-buttons-in-tablet-mode";
+
 // True if the shelf should be hidden when this window is put into fullscreen.
 // Exposed because some windows want to explicitly opt-out of this.
 const string kHideShelfWhenFullscreen_Property =

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

@@ -126,7 +126,7 @@ BrowserNonClientFrameViewAsh::BrowserNonClientFrameViewAsh(
   ash::Shell::Get()->AddShellObserver(this);
 
   // The ServiceManagerConnection may be nullptr in tests.
-  if (content::ServiceManagerConnection::GetForProcess()) {
+  if (content::ServiceManagerConnection::GetForProcess() && !IsMash()) {
     content::ServiceManagerConnection::GetForProcess()
         ->GetConnector()
         ->BindInterface(ash::mojom::kServiceName, &split_view_controller_);
@@ -189,6 +189,17 @@ void BrowserNonClientFrameViewAsh::Init() {
       static_cast<int>(browser->is_app() ? ash::AppType::CHROME_APP
                                          : ash::AppType::BROWSER));
 
+  // In tablet mode, to prevent accidental taps of the window controls, and to
+  // give more horizontal space for tabs and the new tab button especially in
+  // splitscreen view, we hide the window controls. We only do this when the
+  // Home Launcher feature is enabled, since it gives the user the ability to
+  // minimize all windows when pressing the Launcher button on the shelf.
+  window->SetProperty(
+      ash::kHideCaptionButtonsInTabletModeKey,
+      (browser->is_app() || !app_list::features::IsHomeLauncherEnabled())
+          ? false
+          : true);
+
   window_observer_.Add(IsMash() ? window->GetRootWindow() : window);
 
   // To preserve privacy, tag incognito windows so that they won't be included
@@ -676,6 +687,7 @@ gfx::ImageSkia BrowserNonClientFrameViewAsh::GetFrameHeaderOverlayImage(
 }
 
 bool BrowserNonClientFrameViewAsh::IsTabletMode() const {
+  DCHECK(!IsMash());
   return TabletModeClient::Get() &&
          TabletModeClient::Get()->tablet_mode_enabled();
 }
@@ -874,13 +886,9 @@ AvatarButtonStyle BrowserNonClientFrameViewAsh::GetAvatarButtonStyle() const {
 bool BrowserNonClientFrameViewAsh::ShouldShowCaptionButtons() const {
   DCHECK(!IsMash());
 
-  // In tablet mode, to prevent accidental taps of the window controls, and to
-  // give more horizontal space for tabs and the new tab button especially in
-  // splitscreen view, we hide the window controls. We only do this when the
-  // Home Launcher feature is enabled, since it gives the user the ability to
-  // minimize all windows when pressing the Launcher button on the shelf.
-  if (app_list::features::IsHomeLauncherEnabled() && IsTabletMode() &&
-      !browser_view()->browser()->is_app()) {
+  if (frame()->GetNativeWindow()->GetProperty(
+          ash::kHideCaptionButtonsInTabletModeKey) &&
+      IsTabletMode()) {
     return false;
   }
 
@@ -919,6 +927,8 @@ bool BrowserNonClientFrameViewAsh::ShouldPaint() const {
 }
 
 void BrowserNonClientFrameViewAsh::OnOverviewOrSplitviewModeChanged() {
+  // Not necessary as NonClientFrameViewAsh handles the caption button
+  // visibility logic in Mash.
   DCHECK(!IsMash());
 
   caption_button_container_->SetVisible(ShouldShowCaptionButtons());

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

@@ -1304,6 +1304,10 @@ class NonHomeLauncherBrowserNonClientFrameViewAshTest
 
 IN_PROC_BROWSER_TEST_P(HomeLauncherBrowserNonClientFrameViewAshTest,
                        TabletModeBrowserCaptionButtonVisibility) {
+  // For OopAsh, this is tested by an ash unit test of the same name.
+  if (!features::IsAshInBrowserProcess())
+    return;
+
   BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser());
   BrowserNonClientFrameViewAsh* frame_view = GetFrameViewAsh(browser_view);
 
@@ -1327,6 +1331,10 @@ IN_PROC_BROWSER_TEST_P(HomeLauncherBrowserNonClientFrameViewAshTest,
 
 IN_PROC_BROWSER_TEST_P(HomeLauncherBrowserNonClientFrameViewAshTest,
                        TabletModeAppCaptionButtonVisibility) {
+  // For OopAsh, this is tested by an ash unit test of the same name.
+  if (!features::IsAshInBrowserProcess())
+    return;
+
   browser()->window()->Close();
 
   // Open a new app window.

testing/buildbot/filters/mash.browser_tests.filter

@@ -41,7 +41,6 @@
 -BrowserNonClientFrameViewAshTest.TopViewInset/*
 
 # Direct access to ash window frames, tablet mode, overview mode, etc.
--HomeLauncherBrowserNonClientFrameViewAshTest.*
 -HostedAppNonClientFrameViewAshTest.*
 -NonHomeLauncherBrowserNonClientFrameViewAshTest.*