Let ash provide window show state if client has SHOW_STATE_DEFAULT

For lock screen app windows the window manager needs to specify the
initial show state of the window. Ensure the client (chrome) does not
overwrite the aura window show-state property in ash if the client
window has ui::SHOW_STATE_DEFAULT.

Fixes LockScreenNoteTakingTest.Launch pass under SingleProcessMash

Bug: 899055
Test: added to views_mus_unittests

Change-Id: Ib7b6f9a060835ebf3e017737e4b98387a31831c8
Reviewed-on: https://chromium-review.googlesource.com/c/1303273Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604517}

chrome/browser/chromeos/lock_screen_apps/note_taking_browsertest.cc

@@ -17,10 +17,13 @@
 #include "chromeos/chromeos_switches.h"
 #include "components/prefs/pref_service.h"
 #include "components/session_manager/core/session_manager.h"
+#include "extensions/browser/app_window/app_window.h"
+#include "extensions/browser/app_window/native_app_window.h"
 #include "extensions/common/api/app_runtime.h"
 #include "extensions/common/switches.h"
 #include "extensions/test/extension_test_message_listener.h"
 #include "extensions/test/result_catcher.h"
+#include "ui/aura/test/mus/change_completion_waiter.h"
 
 namespace {
 
@@ -146,6 +149,20 @@ class LockScreenNoteTakingTest : public extensions::ExtensionBrowserTest {
       return false;
     }
 
+    // By this point the app window is created on the lock screen. Ensure the
+    // asynchronous window maximize from ash completes.
+    aura::test::WaitForAllChangesToComplete();
+    extensions::AppWindow* app_window =
+        lock_screen_apps::StateController::Get()->note_app_window_for_test();
+    if (!app_window) {
+      *error = "No app window";
+      return false;
+    }
+    if (!app_window->GetBaseWindow()->IsMaximized()) {
+      *error = "App window not maximized";
+      return false;
+    }
+
     // Close the app window created by the API test.
     ready_to_close.Reply("close");
 

chrome/browser/chromeos/lock_screen_apps/state_controller.h

@@ -108,6 +108,7 @@ class StateController : public ash::mojom::TrayActionClient,
   // |SetPrimaryProfile|
   void SetLockScreenLockScreenProfileCreatorForTesting(
       std::unique_ptr<LockScreenProfileCreator> profile_creator);
+  extensions::AppWindow* note_app_window_for_test() { return note_app_window_; }
 
   // Initializes mojo bindings for the StateController - it creates binding to
   // ash's tray action interface and sets this object as the interface's client.

chrome/test/data/extensions/lock_screen_apps/app_launch/test.js

@@ -11,7 +11,6 @@ chrome.test.runTests([
   function hasAccessToCurrentWindow() {
     chrome.test.assertTrue(!!chrome.app.window.current);
     chrome.test.assertTrue(!!chrome.app.window.current());
-    chrome.test.assertTrue(chrome.app.window.current().isMaximized());
     chrome.test.succeed();
   },
 

services/ws/test_ws/test_window_service.cc

@@ -16,6 +16,7 @@
 #include "services/ws/test_host_event_dispatcher.h"
 #include "services/ws/test_ws/test_gpu_interface_provider.h"
 #include "services/ws/window_service.h"
+#include "ui/aura/client/aura_constants.h"
 #include "ui/aura/env.h"
 #include "ui/aura/mus/property_utils.h"
 #include "ui/compositor/test/context_factories_for_test.h"
@@ -74,6 +75,11 @@ std::unique_ptr<aura::Window> TestWindowService::NewTopLevel(
     property_converter->SetPropertyFromTransportValue(
         top_level.get(), property.first, &property.second);
   }
+  if (maximize_next_window_) {
+    top_level->SetProperty(aura::client::kShowStateKey,
+                           ui::SHOW_STATE_MAXIMIZED);
+    maximize_next_window_ = false;
+  }
   return top_level;
 }
 
@@ -153,6 +159,11 @@ void TestWindowService::OnGpuServiceInitialized() {
     std::move(pending_create_service_).Run();
 }
 
+void TestWindowService::MaximizeNextWindow(MaximizeNextWindowCallback cb) {
+  maximize_next_window_ = true;
+  std::move(cb).Run();
+}
+
 void TestWindowService::Shutdown(
     test_ws::mojom::TestWs::ShutdownCallback callback) {
   // WindowService depends upon Screen, which is owned by AuraTestHelper.

services/ws/test_ws/test_window_service.h

@@ -86,6 +86,7 @@ class TestWindowService : public service_manager::Service,
   void OnGpuServiceInitialized() override;
 
   // test_ws::mojom::TestWs:
+  void MaximizeNextWindow(MaximizeNextWindowCallback cb) override;
   void Shutdown(test_ws::mojom::TestWs::ShutdownCallback callback) override;
 
   void BindServiceFactory(
@@ -120,6 +121,7 @@ class TestWindowService : public service_manager::Service,
 
   bool started_ = false;
   bool ui_service_created_ = false;
+  bool maximize_next_window_ = false;
 
   base::OnceClosure pending_create_service_;
 

services/ws/test_ws/test_ws.mojom

@@ -8,6 +8,10 @@ const string kServiceName = "test_ws";
 
 // Implemented by TestWindowService.
 interface TestWs {
+  // Requests that the window service create the next top-level window with
+  // show state maximized.
+  MaximizeNextWindow() => ();
+
   // Used when caller needs to explicitly shutdown the window service hosted
   // in test_ws. Callback is provided so that caller can resume its shutdown
   // sequence.

testing/buildbot/filters/chromeos.mash.fyi.browser_tests.filter

@@ -126,9 +126,6 @@
 -LauncherPlatformAppBrowserTest.PanelAttentionStatus
 -LauncherPlatformAppBrowserTest.PanelItemClickBehavior
 
-# JS failure: hasAccessToCurrentWindow: FAIL (no message)
--LockScreenNoteTakingTest.*
-
 # Missing magnification manager and also RefCounted check failed:
 # CalledOnValidSequence() from SchedulerWorkerDelegate::OnMainExit
 -LoginScreenDefaultPolicyInSessionBrowsertest.*

testing/buildbot/filters/chromeos.single_process_mash.browser_tests.filter

@@ -49,9 +49,6 @@
 -PDFExtensionTest.*
 -PDFExtensionHitTestTest.*
 
-# JS failure: hasAccessToCurrentWindow: FAIL (no message)
--LockScreenNoteTakingTest.*
-
 # Missing magnification manager and also RefCounted check failed:
 # CalledOnValidSequence() from SchedulerWorkerDelegate::OnMainExit
 -LoginScreenDefaultPolicyLoginScreenBrowsertest.*
@@ -135,9 +132,6 @@
 # Flaky on ASAN bot. crbug.com/884216
 -KeyboardOverlayUIBrowserTest.*
 
-# Lock screen tests.
--LockScreenNoteTakingTest.Launch
-
 # PRE steps crash on shutdown.
 # FATAL:ref_counted.h(85)] Check failed: CalledOnValidSequence()
 -ManagedWithoutPermission/ManagedWithoutPermissionPlatformKeysTest.*

ui/base/class_property.cc

@@ -67,4 +67,4 @@ std::set<const void*> PropertyHandler::GetAllPropertyKeys() const {
   return keys;
 }
 
-} // namespace ui
+}  // namespace ui
\ No newline at end of file

ui/views/mus/BUILD.gn

@@ -169,6 +169,7 @@ test("views_mus_unittests") {
     "//cc",
     "//net",
     "//services/ws/public/mojom",
+    "//services/ws/test_ws:mojom",
     "//skia",
     "//testing/gtest",
     "//third_party/icu",

ui/views/mus/desktop_window_tree_host_mus.cc

@@ -412,7 +412,10 @@ void DesktopWindowTreeHostMus::Init(const Widget::InitParams& params) {
   content_window()->SetTransparent(translucent);
   window()->SetTransparent(translucent);
 
-  window()->SetProperty(aura::client::kShowStateKey, params.show_state);
+  // The window manager may provide the initial show state, for example for
+  // Chrome OS lock screen windows. https://crbug.com/899055
+  if (params.show_state != ui::SHOW_STATE_DEFAULT)
+    window()->SetProperty(aura::client::kShowStateKey, params.show_state);
 
   if (!params.bounds.IsEmpty())
     SetBoundsInDIP(params.bounds);

ui/views/mus/desktop_window_tree_host_mus_unittest.cc

@@ -5,6 +5,7 @@
 #include "ui/views/mus/desktop_window_tree_host_mus.h"
 
 #include "base/strings/utf_string_conversions.h"
+#include "services/ws/test_ws/test_ws.mojom.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/aura/client/cursor_client.h"
 #include "ui/aura/client/focus_client.h"
@@ -100,6 +101,29 @@ class ExpectsNullCursorClientDuringTearDown : public aura::WindowObserver {
   DISALLOW_COPY_AND_ASSIGN(ExpectsNullCursorClientDuringTearDown);
 };
 
+// Tests that the window service can set the initial show state for a window.
+// https://crbug.com/899055
+TEST_F(DesktopWindowTreeHostMusTest, ShowStateFromWindowService) {
+  // Configure the window service to maximize the next top-level window.
+  test_ws::mojom::TestWsPtr test_ws_ptr;
+  MusClient::Get()->window_tree_client()->connector()->BindInterface(
+      test_ws::mojom::kServiceName, &test_ws_ptr);
+  test_ws::mojom::TestWsAsyncWaiter wait_for(test_ws_ptr.get());
+  wait_for.MaximizeNextWindow();
+
+  // Create a widget with the default show state.
+  Widget widget;
+  Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_WINDOW);
+  params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+  params.bounds = gfx::Rect(0, 0, 100, 100);
+  EXPECT_EQ(ui::SHOW_STATE_DEFAULT, params.show_state);
+  widget.Init(params);
+  aura::test::WaitForAllChangesToComplete();
+
+  // Window service provided the show state.
+  EXPECT_TRUE(widget.IsMaximized());
+}
+
 TEST_F(DesktopWindowTreeHostMusTest, Visibility) {
   std::unique_ptr<Widget> widget(CreateWidget());
   EXPECT_FALSE(widget->IsVisible());