ws: Wire up property for showing top-level window titles

Plumb views::WidgetDelegate::ShouldShowWindowTitle to the ws host frame.
Move prop to aura/client for MusClient and DesktopWindowTreeHostMus.
Make the KSV's delegate return false for ShouldShowWindowTitle.

Based on Scott's CL (PS1 matches crrev.com/c/1112587 PS4)

BUG=854324
TEST=covered by tests; No title in Chrome OS KSV (Ctrl-Alt-/)

Change-Id: I190c8245edbb310a1f03f6ec6ad318b325be04bc
Reviewed-on: https://chromium-review.googlesource.com/1113938
Commit-Queue: Michael Wasserman <msw@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571300}

ash/components/shortcut_viewer/views/keyboard_shortcut_view.cc

@@ -447,6 +447,10 @@ bool KeyboardShortcutView::CanResize() const {
   return false;
 }
 
+bool KeyboardShortcutView::ShouldShowWindowTitle() const {
+  return false;
+}
+
 views::ClientView* KeyboardShortcutView::CreateClientView(
     views::Widget* widget) {
   return new views::ClientView(widget, this);

ash/components/shortcut_viewer/views/keyboard_shortcut_view.h

@@ -74,6 +74,7 @@ class KeyboardShortcutView : public views::WidgetDelegateView,
   bool CanMaximize() const override;
   bool CanMinimize() const override;
   bool CanResize() const override;
+  bool ShouldShowWindowTitle() const override;
   views::ClientView* CreateClientView(views::Widget* widget) override;
 
   static KeyboardShortcutView* GetInstanceForTesting();

ash/public/cpp/mus_property_mirror_ash.cc

@@ -61,6 +61,9 @@ void MusPropertyMirrorAsh::MirrorPropertyFromWidgetWindowToRootWindow(
     MirrorOwnedProperty(window, root_window, aura::client::kMinimumSize);
   } else if (key == aura::client::kTitleKey) {
     MirrorOwnedProperty(window, root_window, aura::client::kTitleKey);
+  } else if (key == aura::client::kTitleShownKey) {
+    root_window->SetProperty(aura::client::kTitleShownKey,
+                             window->GetProperty(aura::client::kTitleShownKey));
   } else if (key == aura::client::kWindowIconKey) {
     MirrorOwnedProperty(window, root_window, aura::client::kWindowIconKey);
   } else if (key == kFrameBackButtonStateKey) {
@@ -80,9 +83,6 @@ void MusPropertyMirrorAsh::MirrorPropertyFromWidgetWindowToRootWindow(
     MirrorOwnedProperty(window, root_window, kFrameImageOverlayActiveKey);
   } else if (key == kFrameImageOverlayInactiveKey) {
     MirrorOwnedProperty(window, root_window, kFrameImageOverlayInactiveKey);
-  } else if (key == kWindowTitleShownKey) {
-    root_window->SetProperty(kWindowTitleShownKey,
-                             window->GetProperty(kWindowTitleShownKey));
   }
 }
 

ash/public/cpp/window_properties.cc

@@ -13,6 +13,7 @@
 #include "ash/public/interfaces/window_state_type.mojom.h"
 #include "base/unguessable_token.h"
 #include "services/ui/public/interfaces/window_manager.mojom.h"
+#include "ui/aura/client/aura_constants.h"
 #include "ui/aura/mus/property_converter.h"
 #include "ui/aura/window.h"
 #include "ui/gfx/geometry/rect.h"
@@ -94,7 +95,7 @@ void RegisterWindowProperties(aura::PropertyConverter* property_converter) {
       mojom::kRestoreWindowStateTypeOverride_Property,
       base::BindRepeating(&IsValidWindowStateType));
   property_converter->RegisterPrimitiveProperty(
-      kWindowTitleShownKey,
+      aura::client::kTitleShownKey,
       ui::mojom::WindowManager::kWindowTitleShown_Property,
       aura::PropertyConverter::CreateAcceptAnyValueCallback());
 }
@@ -151,6 +152,5 @@ DEFINE_UI_CLASS_PROPERTY_KEY(bool, kWindowPositionManagedTypeKey, false);
 DEFINE_UI_CLASS_PROPERTY_KEY(mojom::WindowStateType,
                              kWindowStateTypeKey,
                              mojom::WindowStateType::DEFAULT);
-DEFINE_UI_CLASS_PROPERTY_KEY(bool, kWindowTitleShownKey, true);
 
 }  // namespace ash

ash/public/cpp/window_properties.h

