Mash: Fix browser split view mode and browser tests.

Bug: 854704
Change-Id: Ic04c377547f74c4866b46fd91f7f3fc0b075fdd4
Reviewed-on: https://chromium-review.googlesource.com/c/1290139Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601330}

ash/public/interfaces/shell_test_api.mojom

@@ -15,4 +15,8 @@ interface ShellTestApi {
   // Enables the keyboard and associates it with the primary root window
   // controller. In tablet mode, enables the virtual keyboard.
   EnableVirtualKeyboard() => ();
+
+  // Tells the SplitViewController to snap the given window to the left.
+  // The client name is used to find the client's WindowTree.
+  SnapWindowInSplitView(string client_name, uint64 window_id) => ();
 };

ash/shell_test_api.cc

@@ -10,9 +10,13 @@
 #include "ash/shell.h"
 #include "ash/system/power/backlights_forced_off_setter.h"
 #include "ash/system/power/power_button_controller.h"
+#include "ash/wm/splitview/split_view_controller.h"
 #include "ash/wm/tablet_mode/tablet_mode_controller.h"
+#include "ash/ws/window_service_owner.h"
 #include "components/prefs/testing_pref_service.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
+#include "services/ws/window_service.h"
+#include "services/ws/window_tree.h"
 
 namespace ash {
 
@@ -81,4 +85,22 @@ void ShellTestApi::EnableVirtualKeyboard(EnableVirtualKeyboardCallback cb) {
   std::move(cb).Run();
 }
 
+void ShellTestApi::SnapWindowInSplitView(const std::string& client_name,
+                                         ws::Id window_id,
+                                         SnapWindowInSplitViewCallback cb) {
+  auto* window_service = shell_->window_service_owner()->window_service();
+  aura::Window* window = nullptr;
+  for (ws::WindowTree* window_tree : window_service->window_trees()) {
+    if (client_name == window_tree->client_name()) {
+      window = window_tree->GetWindowByTransportId(window_id);
+      break;
+    }
+  }
+  DCHECK(window);
+  shell_->split_view_controller()->SnapWindow(window,
+                                              ash::SplitViewController::LEFT);
+  shell_->split_view_controller()->FlushForTesting();
+  std::move(cb).Run();
+}
+
 }  // namespace ash

ash/shell_test_api.h

@@ -9,6 +9,7 @@
 
 #include "ash/public/interfaces/shell_test_api.mojom.h"
 #include "base/macros.h"
+#include "services/ws/common/types.h"
 
 class PrefService;
 
