OopAsh: Fix appearance of browser header when toggling between states
(restored and maximized)
It would be nice if the window's size change and state change were
synced up. As it is, the size change comes through first and causes a
layout which doesn't take into account the new state. Then, the state
change comes through but doesn't cause a layout to occur. This patch
makes the OopAsh frame do an explicit layout due to state changes.
Bug: 854704
Change-Id: Ieab1883cebab64873e0e1e88da85d8de54af8e7f
Reviewed-on: https://chromium-review.googlesource.com/1142453
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576574}diff --git a/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc b/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
index 40aba85b8..8d73777 100644
--- a/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
+++ b/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
@@ -173,10 +173,8 @@
if (immersive_controller)
immersive_controller->RemoveObserver(this);
- if (frame() && frame()->GetNativeWindow() &&
- frame()->GetNativeWindow()->HasObserver(this)) {
- frame()->GetNativeWindow()->RemoveObserver(this);
- }
+ window_observer_.RemoveAll();
+
if (TabletModeClient::Get())
TabletModeClient::Get()->RemoveObserver(this);
ash::Shell::Get()->RemoveShellObserver(this);
@@ -208,22 +206,22 @@
if (browser->is_app() && IsV1AppBackButtonEnabled())
browser->command_controller()->AddCommandObserver(IDC_BACK, this);
- frame()->GetNativeWindow()->SetProperty(
+ aura::Window* window = frame()->GetNativeWindow();
+ window->SetProperty(
aura::client::kAppType,
static_cast<int>(browser->is_app() ? ash::AppType::CHROME_APP
: ash::AppType::BROWSER));
+ window_observer_.Add(IsMash() ? window->GetRootWindow() : window);
+
// To preserve privacy, tag incognito windows so that they won't be included
// in screenshot sent to assistant server.
- if (browser->profile()->IsOffTheRecord()) {
- frame()->GetNativeWindow()->SetProperty(
- ash::kBlockedForAssistantSnapshotKey, true);
- }
+ if (browser->profile()->IsOffTheRecord())
+ window->SetProperty(ash::kBlockedForAssistantSnapshotKey, true);
// TODO(estade): how much of the rest of this needs porting to Mash?
if (IsMash()) {
- frame()->GetNativeWindow()->SetProperty(ash::kFrameTextColorKey,
- GetTitleColor());
+ window->SetProperty(ash::kFrameTextColorKey, GetTitleColor());
OnThemeChanged();
return;
}
@@ -240,8 +238,6 @@
if (TabletModeClient::Get())
TabletModeClient::Get()->AddObserver(this);
- frame()->GetNativeWindow()->AddObserver(this);
-
browser_view()->immersive_mode_controller()->AddObserver(this);
UpdateFrameColors();
@@ -837,20 +833,27 @@
// TODO(estade): remove this interface. Ash handles it for us with HeaderView.
void BrowserNonClientFrameViewAsh::OnWindowDestroying(aura::Window* window) {
- DCHECK(!IsMash());
- DCHECK_EQ(frame()->GetNativeWindow(), window);
- window->RemoveObserver(this);
+ window_observer_.RemoveAll();
}
void BrowserNonClientFrameViewAsh::OnWindowPropertyChanged(aura::Window* window,
const void* key,
intptr_t old) {
- DCHECK(!IsMash());
- DCHECK_EQ(frame()->GetNativeWindow(), window);
if (key != aura::client::kShowStateKey)
return;
- frame_header_->OnShowStateChanged(
- window->GetProperty(aura::client::kShowStateKey));
+
+ if (IsMash()) {
+ const ui::WindowShowState new_state =
+ window->GetProperty(aura::client::kShowStateKey);
+ if (new_state == ui::SHOW_STATE_NORMAL ||
+ new_state == ui::SHOW_STATE_MAXIMIZED) {
+ InvalidateLayout();
+ frame()->GetRootView()->Layout();
+ }
+ } else {
+ frame_header_->OnShowStateChanged(
+ window->GetProperty(aura::client::kShowStateKey));
+ }
}
///////////////////////////////////////////////////////////////////////////////
diff --git a/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h b/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
index 00b30a328..fb8247e 100644
--- a/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
+++ b/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
@@ -12,6 +12,7 @@
#include "ash/shell_observer.h"
#include "base/gtest_prod_util.h"
#include "base/macros.h"
+#include "base/scoped_observer.h"
#include "chrome/browser/command_observer.h"
#include "chrome/browser/ui/ash/browser_image_registrar.h"
#include "chrome/browser/ui/ash/tablet_mode_client_observer.h"
@@ -228,6 +229,8 @@
// The binding this instance uses to implement mojom::SplitViewObserver.
mojo::Binding<ash::mojom::SplitViewObserver> observer_binding_{this};
+ ScopedObserver<aura::Window, aura::WindowObserver> window_observer_{this};
+
// Indicates whether overview mode is active. Hide the header for V1 apps in
// overview mode because a fake header is added for better UX. If also in
// immersive mode before entering overview mode, the flag will be ignored
