cros: Fix some shelf browser tests under --mash

* Don't start quick launch in tests, because that changes
  the number of shelf items vs. classic ash
* Fix "draw attention" property mirroring, such that
  ash updates when chrome sets the property
* Eliminate direct access to ash::Shell and ash::Shelf*
  in ChromeLauncherController test setup, which fixes a
  bunch of test crashes

Bug: 678687
Test: browser_tests --mash
Change-Id: I721d95b42f2bf36fffbfde94db32db46626a48df
Reviewed-on: https://chromium-review.googlesource.com/767168
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516424}

ash/mus/window_manager.cc

@@ -91,6 +91,10 @@ WindowManager::WindowManager(service_manager::Connector* connector,
   property_converter_->RegisterPrimitiveProperty(
       kCanConsumeSystemKeysKey, ash::mojom::kCanConsumeSystemKeys_Property,
       aura::PropertyConverter::CreateAcceptAnyValueCallback());
+  property_converter_->RegisterPrimitiveProperty(
+      aura::client::kDrawAttentionKey,
+      ui::mojom::WindowManager::kDrawAttention_Property,
+      aura::PropertyConverter::CreateAcceptAnyValueCallback());
   property_converter_->RegisterPrimitiveProperty(
       kPanelAttachedKey, ui::mojom::WindowManager::kPanelAttached_Property,
       aura::PropertyConverter::CreateAcceptAnyValueCallback());

chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc

@@ -21,7 +21,6 @@
 #include "base/run_loop.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
-#include "build/build_config.h"
 #include "chrome/browser/apps/app_browsertest_util.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/extension_apitest.h"
@@ -74,10 +73,12 @@
 #include "ui/app_list/views/start_page_view.h"
 #include "ui/app_list/views/tile_item_view.h"
 #include "ui/aura/client/aura_constants.h"
+#include "ui/aura/test/mus/change_completion_waiter.h"
 #include "ui/aura/window.h"
 #include "ui/base/base_window.h"
 #include "ui/base/window_open_disposition.h"
 #include "ui/display/manager/display_manager.h"
+#include "ui/display/screen.h"
 #include "ui/display/test/display_manager_test_api.h"
 #include "ui/events/event.h"
 #include "ui/events/event_constants.h"
@@ -93,7 +94,10 @@ namespace {
 ash::ShelfAction SelectItem(const ash::ShelfID& id,
                             ui::EventType event_type = ui::ET_MOUSE_PRESSED,
                             int64_t display_id = display::kInvalidDisplayId) {
-  return SelectShelfItem(id, event_type, display_id);
+  ash::ShelfAction action = SelectShelfItem(id, event_type, display_id);
+  // Wait for window manager to stabilize.
+  aura::test::WaitForAllChangesToComplete();
+  return action;
 }
 
 class TestEvent : public ui::Event {
@@ -137,6 +141,13 @@ void CloseBrowserWindow(Browser* browser,
   close_observer.Wait();
 }
 
+// Returns the shelf view for the primary display.
+// TODO(jamescook): Convert users of this function to the mojo shelf test API.
+ash::ShelfView* GetPrimaryShelfView() {
+  return ash::Shelf::ForWindow(ash::Shell::GetPrimaryRootWindow())
+      ->GetShelfViewForTesting();
+}
+
 }  // namespace
 
 class LauncherPlatformAppBrowserTest
@@ -212,7 +223,6 @@ class ShelfAppBrowserTest : public ExtensionBrowserTest {
     // Ensure ash starts the session and creates the shelf and controller.
     SessionControllerClient::FlushForTesting();
 
-    shelf_ = Shelf::ForWindow(ash::Shell::GetPrimaryRootWindow());
     controller_ = ChromeLauncherController::instance();
     ASSERT_TRUE(controller_);
     ExtensionBrowserTest::SetUpOnMainThread();
@@ -316,7 +326,6 @@ class ShelfAppBrowserTest : public ExtensionBrowserTest {
     return menu->GetIndexOfCommandId(command_id) != -1;
   }
 
-  Shelf* shelf_ = nullptr;
   ChromeLauncherController* controller_ = nullptr;
 
  private:
@@ -744,14 +753,9 @@ IN_PROC_BROWSER_TEST_F(LauncherPlatformAppBrowserTest, BrowserActivation) {
   EXPECT_EQ(ash::STATUS_RUNNING, shelf_model()->ItemByID(item_id1)->status);
 }
 
-// TODO(crbug.com/735842): Flaky on CrOS MSan.
-#if defined(OS_CHROMEOS)
-#define MAYBE_SetIcon DISABLED_SetIcon
-#else
-#define MAYBE_SetIcon SetIcon
-#endif
 // Test that opening an app sets the correct icon
-IN_PROC_BROWSER_TEST_F(LauncherPlatformAppBrowserTest, MAYBE_SetIcon) {
+// TODO(crbug.com/735842): Flaky on CrOS MSan.
+IN_PROC_BROWSER_TEST_F(LauncherPlatformAppBrowserTest, DISABLED_SetIcon) {
   TestAppWindowIconObserver test_observer(browser()->profile());
 
   // Enable experimental APIs to allow panel creation.
@@ -1658,7 +1662,7 @@ IN_PROC_BROWSER_TEST_F(ShelfAppBrowserTest, DISABLED_DragAndDrop) {
   // Get a number of interfaces we need.
   ui::test::EventGenerator generator(ash::Shell::GetPrimaryRootWindow(),
                                      gfx::Point());
-  ash::ShelfViewTestAPI test(shelf_->GetShelfViewForTesting());
+  ash::ShelfViewTestAPI test(GetPrimaryShelfView());
   AppListService* service = AppListService::Get();
 
   // There should be two items in our launcher by this time.
@@ -1870,7 +1874,7 @@ IN_PROC_BROWSER_TEST_F(ShelfAppBrowserTest, MultiDisplayBasicDragAndDrop) {
 IN_PROC_BROWSER_TEST_F(ShelfAppBrowserTest, DISABLED_DragOffShelf) {
   ui::test::EventGenerator generator(ash::Shell::GetPrimaryRootWindow(),
                                      gfx::Point());
-  ash::ShelfViewTestAPI test(shelf_->GetShelfViewForTesting());
+  ash::ShelfViewTestAPI test(GetPrimaryShelfView());
   test.SetAnimationDuration(1);  // Speed up animations for test.
   // Create a known application and check that we have 3 items in the shelf.
   CreateShortcut("app1");
@@ -1974,7 +1978,7 @@ IN_PROC_BROWSER_TEST_F(ShelfAppBrowserTest, ClickItem) {
   // Get a number of interfaces we need.
   ui::test::EventGenerator generator(ash::Shell::GetPrimaryRootWindow(),
                                      gfx::Point());
-  ash::ShelfViewTestAPI test(shelf_->GetShelfViewForTesting());
+  ash::ShelfViewTestAPI test(GetPrimaryShelfView());
   AppListService* service = AppListService::Get();
   // There should be two items in our shelf by this time.
   EXPECT_EQ(2, shelf_model()->item_count());

chrome/browser/ui/views/chrome_browser_main_extra_parts_views.cc

@@ -6,6 +6,7 @@
 
 #include <utility>
 
+#include "base/command_line.h"
 #include "base/memory/ptr_util.h"
 #include "chrome/browser/ui/views/chrome_constrained_window_views_client.h"
 #include "chrome/browser/ui/views/chrome_views_delegate.h"
@@ -39,7 +40,6 @@
 #include <sys/types.h>
 #include <unistd.h>
 
-#include "base/command_line.h"
 #include "chrome/browser/ui/simple_message_box.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
@@ -49,6 +49,7 @@
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/ash_config.h"
+#include "content/public/common/content_switches.h"
 #include "mash/common/config.h"                                   // nogncheck
 #include "mash/quick_launch/public/interfaces/constants.mojom.h"  // nogncheck
 #endif
@@ -174,8 +175,13 @@ void ChromeBrowserMainExtraPartsViews::ServiceManagerConnectionStarted(
         service_manager::Identity(ui::mojom::kServiceName));
     connection->GetConnector()->StartService(
         service_manager::Identity(mash::common::GetWindowManagerServiceName()));
-    connection->GetConnector()->StartService(
-        service_manager::Identity(mash::quick_launch::mojom::kServiceName));
+    // Don't start QuickLaunch in tests because it changes the startup shelf
+    // state vs. classic ash.
+    if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
+            switches::kTestType)) {
+      connection->GetConnector()->StartService(
+          service_manager::Identity(mash::quick_launch::mojom::kServiceName));
+    }
   }
 #endif
 

services/ui/public/interfaces/window_manager.mojom

@@ -93,6 +93,10 @@ interface WindowManager {
   // The application icon; typically larger for shelf icons, etc. Type: SkBitmap
   const string kAppIcon_Property = "prop:app-icon";
 
+  // Whether the window is trying to draw attention to itself (e.g. pulsing its
+  // shelf icon). Type: bool.
+  const string kDrawAttention_Property = "prop:draw-attention";
+
   // Used to explicitly control whether a window appears in the most recently
   // used list of windows. Maps to aura::client::kExcludeFromMruKey. Type: bool.
   const string kExcludeFromMru_Property = "prop:exclude_from_mru";

ui/aura/mus/property_converter.cc

@@ -73,6 +73,9 @@ PropertyConverter::PropertyConverter() {
   RegisterPrimitiveProperty(client::kAlwaysOnTopKey,
                             ui::mojom::WindowManager::kAlwaysOnTop_Property,
                             CreateAcceptAnyValueCallback());
+  RegisterPrimitiveProperty(client::kDrawAttentionKey,
+                            ui::mojom::WindowManager::kDrawAttention_Property,
+                            CreateAcceptAnyValueCallback());
   RegisterPrimitiveProperty(
       client::kImmersiveFullscreenKey,
       ui::mojom::WindowManager::kImmersiveFullscreen_Property,