@@ -54,6 +55,9 @@ class ShellTestApi : public mojom::ShellTestApi {
   void IsSystemModalWindowOpen(IsSystemModalWindowOpenCallback cb) override;
   void EnableTabletModeWindowManager(bool enable) override;
   void EnableVirtualKeyboard(EnableVirtualKeyboardCallback cb) override;
+  void SnapWindowInSplitView(const std::string& client_name,
+                             ws::Id window_id,
+                             SnapWindowInSplitViewCallback cb) override;
 
  private:
   Shell* shell_;  // not owned

ash/wm/splitview/split_view_controller.cc

@@ -639,6 +639,11 @@ void SplitViewController::RemoveObserver(Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
+void SplitViewController::FlushForTesting() {
+  mojo_observers_.FlushForTesting();
+  bindings_.FlushForTesting();
+}
+
 void SplitViewController::AddObserver(mojom::SplitViewObserverPtr observer) {
   mojom::SplitViewObserver* observer_ptr = observer.get();
   mojo_observers_.AddPtr(std::move(observer));

ash/wm/splitview/split_view_controller.h

@@ -159,6 +159,8 @@ class ASH_EXPORT SplitViewController : public mojom::SplitViewController,
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
 
+  void FlushForTesting();
+
   // mojom::SplitViewObserver:
   void AddObserver(mojom::SplitViewObserverPtr observer) override;
 

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

@@ -91,7 +91,7 @@ bool IsV1AppBackButtonEnabled() {
 }
 
 // Returns true if |window| is currently snapped in split view mode.
-bool IsSnappedInSplitView(aura::Window* window,
+bool IsSnappedInSplitView(const aura::Window* window,
                           ash::mojom::SplitViewState state) {
   ash::mojom::WindowStateType type =
       window->GetProperty(ash::kWindowStateTypeKey);
@@ -741,7 +741,7 @@ bool BrowserNonClientFrameViewAsh::ShouldShowCaptionButtons() const {
   }
 
   return !IsInOverviewMode() ||
-         IsSnappedInSplitView(frame()->GetNativeWindow(), split_view_state_);
+         IsSnappedInSplitView(GetFrameWindow(), split_view_state_);
 }
 
 int BrowserNonClientFrameViewAsh::GetTabStripLeftInset() const {

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

@@ -17,6 +17,8 @@
 #include "ash/public/cpp/immersive/immersive_fullscreen_controller_test_api.h"
 #include "ash/public/cpp/vector_icons/vector_icons.h"
 #include "ash/public/cpp/window_properties.h"
+#include "ash/public/interfaces/constants.mojom.h"
+#include "ash/public/interfaces/shell_test_api.mojom.h"
 #include "ash/shell.h"
 #include "ash/wm/overview/window_selector_controller.h"
 #include "ash/wm/splitview/split_view_controller.h"
@@ -71,8 +73,11 @@
 #include "components/keep_alive_registry/scoped_keep_alive.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
+#include "content/public/common/service_manager_connection.h"
+#include "content/public/common/service_names.mojom.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "net/dns/mock_host_resolver.h"
+#include "services/service_manager/public/cpp/connector.h"
 #include "services/ws/public/mojom/window_tree_constants.mojom.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/aura/test/env_test_helper.h"
@@ -1356,6 +1361,8 @@ IN_PROC_BROWSER_TEST_P(NonHomeLauncherBrowserNonClientFrameViewAshTest,
       ws::mojom::kResizeBehaviorCanMaximize |
           ws::mojom::kResizeBehaviorCanResize);
 
+  const int restored_height = frame_view->frame_header_->GetHeaderHeight();
+
   // Setting the tablet mode must be done before calculating the expected height
   // since the height may change depending on the tablet mode when in the
   // dynamic refresh MD mode.
@@ -1364,21 +1371,39 @@ IN_PROC_BROWSER_TEST_P(NonHomeLauncherBrowserNonClientFrameViewAshTest,
   // Maximize the widget and store its frame header height.
   widget->Maximize();
   const int expected_height = frame_view->frame_header_->GetHeaderHeight();
+  EXPECT_NE(expected_height, restored_height);
   widget->Restore();
-
-  ash::Shell* shell = ash::Shell::Get();
-  ash::SplitViewController* split_view_controller =
-      shell->split_view_controller();
-  split_view_controller->BindRequest(
-      mojo::MakeRequest(&frame_view->split_view_controller_));
-  split_view_controller->AddObserver(
-      frame_view->CreateInterfacePtrForTesting());
-  frame_view->split_view_controller_.FlushForTesting();
+  // Restored in tablet mode has the same header height as maximized in tablet
+  // mode.
+  EXPECT_EQ(expected_height, frame_view->frame_header_->GetHeaderHeight());
 
   frame_view->GetFrameWindow()->SetProperty(ash::kIsShowingInOverviewKey, true);
-  split_view_controller->SnapWindow(widget->GetNativeWindow(),
-                                    ash::SplitViewController::LEFT);
-  frame_view->split_view_controller_.FlushForTesting();
+
+  if (features::IsUsingWindowService()) {
+    ash::mojom::ShellTestApiPtr shell_test_api;
+    content::ServiceManagerConnection::GetForProcess()
+        ->GetConnector()
+        ->BindInterface(ash::mojom::kServiceName, &shell_test_api);
+    base::RunLoop run_loop;
+    shell_test_api->SnapWindowInSplitView(content::mojom::kBrowserServiceName,
+                                          frame_view->GetServerWindowId(),
+                                          run_loop.QuitClosure());
+    run_loop.Run();
+  } else {
+    ash::Shell* shell = ash::Shell::Get();
+    ash::SplitViewController* split_view_controller =
+        shell->split_view_controller();
+    split_view_controller->BindRequest(
+        mojo::MakeRequest(&frame_view->split_view_controller_));
+    split_view_controller->AddObserver(
+        frame_view->CreateInterfacePtrForTesting());
+    frame_view->split_view_controller_.FlushForTesting();
+
+    split_view_controller->SnapWindow(widget->GetNativeWindow(),
+                                      ash::SplitViewController::LEFT);
+    frame_view->split_view_controller_.FlushForTesting();
+  }
+
   EXPECT_TRUE(frame_view->caption_button_container_->visible());
   EXPECT_EQ(expected_height, frame_view->frame_header_->GetHeaderHeight());
 }

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

@@ -41,9 +41,6 @@
 -TopControlsSlideControllerTest.TestFocusEditableElements
 -TopControlsSlideControllerTest.DisplayRotation
 
-# Uses Shell/SplitViewController.
--NonHomeLauncherBrowserNonClientFrameViewAshTest.HeaderHeightForSnappedBrowserInSplitView/*
-
 # ash::Shell access from ChromeViewsDelegate::CreateDefaultNonClientFrameView()
 # e.g. from chromeos::CaptivePortalWindowProxy::Show().
 # See https://crbug.com/838974

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

@@ -35,9 +35,6 @@
 -TopControlsSlideControllerTest.TestScrollingPage
 -TopControlsSlideControllerTest.TestScrollingPageAndSwitchingToNTP
 
-# Crash in ash::SplitViewController
--NonHomeLauncherBrowserNonClientFrameViewAshTest.HeaderHeightForSnappedBrowserInSplitView/*
-
 # Need pixel copy support. http://crbug.com/754864 http://crbug.com/754872
 -CastMirroringServiceHostBrowserTest.CaptureTabVideo
 -CastStreamingApiTestWithPixelOutput.*