Pass whether the window can consume system keys as a property.

This replaces a bool on ash::wm::WindowState with an aura::Window property. Bug: 780160 Change-Id: I1537071cb1dcc985bd6757c906dfe3e87acda356 Reviewed-on: https://chromium-review.googlesource.com/752484 Commit-Queue: Elliot Glaysher <erg@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: James Cook <jamescook@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org> Cr-Commit-Position: refs/heads/master@{#515243}
Denger-Network · Nov 9, 2017 · b372ee5 · b372ee5
1 parent f62ed93
commit b372ee5
Show file tree

Hide file tree

Showing 13 changed files with 74 additions and 13 deletions.
diff --git a/ash/accelerators/accelerator_filter_unittest.cc b/ash/accelerators/accelerator_filter_unittest.cc
@@ -115,7 +115,7 @@ TEST_F(AcceleratorFilterTest, CanConsumeSystemKeys) {
 
   // Setting a window property on the target allows system keys to pass through.
   std::unique_ptr<aura::Window> window(CreateTestWindowInShellWithId(1));
-  wm::GetWindowState(window.get())->set_can_consume_system_keys(true);
+  wm::GetWindowState(window.get())->SetCanConsumeSystemKeys(true);
   ui::KeyEvent press_volume_up(ui::ET_KEY_PRESSED, ui::VKEY_VOLUME_UP,
                                ui::EF_NONE);
   ui::Event::DispatcherApi dispatch_helper(&press_volume_up);

diff --git a/ash/accelerators/accelerator_router.cc b/ash/accelerators/accelerator_router.cc
@@ -92,7 +92,7 @@ bool AcceleratorRouter::CanConsumeSystemKeys(aura::Window* target,
   // Uses the top level window so if the target is a web contents window the
   // containing parent window will be checked for the property.
   aura::Window* top_level = ::wm::GetToplevelWindow(target);
-  return top_level && wm::GetWindowState(top_level)->can_consume_system_keys();
+  return top_level && wm::GetWindowState(top_level)->CanConsumeSystemKeys();
 }
 
 bool AcceleratorRouter::ShouldProcessAcceleratorNow(

diff --git a/ash/mus/window_manager.cc b/ash/mus/window_manager.cc
@@ -39,6 +39,7 @@
 #include "ash/wayland/wayland_server_controller.h"
 #include "ash/wm/ash_focus_rules.h"
 #include "ash/wm/tablet_mode/tablet_mode_controller.h"
+#include "ash/wm/window_properties.h"
 #include "ash/wm/window_state.h"
 #include "services/service_manager/public/cpp/connector.h"
 #include "services/ui/common/accelerator_util.h"
@@ -87,6 +88,9 @@ WindowManager::WindowManager(service_manager::Connector* connector,
       show_primary_host_on_connect_(show_primary_host_on_connect),
       wm_state_(std::make_unique<::wm::WMState>()),
       property_converter_(std::make_unique<aura::PropertyConverter>()) {
+  property_converter_->RegisterPrimitiveProperty(
+      kCanConsumeSystemKeysKey, ash::mojom::kCanConsumeSystemKeys_Property,
+      aura::PropertyConverter::CreateAcceptAnyValueCallback());
   property_converter_->RegisterPrimitiveProperty(
       kPanelAttachedKey, ui::mojom::WindowManager::kPanelAttached_Property,
       aura::PropertyConverter::CreateAcceptAnyValueCallback());

diff --git a/ash/mus/window_manager_unittest.cc b/ash/mus/window_manager_unittest.cc
@@ -8,7 +8,9 @@
 
 #include "ash/mus/window_manager.h"
 #include "ash/mus/window_manager_application.h"
+#include "ash/public/cpp/window_properties.h"
 #include "ash/public/interfaces/constants.mojom.h"
+#include "ash/public/interfaces/window_properties.mojom.h"
 #include "ash/session/test_session_controller_client.h"
 #include "ash/shell.h"
 #include "ash/test/ash_test_base.h"
@@ -19,6 +21,7 @@
 #include "components/session_manager/session_manager_types.h"
 #include "services/service_manager/public/cpp/service_test.h"
 #include "services/ui/public/cpp/property_type_converters.h"
+#include "services/ui/public/interfaces/window_manager_constants.mojom.h"
 #include "services/ui/public/interfaces/window_tree.mojom.h"
 #include "ui/aura/env.h"
 #include "ui/aura/mus/property_converter.h"
@@ -149,5 +152,18 @@ TEST_F(WindowManagerTest, SystemModalLockIsntReparented) {
   EXPECT_EQ(kShellWindowId_LockSystemModalContainer, window->parent()->id());
 }
 
+TEST_F(WindowManagerTest, CanConsumeSystemKeysFromContentBrowser) {
+  std::map<std::string, std::vector<uint8_t>> properties;
+  properties[ash::mojom::kCanConsumeSystemKeys_Property] =
+      mojo::ConvertTo<std::vector<uint8_t>>(static_cast<int64_t>(true));
+
+  aura::WindowManagerDelegate* window_manager_delegate =
+      ash_test_helper()->window_manager_app()->window_manager();
+  aura::Window* window = window_manager_delegate->OnWmCreateTopLevelWindow(
+      ui::mojom::WindowType::WINDOW, &properties);
+
+  EXPECT_EQ(true, window->GetProperty(kCanConsumeSystemKeysKey));
+}
+
 }  // namespace mus
 }  // namespace ash
diff --git a/ash/public/cpp/window_properties.cc b/ash/public/cpp/window_properties.cc
@@ -15,6 +15,7 @@ DECLARE_EXPORTED_UI_CLASS_PROPERTY_TYPE(ASH_PUBLIC_EXPORT,
 
 namespace ash {
 
+DEFINE_UI_CLASS_PROPERTY_KEY(bool, kCanConsumeSystemKeysKey, false);
 DEFINE_UI_CLASS_PROPERTY_KEY(bool, kPanelAttachedKey, true);
 DEFINE_OWNED_UI_CLASS_PROPERTY_KEY(std::string, kShelfIDKey, nullptr);
 DEFINE_UI_CLASS_PROPERTY_KEY(int32_t, kShelfItemTypeKey, TYPE_UNDEFINED);

diff --git a/ash/public/cpp/window_properties.h b/ash/public/cpp/window_properties.h
@@ -27,6 +27,10 @@ enum class WindowStateType;
 
 // Alphabetical sort.
 
+// If true, will send system keys to the window for dispatch.
+ASH_PUBLIC_EXPORT extern const aura::WindowProperty<bool>* const
+    kCanConsumeSystemKeysKey;
+
 // If true (and the window is a panel), it's attached to its shelf item.
 ASH_PUBLIC_EXPORT extern const aura::WindowProperty<bool>* const
     kPanelAttachedKey;

diff --git a/ash/public/interfaces/window_properties.mojom b/ash/public/interfaces/window_properties.mojom
@@ -4,6 +4,11 @@
 
 module ash.mojom;
 
+// V1 apps can intercept system keys. This will let the app handle F-keys instead
+// of the window manager.
+const string kCanConsumeSystemKeys_Property =
+  "ash:can-consume-system-keys";
+
 // This is put on windows to indicate that ash should perform auto management of
 // window positions; when you open a second browser, ash will move the two to
 // minimize overlap.

diff --git a/ash/wm/window_state.cc b/ash/wm/window_state.cc
@@ -336,6 +336,14 @@ void WindowState::SetWindowPositionManaged(bool managed) {
   window_->SetProperty(kWindowPositionManagedTypeKey, managed);
 }
 
+bool WindowState::CanConsumeSystemKeys() const {
+  return window_->GetProperty(kCanConsumeSystemKeysKey);
+}
+
+void WindowState::SetCanConsumeSystemKeys(bool can_consume_system_keys) {
+  window_->SetProperty(kCanConsumeSystemKeysKey, can_consume_system_keys);
+}
+
 void WindowState::set_bounds_changed_by_user(bool bounds_changed_by_user) {
   bounds_changed_by_user_ = bounds_changed_by_user;
   if (bounds_changed_by_user)

diff --git a/ash/wm/window_state.h b/ash/wm/window_state.h
@@ -274,10 +274,8 @@ class ASH_EXPORT WindowState : public aura::WindowObserver {
   // True if the window should be offered a chance to consume special system
   // keys such as brightness, volume, etc. that are usually handled by the
   // shell.
-  bool can_consume_system_keys() const { return can_consume_system_keys_; }
-  void set_can_consume_system_keys(bool can_consume_system_keys) {
-    can_consume_system_keys_ = can_consume_system_keys;
-  }
+  bool CanConsumeSystemKeys() const;
+  void SetCanConsumeSystemKeys(bool can_consume_system_keys);
 
   // True if the window is in "immersive full screen mode" which is slightly
   // different from the normal fullscreen mode by allowing the user to reveal

diff --git a/ash/wm/window_state_unittest.cc b/ash/wm/window_state_unittest.cc
@@ -6,6 +6,7 @@
 
 #include <utility>
 
+#include "ash/public/cpp/window_properties.h"
 #include "ash/test/ash_test_base.h"
 #include "ash/wm/window_state.h"
 #include "ash/wm/window_state_util.h"
@@ -458,6 +459,17 @@ TEST_F(WindowStateTest, FullscreenMinimizedSwitching) {
   ASSERT_TRUE(window_state->IsMaximized());
 }
 
+TEST_F(WindowStateTest, CanConsumeSystemKeys) {
+  std::unique_ptr<aura::Window> window(
+      CreateTestWindowInShellWithBounds(gfx::Rect(100, 100, 100, 100)));
+  WindowState* window_state = GetWindowState(window.get());
+
+  EXPECT_FALSE(window_state->CanConsumeSystemKeys());
+
+  window->SetProperty(kCanConsumeSystemKeysKey, true);
+  EXPECT_TRUE(window_state->CanConsumeSystemKeys());
+}
+
 // TODO(skuhne): Add more unit test to verify the correctness for the restore
 // operation.
 

diff --git a/chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.cc b/chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.cc
@@ -66,6 +66,10 @@ void ChromeBrowserMainExtraPartsAsh::ServiceManagerConnectionStarted(
     aura::WindowTreeClientDelegate* delegate = mus_client;
     aura::PropertyConverter* converter = delegate->GetPropertyConverter();
 
+    converter->RegisterPrimitiveProperty(
+        ash::kCanConsumeSystemKeysKey,
+        ash::mojom::kCanConsumeSystemKeys_Property,
+        aura::PropertyConverter::CreateAcceptAnyValueCallback());
     converter->RegisterPrimitiveProperty(
         ash::kPanelAttachedKey,
         ui::mojom::WindowManager::kPanelAttached_Property,

diff --git a/chrome/browser/ui/views/frame/browser_frame_ash.cc b/chrome/browser/ui/views/frame/browser_frame_ash.cc
@@ -6,12 +6,14 @@
 
 #include <memory>
 
-#include "ash/public/cpp/window_properties.h"
-#include "ash/shell.h"
-#include "ash/wm/window_properties.h"
-#include "ash/wm/window_state.h"
-#include "ash/wm/window_state_delegate.h"
-#include "ash/wm/window_util.h"
+// This file is only instantiated in classic ash/mus. It is never used in mash.
+// See native_browser_frame_factory_chromeos.cc switches on GetAshConfig().
+#include "ash/public/cpp/window_properties.h"  // mash-ok
+#include "ash/shell.h"                         // mash-ok
+#include "ash/wm/window_properties.h"          // mash-ok
+#include "ash/wm/window_state.h"               // mash-ok
+#include "ash/wm/window_state_delegate.h"      // mash-ok
+#include "ash/wm/window_util.h"                // mash-ok
 #include "base/macros.h"
 #include "build/build_config.h"
 #include "chrome/browser/ui/browser_commands.h"
@@ -73,7 +75,7 @@ BrowserFrameAsh::BrowserFrameAsh(BrowserFrame* browser_frame,
   // For legacy reasons v1 apps (like Secure Shell) are allowed to consume keys
   // like brightness, volume, etc. Otherwise these keys are handled by the
   // Ash window manager.
-  window_state->set_can_consume_system_keys(browser->is_app());
+  window_state->SetCanConsumeSystemKeys(browser->is_app());
 }
 
 BrowserFrameAsh::~BrowserFrameAsh() {}

diff --git a/chrome/browser/ui/views/frame/browser_frame_mus.cc b/chrome/browser/ui/views/frame/browser_frame_mus.cc
@@ -19,6 +19,7 @@
 
 #if defined(OS_CHROMEOS)
 #include "ash/public/cpp/shelf_types.h"
+#include "ash/public/interfaces/window_properties.mojom.h"
 #include "ash/public/interfaces/window_style.mojom.h"
 #include "services/ui/public/interfaces/window_manager.mojom.h"
 #endif
@@ -52,6 +53,12 @@ views::Widget::InitParams BrowserFrameMus::GetWidgetParams() {
   properties[ui::mojom::WindowManager::kShelfItemType_Property] =
       mojo::ConvertTo<std::vector<uint8_t>>(
           static_cast<int64_t>(ash::TYPE_BROWSER_SHORTCUT));
+  properties[ash::mojom::kWindowPositionManaged_Property] =
+      mojo::ConvertTo<std::vector<uint8_t>>(
+          static_cast<int64_t>(browser_view_->browser()->is_type_popup()));
+  properties[ash::mojom::kCanConsumeSystemKeys_Property] =
+      mojo::ConvertTo<std::vector<uint8_t>>(
+          static_cast<int64_t>(browser_view_->browser()->is_app()));
 #endif
   aura::WindowTreeHostMusInitParams window_tree_host_init_params =
       aura::CreateInitParamsForTopLevel(