Commit c2571fa2 authored by Maggie Cai's avatar Maggie Cai Committed by Commit Bot

Revert "Add TabletState::Observer and adopt in BrowserNonClientFrameViewAsh"

This reverts commit 2ad3f0c5.

Reason for revert: CL:https://crrev.com/c/2458768 is likely the cause that create ash_unittests and browser_tests failures in linux-chromeos-dbg build starting from https://ci.chromium.org/p/chromium/builders/ci/linux-chromeos-dbg/20815. The failed tests are FrameSizeButtonTest, ImmersiveModeBrowserViewTestNoWebUiTabStrip and ImmersiveModeBrowserViewTest. I'm not 100% sure this CL is depend on the culprit, but it is linked from that CL, so I reverted this just in case. 

Original change's description:
> Add TabletState::Observer and adopt in BrowserNonClientFrameViewAsh
>
> This replaces the use of ash::TabletMode and ash::TabletModeObserver in
> BrowserNonClientFrameViewAsh.
>
> This corresponds to phase 2.6 on the design document [1].
>
> Covered by existing tests, eg
>
> BrowserNonClientFrameViewAshTest.BrowserHeaderVisibilityInTabletModeTest
> HomeLauncherBrowserNonClientFrameViewAshTest.TabletModeBrowserCaptionButtonVisibility
>
> [1] https://docs.google.com/document/d/1xHwcHrAiEaWuA4usGEqKZ9zm1H8SGk3WkS-Jf2Q_los/
>
> BUG=1113900
> R=​sky@chromium.org
>
> Change-Id: I6a21ca44538e52a2712bbb936221aa24063daa36
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2462560
> Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
> Reviewed-by: James Cook <jamescook@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#816316}

TBR=jamescook@chromium.org,sky@chromium.org,tonikitoo@igalia.com