@@ -172,11 +172,6 @@ ASH_PUBLIC_EXPORT extern const aura::WindowProperty<bool>* const
 ASH_PUBLIC_EXPORT extern const aura::WindowProperty<
     mojom::WindowStateType>* const kWindowStateTypeKey;
 
-// Determines whether the window title should be drawn. For example, app and
-// non-tabbed, trusted source windows (such as Settings) will not show a title.
-ASH_PUBLIC_EXPORT extern const aura::WindowProperty<bool>* const
-    kWindowTitleShownKey;
-
 // Alphabetical sort.
 
 }  // namespace ash

ash/wm/non_client_frame_controller.cc

@@ -400,10 +400,10 @@ NonClientFrameController::~NonClientFrameController() {
 }
 
 base::string16 NonClientFrameController::GetWindowTitle() const {
-  if (!window_ || !window_->GetProperty(aura::client::kTitleKey))
+  if (!window_)
     return base::string16();
 
-  base::string16 title = *window_->GetProperty(aura::client::kTitleKey);
+  base::string16 title = window_->GetTitle();
 
   if (window_->GetProperty(kWindowIsJanky))
     title += base::ASCIIToUTF16(" !! Not responding !!");
@@ -433,7 +433,7 @@ bool NonClientFrameController::CanActivate() const {
 }
 
 bool NonClientFrameController::ShouldShowWindowTitle() const {
-  return window_ && window_->GetProperty(kWindowTitleShownKey);
+  return window_ && window_->GetProperty(aura::client::kTitleShownKey);
 }
 
 views::ClientView* NonClientFrameController::CreateClientView(
@@ -450,13 +450,12 @@ void NonClientFrameController::OnWindowPropertyChanged(aura::Window* window,
   if (!did_init_native_widget_)
     return;
 
-  if (key == kWindowIsJanky) {
+  if (key == kWindowIsJanky || key == aura::client::kTitleKey ||
+      key == aura::client::kTitleShownKey) {
     widget_->UpdateWindowTitle();
     widget_->non_client_view()->frame_view()->SchedulePaint();
   } else if (key == aura::client::kResizeBehaviorKey) {
     widget_->OnSizeConstraintsChanged();
-  } else if (key == aura::client::kTitleKey) {
-    widget_->UpdateWindowTitle();
   }
 }
 

ash/wm/non_client_frame_controller_unittest.cc

@@ -12,6 +12,7 @@
 #include "ash/window_manager.h"
 #include "ash/window_manager_service.h"
 #include "ash/wm/top_level_window_factory.h"
+#include "base/strings/utf_string_conversions.h"
 #include "cc/base/math_util.h"
 #include "cc/trees/layer_tree_settings.h"
 #include "components/viz/common/quads/compositor_frame.h"
@@ -19,6 +20,7 @@
 #include "services/ui/public/interfaces/window_tree_constants.mojom.h"
 #include "services/ui/ws2/test_change_tracker.h"
 #include "services/ui/ws2/test_window_tree_client.h"
+#include "ui/aura/client/aura_constants.h"
 #include "ui/aura/env.h"
 #include "ui/aura/window.h"
 #include "ui/compositor/compositor.h"
@@ -206,4 +208,24 @@ TEST_F(NonClientFrameControllerTest, CallsRequestClose) {
   EXPECT_EQ("RequestClose", ui::ws2::ChangeToDescription(changes->back()));
 }
 
+TEST_F(NonClientFrameControllerTest, WindowTitle) {
+  std::unique_ptr<aura::Window> window = CreateTestWindow();
+  NonClientFrameController* non_client_frame_controller =
+      NonClientFrameController::Get(window.get());
+  ASSERT_TRUE(non_client_frame_controller);
+  views::WidgetDelegate* widget_delegate =
+      static_cast<views::WidgetDelegate*>(non_client_frame_controller);
+  EXPECT_TRUE(widget_delegate->ShouldShowWindowTitle());
+  EXPECT_TRUE(widget_delegate->GetWindowTitle().empty());
+
+  // Verify GetWindowTitle() mirrors window->SetTitle().
+  const base::string16 title = base::ASCIIToUTF16("X");
+  window->SetTitle(title);
+  EXPECT_EQ(title, widget_delegate->GetWindowTitle());
+
+  // ShouldShowWindowTitle() mirrors |aura::client::kTitleShownKey|.
+  window->SetProperty(aura::client::kTitleShownKey, false);
+  EXPECT_FALSE(widget_delegate->ShouldShowWindowTitle());
+}
+
 }  // namespace ash

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

@@ -65,9 +65,6 @@ views::Widget::InitParams BrowserFrameMash::GetWidgetParams() {
   properties[ui::mojom::WindowManager::kShelfItemType_Property] =
       mojo::ConvertTo<std::vector<uint8_t>>(
           static_cast<int64_t>(ash::TYPE_BROWSER_SHORTCUT));
-  properties[ui::mojom::WindowManager::kWindowTitleShown_Property] =
-      mojo::ConvertTo<std::vector<uint8_t>>(
-          static_cast<int64_t>(browser_view_->ShouldShowWindowTitle()));
 
   // TODO(estade): to match classic Ash, this property should be toggled to true
   // for non-popups after the window is initially shown.

ui/aura/client/aura_constants.cc

@@ -71,6 +71,7 @@ DEFINE_OWNED_UI_CLASS_PROPERTY_KEY(gfx::Rect, kRestoreBoundsKey, nullptr);
 DEFINE_UI_CLASS_PROPERTY_KEY(
     ui::WindowShowState, kShowStateKey, ui::SHOW_STATE_DEFAULT);
 DEFINE_OWNED_UI_CLASS_PROPERTY_KEY(base::string16, kTitleKey, nullptr);
+DEFINE_UI_CLASS_PROPERTY_KEY(bool, kTitleShownKey, true);
 DEFINE_UI_CLASS_PROPERTY_KEY(int, kTopViewInset, 0);
 DEFINE_UI_CLASS_PROPERTY_KEY(SkColor, kTopViewColor, SK_ColorTRANSPARENT);
 DEFINE_OWNED_UI_CLASS_PROPERTY_KEY(gfx::ImageSkia, kWindowIconKey, nullptr);

ui/aura/client/aura_constants.h

@@ -138,6 +138,10 @@ AURA_EXPORT extern const WindowProperty<ui::WindowShowState>* const
 // A property key to store the title of the window; sometimes shown to users.
 AURA_EXPORT extern const WindowProperty<base::string16*>* const kTitleKey;
 
+// Indicates if the title of the window should be shown. This is only used for
+// top-levels that show a title. Default is false.
+AURA_EXPORT extern const WindowProperty<bool>* const kTitleShownKey;
+
 // The inset of the topmost view in the client view from the top of the
 // non-client view. The topmost view depends on the window type. The topmost
 // view is the tab strip for tabbed browser windows, the toolbar for popups,

ui/views/bubble/bubble_frame_view.cc

@@ -268,7 +268,8 @@ void BubbleFrameView::UpdateWindowIcon() {
 void BubbleFrameView::UpdateWindowTitle() {
   if (default_title_) {
     const WidgetDelegate* delegate = GetWidget()->widget_delegate();
-    default_title_->SetVisible(delegate->ShouldShowWindowTitle());
+    default_title_->SetVisible(delegate->ShouldShowWindowTitle() &&
+                               !delegate->GetWindowTitle().empty());
     default_title_->SetText(delegate->GetWindowTitle());
   }  // custom_title_'s updates are handled by its creator.
 }
@@ -408,6 +409,9 @@ void BubbleFrameView::OnNativeThemeChanged(const ui::NativeTheme* theme) {
 
 void BubbleFrameView::ViewHierarchyChanged(
     const ViewHierarchyChangedDetails& details) {
+  if (details.is_add && details.child == this)
+    OnThemeChanged();
+
   if (!details.is_add && details.parent == footnote_container_ &&
       footnote_container_->child_count() == 1 &&
       details.child == footnote_container_->child_at(0)) {

ui/views/mus/desktop_window_tree_host_mus.cc

@@ -637,8 +637,14 @@ bool DesktopWindowTreeHostMus::IsVisibleOnAllWorkspaces() const {
 }
 
 bool DesktopWindowTreeHostMus::SetWindowTitle(const base::string16& title) {
-  if (window()->GetTitle() == title)
+  WidgetDelegate* widget_delegate =
+      native_widget_delegate_->AsWidget()->widget_delegate();
+  const bool show = widget_delegate && widget_delegate->ShouldShowWindowTitle();
+  if (window()->GetTitle() == title &&
+      window()->GetProperty(aura::client::kTitleShownKey) == show) {
     return false;
+  }
+  window()->SetProperty(aura::client::kTitleShownKey, show);
   window()->SetTitle(title);
   return true;
 }

ui/views/mus/desktop_window_tree_host_mus_unittest.cc

@@ -4,9 +4,7 @@
 
 #include "ui/views/mus/desktop_window_tree_host_mus.h"
 
-#include "base/debug/stack_trace.h"
-#include "base/run_loop.h"
-
+#include "base/strings/utf_string_conversions.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/aura/client/cursor_client.h"
 #include "ui/aura/client/focus_client.h"
@@ -396,4 +394,57 @@ TEST_F(DesktopWindowTreeHostMusTest, GetWindowBoundsInScreen) {
   EXPECT_EQ(gfx::Rect(800, 0, 100, 100), widget2.GetWindowBoundsInScreen());
 }
 
+// WidgetDelegate implementation that allows setting window-title and whether
+// the title should be shown.
+class WindowTitleWidgetDelegate : public WidgetDelegateView {
+ public:
+  WindowTitleWidgetDelegate() = default;
+  ~WindowTitleWidgetDelegate() override = default;
+
+  void set_window_title(const base::string16& title) { window_title_ = title; }
+  void set_should_show_window_title(bool value) {
+    should_show_window_title_ = value;
+  }
+
+  // WidgetDelegateView:
+  base::string16 GetWindowTitle() const override { return window_title_; }
+  bool ShouldShowWindowTitle() const override {
+    return should_show_window_title_;
+  }
+
+ private:
+  base::string16 window_title_;
+  bool should_show_window_title_ = true;
+
+  DISALLOW_COPY_AND_ASSIGN(WindowTitleWidgetDelegate);
+};
+
+TEST_F(DesktopWindowTreeHostMusTest, WindowTitle) {
+  // Owned by |widget|.
+  WindowTitleWidgetDelegate* delegate = new WindowTitleWidgetDelegate();
+  std::unique_ptr<Widget> widget(CreateWidget(delegate));
+  aura::Window* window = widget->GetNativeWindow()->GetRootWindow();
+
+  // Set the title in the delegate and verify it propagates.
+  const base::string16 title1 = base::ASCIIToUTF16("X");
+  delegate->set_window_title(title1);
+  widget->UpdateWindowTitle();
+  EXPECT_TRUE(window->GetProperty(aura::client::kTitleShownKey));
+  EXPECT_EQ(title1, window->GetTitle());
+
+  // Hiding the title should not change the title.
+  delegate->set_should_show_window_title(false);
+  widget->UpdateWindowTitle();
+  EXPECT_FALSE(window->GetProperty(aura::client::kTitleShownKey));
+  EXPECT_EQ(title1, window->GetTitle());
+
+  // Show the title again with a different value.
+  delegate->set_should_show_window_title(true);
+  const base::string16 title2 = base::ASCIIToUTF16("Z");
+  delegate->set_window_title(title2);
+  widget->UpdateWindowTitle();
+  EXPECT_TRUE(window->GetProperty(aura::client::kTitleShownKey));
+  EXPECT_EQ(title2, window->GetTitle());
+}
+
 }  // namespace views

ui/views/mus/mus_client.cc

@@ -241,6 +241,18 @@ MusClient::ConfigurePropertiesFromParams(
               init_params.delegate->GetResizeBehavior()));
     }
 
+    if (init_params.delegate->ShouldShowWindowTitle()) {
+      properties[WindowManager::kWindowTitleShown_Property] =
+          mojo::ConvertTo<TransportType>(static_cast<PrimitiveType>(
+              init_params.delegate->ShouldShowWindowTitle()));
+    }
+
+    if (!init_params.delegate->GetWindowTitle().empty()) {
+      properties[WindowManager::kWindowTitle_Property] =
+          mojo::ConvertTo<TransportType>(
+              init_params.delegate->GetWindowTitle());
+    }
+
     // TODO(crbug.com/667566): Support additional scales or gfx::Image[Skia].
     gfx::ImageSkia app_icon = init_params.delegate->GetWindowAppIcon();
     SkBitmap app_bitmap = app_icon.GetRepresentation(1.f).sk_bitmap();
@@ -248,6 +260,7 @@ MusClient::ConfigurePropertiesFromParams(
       properties[WindowManager::kAppIcon_Property] =
           mojo::ConvertTo<TransportType>(app_bitmap);
     }
+
     // TODO(crbug.com/667566): Support additional scales or gfx::Image[Skia].
     gfx::ImageSkia window_icon = init_params.delegate->GetWindowIcon();
     SkBitmap window_bitmap = window_icon.GetRepresentation(1.f).sk_bitmap();

ui/views/window/dialog_delegate_unittest.cc

@@ -40,9 +40,7 @@ class TestDialog : public DialogDelegateView {
   }
 
   // WidgetDelegate overrides:
-  bool ShouldShowWindowTitle() const override {
-    return !title_.empty();
-  }
+  bool ShouldShowWindowTitle() const override { return !title_.empty(); }
   bool ShouldShowCloseButton() const override { return show_close_button_; }
 
   // DialogDelegateView overrides: