Revert "OopAsh: improve immersive fullscreen for app windows"

This reverts commit 4480bf47.

Reason for revert: Strong suspect for causing crashes in WidgetTest.FullscreenFrameLayout on linux-chromeos-rel

Bug: 868004

Original change's description:
> OopAsh: improve immersive fullscreen for app windows
> 
> Immersive fullscreen for an app window almost works aside from the fact
> that the normal frame remains when the app window is fullscreened. This
> is a result of not updating the client area inset when the window show
> state changes (e.g. from restored to fullscreen). The client area is
> updated when the window bounds change, as they would when entering
> fullscreen, but the bounds change comes through just before the state
> change. Thus, when ClientSideNonClientFrameView checks fullscreen state
> during layout, it still finds the widget is restored and calculates the
> wrong bounds. The solution is to explicitly kick off a layout when
> entering or exiting fullscreen.
> 
>       after launching with --enable-features=Mash --ash-dev-shortcuts
> 
> Test: enter and exit fullscreen on an app window with ctrl+shift+F
> Bug: 640365
> Change-Id: I499e29427ed072f3428de1f195ede1dc9892e71f
> Reviewed-on: https://chromium-review.googlesource.com/1150618
> Reviewed-by: Scott Violet <sky@chromium.org>
> Commit-Queue: Evan Stade <estade@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#578353}

TBR=sky@chromium.org,estade@chromium.org

Change-Id: I2effa307fa2da94610130e6b084b6979fc4c070e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 640365
Reviewed-on: https://chromium-review.googlesource.com/1151908Reviewed-by: Stephen McGruer <smcgruer@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578380}

ui/views/mus/desktop_window_tree_host_mus.cc

@@ -44,16 +44,13 @@ namespace {
 
 // As the window manager renderers the non-client decorations this class does
 // very little but honor the client area insets from the window manager.
-class ClientSideNonClientFrameView : public NonClientFrameView,
-                                     public aura::WindowObserver {
+class ClientSideNonClientFrameView : public NonClientFrameView {
  public:
   explicit ClientSideNonClientFrameView(views::Widget* widget)
       : widget_(widget) {
     // Not part of the accessibility node hierarchy because the window frame is
     // provided by the window manager.
     GetViewAccessibility().set_is_ignored(true);
-
-    observed_.Add(widget_->GetNativeWindow()->GetRootWindow());
   }
   ~ClientSideNonClientFrameView() override {}
 
@@ -128,30 +125,7 @@ class ClientSideNonClientFrameView : public NonClientFrameView,
                      max_size.height() == 0 ? 0 : converted_size.height());
   }
 
-  // aura::WindowObserver:
-  void OnWindowDestroying(aura::Window* window) override {
-    observed_.Remove(window);
-  }
-
-  void OnWindowPropertyChanged(aura::Window* window,
-                               const void* key,
-                               intptr_t old) override {
-    // Do a re-layout on state changes which affect GetBoundsForClientView().
-    // The associated bounds change would also cause a re-layout, but there may
-    // not be a bounds change or it may come from the server before the state is
-    // updated.
-    if (key == aura::client::kShowStateKey) {
-      if (GetBoundsForClientView() != widget_->client_view()->bounds() &&
-          window->GetProperty(aura::client::kShowStateKey) !=
-              ui::SHOW_STATE_MINIMIZED) {
-        InvalidateLayout();
-        widget_->GetRootView()->Layout();
-      }
-    }
-  }
-
   views::Widget* widget_;
-  ScopedObserver<aura::Window, aura::WindowObserver> observed_{this};
 
   DISALLOW_COPY_AND_ASSIGN(ClientSideNonClientFrameView);
 };
@@ -673,7 +647,6 @@ bool DesktopWindowTreeHostMus::IsActive() const {
 void DesktopWindowTreeHostMus::Maximize() {
   window()->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_MAXIMIZED);
 }
-
 void DesktopWindowTreeHostMus::Minimize() {
   window()->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_MINIMIZED);
 }
@@ -773,10 +746,7 @@ NonClientFrameView* DesktopWindowTreeHostMus::CreateNonClientFrameView() {
   if (!ShouldSendClientAreaToServer())
     return nullptr;
 
-  auto* frame =
-      new ClientSideNonClientFrameView(native_widget_delegate_->AsWidget());
-  observed_frame_.Add(frame);
-  return frame;
+  return new ClientSideNonClientFrameView(native_widget_delegate_->AsWidget());
 }
 
 bool DesktopWindowTreeHostMus::ShouldUseNativeFrame() const {
@@ -802,7 +772,6 @@ bool DesktopWindowTreeHostMus::IsFullscreen() const {
   return window()->GetProperty(aura::client::kShowStateKey) ==
          ui::SHOW_STATE_FULLSCREEN;
 }
-
 void DesktopWindowTreeHostMus::SetOpacity(float opacity) {
   WindowTreeHostMus::SetOpacity(opacity);
 }
@@ -933,17 +902,13 @@ void DesktopWindowTreeHostMus::SetBoundsInPixels(
       size.SetToMin(max_size_in_pixels);
     final_bounds_in_pixels.set_size(size);
   }
+  const gfx::Rect old_bounds_in_pixels = GetBoundsInPixels();
   WindowTreeHostMus::SetBoundsInPixels(final_bounds_in_pixels,
                                        local_surface_id);
-}
-
-void DesktopWindowTreeHostMus::OnViewBoundsChanged(views::View* observed_view) {
-  DCHECK_EQ(
-      observed_view,
-      native_widget_delegate_->AsWidget()->non_client_view()->frame_view());
-
-  SendClientAreaToServer();
-  SendHitTestMaskToServer();
+  if (old_bounds_in_pixels.size() != final_bounds_in_pixels.size()) {
+    SendClientAreaToServer();
+    SendHitTestMaskToServer();
+  }
 }
 
 aura::Window* DesktopWindowTreeHostMus::content_window() {

ui/views/mus/desktop_window_tree_host_mus.h

@@ -8,13 +8,11 @@
 #include <memory>
 #include <set>
 
-#include "base/scoped_observer.h"
 #include "base/macros.h"
 #include "ui/aura/mus/focus_synchronizer_observer.h"
 #include "ui/aura/mus/window_tree_host_mus.h"
 #include "ui/aura/window_observer.h"
 #include "ui/views/mus/mus_client_observer.h"
-#include "ui/views/view_observer.h"
 #include "ui/views/mus/mus_export.h"
 #include "ui/views/widget/desktop_aura/desktop_window_tree_host.h"
 #include "ui/views/widget/widget.h"
@@ -30,8 +28,7 @@ class VIEWS_MUS_EXPORT DesktopWindowTreeHostMus
       public MusClientObserver,
       public aura::FocusSynchronizerObserver,
       public aura::WindowObserver,
-      public aura::WindowTreeHostMus,
-      public views::ViewObserver {
+      public aura::WindowTreeHostMus {
  public:
   DesktopWindowTreeHostMus(
       aura::WindowTreeHostMusInitParams init_params,
@@ -147,9 +144,6 @@ class VIEWS_MUS_EXPORT DesktopWindowTreeHostMus
   void SetBoundsInPixels(const gfx::Rect& bounds_in_pixels,
                          const viz::LocalSurfaceId& local_surface_id) override;
 
-  // views::ViewObserver:
-  void OnViewBoundsChanged(views::View* observed_view) override;
-
   // Accessor for DesktopNativeWidgetAura::content_window().
   aura::Window* content_window();
 
@@ -168,8 +162,6 @@ class VIEWS_MUS_EXPORT DesktopWindowTreeHostMus
 
   bool auto_update_client_area_ = true;
 
-  ScopedObserver<views::View, views::ViewObserver> observed_frame_{this};
-
   // Used so that Close() isn't immediate.
   base::WeakPtrFactory<DesktopWindowTreeHostMus> close_widget_factory_;
 

ui/views/mus/desktop_window_tree_host_mus_unittest.cc

@@ -23,7 +23,6 @@
 #include "ui/views/accessibility/view_accessibility.h"
 #include "ui/views/mus/mus_client.h"
 #include "ui/views/mus/screen_mus.h"
-#include "ui/views/mus/window_manager_frame_values.h"
 #include "ui/views/test/views_test_base.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_delegate.h"
@@ -239,44 +238,6 @@ TEST_F(DesktopWindowTreeHostMusTest, ActivateBeforeShow) {
                                      ->active_focus_client());
 }
 
-// Tests that changes to a widget's show state will cause the client area to be
-// updated.
-TEST_F(DesktopWindowTreeHostMusTest, ServerShowStateChangeUpdatesClientArea) {
-  WindowManagerFrameValues test_frame_values;
-  test_frame_values.normal_insets = {3, 0, 0, 0};
-  test_frame_values.maximized_insets = {7, 0, 0, 0};
-  WindowManagerFrameValues::SetInstance(test_frame_values);
-
-  std::unique_ptr<Widget> widget(CreateWidget());
-  widget->Show();
-
-  // Simulate state changes from the server.
-  auto set_widget_state =
-      [&widget](ui::WindowShowState state) {
-        widget->GetNativeWindow()->GetRootWindow()->SetProperty(
-            aura::client::kShowStateKey, state);
-      };
-
-  // A restored window respects normal_insets (the client area is inset from the
-  // root view).
-  gfx::Rect expected_restored_bounds = widget->GetRootView()->bounds();
-  expected_restored_bounds.Inset(test_frame_values.normal_insets);
-  EXPECT_EQ(expected_restored_bounds, widget->client_view()->bounds());
-
-  // A fullscreen window has no insets.
-  EXPECT_FALSE(widget->IsFullscreen());
-  set_widget_state(ui::SHOW_STATE_FULLSCREEN);
-  EXPECT_TRUE(widget->IsFullscreen());
-  EXPECT_EQ(widget->GetRootView()->bounds(), widget->client_view()->bounds());
-
-  // A maximized window respects maximized_insets.
-  gfx::Rect expected_maximized_bounds = widget->GetRootView()->bounds();
-  expected_maximized_bounds.Inset(test_frame_values.maximized_insets);
-  set_widget_state(ui::SHOW_STATE_MAXIMIZED);
-  EXPECT_FALSE(widget->IsFullscreen());
-  EXPECT_EQ(expected_maximized_bounds, widget->client_view()->bounds());
-}
-
 TEST_F(DesktopWindowTreeHostMusTest, CursorClientDuringTearDown) {
   std::unique_ptr<Widget> widget(CreateWidget());
   widget->Show();