Change-Id: I8f8ad38e07c26eee09fec0efc6ca3cbf882a2719
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1113900
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2466014Reviewed-by: default avatarMaggie Cai <mxcai@chromium.org>
Commit-Queue: Maggie Cai <mxcai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816384}
parent de644d3c
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "ash/public/cpp/app_types.h" #include "ash/public/cpp/app_types.h"
#include "ash/public/cpp/default_frame_header.h" #include "ash/public/cpp/default_frame_header.h"
#include "ash/public/cpp/frame_utils.h" #include "ash/public/cpp/frame_utils.h"
#include "ash/public/cpp/tablet_mode.h"
#include "ash/public/cpp/window_properties.h" #include "ash/public/cpp/window_properties.h"
#include "ash/wm/window_util.h" #include "ash/wm/window_util.h"
#include "base/metrics/user_metrics.h" #include "base/metrics/user_metrics.h"
...@@ -94,7 +95,7 @@ BrowserNonClientFrameViewAsh::BrowserNonClientFrameViewAsh( ...@@ -94,7 +95,7 @@ BrowserNonClientFrameViewAsh::BrowserNonClientFrameViewAsh(
} }
BrowserNonClientFrameViewAsh::~BrowserNonClientFrameViewAsh() { BrowserNonClientFrameViewAsh::~BrowserNonClientFrameViewAsh() {
chromeos::TabletState::Get()->RemoveObserver(this); ash::TabletMode::Get()->RemoveObserver(this);
ImmersiveModeController* immersive_controller = ImmersiveModeController* immersive_controller =
browser_view()->immersive_mode_controller(); browser_view()->immersive_mode_controller();
...@@ -133,7 +134,7 @@ void BrowserNonClientFrameViewAsh::Init() { ...@@ -133,7 +134,7 @@ void BrowserNonClientFrameViewAsh::Init() {
if (browser->profile()->IsOffTheRecord()) if (browser->profile()->IsOffTheRecord())
window->SetProperty(ash::kBlockedForAssistantSnapshotKey, true); window->SetProperty(ash::kBlockedForAssistantSnapshotKey, true);
chromeos::TabletState::Get()->AddObserver(this); ash::TabletMode::Get()->AddObserver(this);
if (frame()->ShouldDrawFrameHeader()) if (frame()->ShouldDrawFrameHeader())
frame_header_ = CreateFrameHeader(); frame_header_ = CreateFrameHeader();
...@@ -414,19 +415,12 @@ gfx::ImageSkia BrowserNonClientFrameViewAsh::GetFrameHeaderOverlayImage( ...@@ -414,19 +415,12 @@ gfx::ImageSkia BrowserNonClientFrameViewAsh::GetFrameHeaderOverlayImage(
: BrowserFrameActiveState::kInactive); : BrowserFrameActiveState::kInactive);
} }
void BrowserNonClientFrameViewAsh::OnTabletStateChanged( void BrowserNonClientFrameViewAsh::OnTabletModeStarted() {
chromeos::TabletState::State state) { OnTabletModeToggled(true);
switch (state) { }
case chromeos::TabletState::State::kInTabletMode:
OnTabletModeToggled(true); void BrowserNonClientFrameViewAsh::OnTabletModeEnded() {
return; OnTabletModeToggled(false);
case chromeos::TabletState::State::kInClamshellMode:
OnTabletModeToggled(false);
return;
case chromeos::TabletState::State::kEnteringTabletMode:
case chromeos::TabletState::State::kExitingTabletMode:
break;
}
} }
void BrowserNonClientFrameViewAsh::OnTabletModeToggled(bool enabled) { void BrowserNonClientFrameViewAsh::OnTabletModeToggled(bool enabled) {
...@@ -565,7 +559,7 @@ bool BrowserNonClientFrameViewAsh::ShouldShowCaptionButtons() const { ...@@ -565,7 +559,7 @@ bool BrowserNonClientFrameViewAsh::ShouldShowCaptionButtons() const {
bool BrowserNonClientFrameViewAsh::ShouldShowCaptionButtonsWhenNotInOverview() bool BrowserNonClientFrameViewAsh::ShouldShowCaptionButtonsWhenNotInOverview()
const { const {
return UsePackagedAppHeaderStyle(browser_view()->browser()) || return UsePackagedAppHeaderStyle(browser_view()->browser()) ||
!chromeos::TabletState::Get()->InTabletMode(); !ash::TabletMode::Get()->InTabletMode();
} }
int BrowserNonClientFrameViewAsh::GetToolbarLeftInset() const { int BrowserNonClientFrameViewAsh::GetToolbarLeftInset() const {
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include <memory> #include <memory>
#include "ash/public/cpp/tablet_mode_observer.h"
#include "base/gtest_prod_util.h" #include "base/gtest_prod_util.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
...@@ -15,7 +16,6 @@ ...@@ -15,7 +16,6 @@
#include "chrome/browser/ui/views/frame/browser_non_client_frame_view.h" #include "chrome/browser/ui/views/frame/browser_non_client_frame_view.h"
#include "chrome/browser/ui/views/frame/immersive_mode_controller.h" #include "chrome/browser/ui/views/frame/immersive_mode_controller.h"
#include "chrome/browser/ui/views/tab_icon_view_model.h" #include "chrome/browser/ui/views/tab_icon_view_model.h"
#include "chromeos/ui/base/tablet_state.h"
#include "ui/aura/window.h" #include "ui/aura/window.h"
#include "ui/aura/window_observer.h" #include "ui/aura/window_observer.h"
...@@ -34,7 +34,7 @@ class FrameCaptionButtonContainerView; ...@@ -34,7 +34,7 @@ class FrameCaptionButtonContainerView;
class BrowserNonClientFrameViewAsh class BrowserNonClientFrameViewAsh
: public BrowserNonClientFrameView, : public BrowserNonClientFrameView,
public BrowserFrameHeaderAsh::AppearanceProvider, public BrowserFrameHeaderAsh::AppearanceProvider,
public chromeos::TabletState::Observer, public ash::TabletModeObserver,
public TabIconViewModel, public TabIconViewModel,
public aura::WindowObserver, public aura::WindowObserver,
public ImmersiveModeController::Observer { public ImmersiveModeController::Observer {
...@@ -81,8 +81,9 @@ class BrowserNonClientFrameViewAsh ...@@ -81,8 +81,9 @@ class BrowserNonClientFrameViewAsh
int GetFrameHeaderImageYInset() override; int GetFrameHeaderImageYInset() override;
gfx::ImageSkia GetFrameHeaderOverlayImage(bool active) override; gfx::ImageSkia GetFrameHeaderOverlayImage(bool active) override;
// chromeos::TabletState::Observer: // ash::TabletModeObserver:
void OnTabletStateChanged(chromeos::TabletState::State) override; void OnTabletModeStarted() override;
void OnTabletModeEnded() override;
void OnTabletModeToggled(bool enabled); void OnTabletModeToggled(bool enabled);
......
...@@ -26,24 +26,9 @@ TabletState::~TabletState() { ...@@ -26,24 +26,9 @@ TabletState::~TabletState() {
g_instance = nullptr; g_instance = nullptr;
} }
void TabletState::AddObserver(Observer* observer) {
observers_.AddObserver(observer);
}
void TabletState::RemoveObserver(Observer* observer) {
observers_.RemoveObserver(observer);
}
bool TabletState::InTabletMode() const { bool TabletState::InTabletMode() const {
return state_ == TabletState::kInTabletMode || return state_ == TabletState::kInTabletMode ||
state_ == TabletState::kEnteringTabletMode; state_ == TabletState::kEnteringTabletMode;
} }
void TabletState::SetState(State state) {
state_ = state;
for (auto& observer : observers_)
observer.OnTabletStateChanged(state_);
}
} // namespace chromeos } // namespace chromeos
...@@ -6,7 +6,6 @@ ...@@ -6,7 +6,6 @@
#define CHROMEOS_UI_BASE_TABLET_STATE_H_ #define CHROMEOS_UI_BASE_TABLET_STATE_H_
#include "base/component_export.h" #include "base/component_export.h"
#include "base/observer_list.h"
namespace ash { namespace ash {
class TabletModeController; class TabletModeController;
...@@ -32,21 +31,12 @@ class COMPONENT_EXPORT(CHROMEOS_UI_BASE) TabletState { ...@@ -32,21 +31,12 @@ class COMPONENT_EXPORT(CHROMEOS_UI_BASE) TabletState {
kExitingTabletMode, kExitingTabletMode,
}; };
class COMPONENT_EXPORT(CHROMEOS_UI_BASE) Observer {
public:
virtual void OnTabletStateChanged(State state) = 0;
protected:
virtual ~Observer() = default;
};
TabletState(); TabletState();
TabletState(const TabletState&) = delete; TabletState(const TabletState&) = delete;
TabletState& operator=(const TabletState&) = delete; TabletState& operator=(const TabletState&) = delete;
~TabletState(); ~TabletState();
void AddObserver(Observer* observer); // TODO(http://crbug.com/1113900): Introduce Add|RemoveObserver support.
void RemoveObserver(Observer* observer);
// Returns true if the system is in tablet mode. // Returns true if the system is in tablet mode.
bool InTabletMode() const; bool InTabletMode() const;
...@@ -58,9 +48,7 @@ class COMPONENT_EXPORT(CHROMEOS_UI_BASE) TabletState { ...@@ -58,9 +48,7 @@ class COMPONENT_EXPORT(CHROMEOS_UI_BASE) TabletState {
// the tablet state. // the tablet state.
friend class ash::TabletModeController; friend class ash::TabletModeController;
void SetState(State state); void SetState(State state) { state_ = state; }
base::ObserverList<Observer>::Unchecked observers_;
State state_ = State::kInClamshellMode; State state_ = State::kInClamshellMode;
}; };
......
...@@ -2,8 +2,8 @@ ...@@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#ifndef CHROMEOS_UI_FRAME_CAPTION_BUTTONS_FRAME_BACK_BUTTON_H_ #ifndef ASH_PUBLIC_CPP_CAPTION_BUTTONS_FRAME_BACK_BUTTON_H_
#define CHROMEOS_UI_FRAME_CAPTION_BUTTONS_FRAME_BACK_BUTTON_H_ #define ASH_PUBLIC_CPP_CAPTION_BUTTONS_FRAME_BACK_BUTTON_H_
#include "base/component_export.h" #include "base/component_export.h"
#include "ui/views/window/frame_caption_button.h" #include "ui/views/window/frame_caption_button.h"
...@@ -28,4 +28,4 @@ class COMPONENT_EXPORT(CHROMEOS_UI_FRAME) FrameBackButton ...@@ -28,4 +28,4 @@ class COMPONENT_EXPORT(CHROMEOS_UI_FRAME) FrameBackButton
} // namespace chromeos } // namespace chromeos
#endif // CHROMEOS_UI_FRAME_CAPTION_BUTTONS_FRAME_BACK_BUTTON_H_ #endif // ASH_PUBLIC_CPP_CAPTION_BUTTONS_FRAME_BACK_BUTTON_H_
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment