Commit 5647239f authored by Antonio Gomes's avatar Antonio Gomes Committed by Commit Bot

Adopt display::DisplayObserver::OnDisplayTabletStateChanged()

.... over chromeos::TabletState::Observer.

This CL is a follow up of crrev.com/c/2491517. It gets rid of
chromeos::TabletState::Observer class in favor of
display::DisplayObserver::OnDisplayTabletStateChanged().

The CL also removes the use of the `friend class` declaration in
chromeos::TabletState: ash::TabletModeController now sets the
tablet state through display::DisplayManager ->
display::DisplayObserver::OnDisplayTabletStateChanged().

Last, the CL deliberately did not got rid of chromeos::TabletState
class. Reason: TabletState can continue to be a central read-only
class for query the tablet state from any where in the source base.

BUG=1113900
R=jamescook@chromium.org

Change-Id: I532e8511c1173139937c36c5e4c282311ea17576
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2518128
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Reviewed-by: default avatarJames Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823963}
parent 2d8061fc
...@@ -841,7 +841,8 @@ void TabletModeController::SetTabletModeEnabledInternal(bool should_enable) { ...@@ -841,7 +841,8 @@ void TabletModeController::SetTabletModeEnabledInternal(bool should_enable) {
DeleteScreenshot(); DeleteScreenshot();
if (should_enable) { if (should_enable) {
tablet_state_.SetState(display::TabletState::kEnteringTabletMode); Shell::Get()->display_manager()->NotifyDisplayTabletStateChanged(
display::TabletState::kEnteringTabletMode);
// Take a screenshot if there is a top window that will get animated. // Take a screenshot if there is a top window that will get animated.
// TODO(sammiequon): Handle the case where the top window is not on the // TODO(sammiequon): Handle the case where the top window is not on the
...@@ -871,7 +872,8 @@ void TabletModeController::SetTabletModeEnabledInternal(bool should_enable) { ...@@ -871,7 +872,8 @@ void TabletModeController::SetTabletModeEnabledInternal(bool should_enable) {
FinishInitTabletMode(); FinishInitTabletMode();
} }
} else { } else {
tablet_state_.SetState(display::TabletState::kExitingTabletMode); Shell::Get()->display_manager()->NotifyDisplayTabletStateChanged(
display::TabletState::kExitingTabletMode);
// We may have entered tablet mode, then tried to exit before the screenshot // We may have entered tablet mode, then tried to exit before the screenshot
// was taken. In this case |tablet_mode_window_manager_| will be null. // was taken. In this case |tablet_mode_window_manager_| will be null.
...@@ -887,7 +889,8 @@ void TabletModeController::SetTabletModeEnabledInternal(bool should_enable) { ...@@ -887,7 +889,8 @@ void TabletModeController::SetTabletModeEnabledInternal(bool should_enable) {
base::RecordAction(base::UserMetricsAction("Touchview_Disabled")); base::RecordAction(base::UserMetricsAction("Touchview_Disabled"));
RecordTabletModeUsageInterval(TABLET_MODE_INTERVAL_ACTIVE); RecordTabletModeUsageInterval(TABLET_MODE_INTERVAL_ACTIVE);
tablet_state_.SetState(display::TabletState::kInClamshellMode); Shell::Get()->display_manager()->NotifyDisplayTabletStateChanged(
display::TabletState::kInClamshellMode);
for (auto& observer : tablet_mode_observers_) for (auto& observer : tablet_mode_observers_)
observer.OnTabletModeEnded(); observer.OnTabletModeEnded();
VLOG(1) << "Exit tablet mode."; VLOG(1) << "Exit tablet mode.";
...@@ -1140,7 +1143,8 @@ void TabletModeController::FinishInitTabletMode() { ...@@ -1140,7 +1143,8 @@ void TabletModeController::FinishInitTabletMode() {
base::RecordAction(base::UserMetricsAction("Touchview_Enabled")); base::RecordAction(base::UserMetricsAction("Touchview_Enabled"));
RecordTabletModeUsageInterval(TABLET_MODE_INTERVAL_INACTIVE); RecordTabletModeUsageInterval(TABLET_MODE_INTERVAL_INACTIVE);
tablet_state_.SetState(display::TabletState::kInTabletMode); Shell::Get()->display_manager()->NotifyDisplayTabletStateChanged(
display::TabletState::kInTabletMode);
for (auto& observer : tablet_mode_observers_) for (auto& observer : tablet_mode_observers_)
observer.OnTabletModeStarted(); observer.OnTabletModeStarted();
......
...@@ -36,6 +36,7 @@ ...@@ -36,6 +36,7 @@
#include "chrome/browser/ui/web_applications/system_web_app_ui_utils.h" #include "chrome/browser/ui/web_applications/system_web_app_ui_utils.h"
#include "chromeos/constants/chromeos_features.h" #include "chromeos/constants/chromeos_features.h"
#include "chromeos/ui/base/chromeos_ui_constants.h" #include "chromeos/ui/base/chromeos_ui_constants.h"
#include "chromeos/ui/base/tablet_state.h"
#include "chromeos/ui/base/window_properties.h" #include "chromeos/ui/base/window_properties.h"
#include "chromeos/ui/base/window_state_type.h" #include "chromeos/ui/base/window_state_type.h"
#include "chromeos/ui/frame/caption_buttons/frame_caption_button_container_view.h" #include "chromeos/ui/frame/caption_buttons/frame_caption_button_container_view.h"
...@@ -47,6 +48,7 @@ ...@@ -47,6 +48,7 @@
#include "ui/aura/env.h" #include "ui/aura/env.h"
#include "ui/base/hit_test.h" #include "ui/base/hit_test.h"
#include "ui/base/layout.h" #include "ui/base/layout.h"
#include "ui/display/screen.h"
#include "ui/events/gestures/gesture_recognizer.h" #include "ui/events/gestures/gesture_recognizer.h"
#include "ui/gfx/canvas.h" #include "ui/gfx/canvas.h"
#include "ui/gfx/geometry/rect.h" #include "ui/gfx/geometry/rect.h"
...@@ -94,7 +96,7 @@ BrowserNonClientFrameViewAsh::BrowserNonClientFrameViewAsh( ...@@ -94,7 +96,7 @@ BrowserNonClientFrameViewAsh::BrowserNonClientFrameViewAsh(
} }
BrowserNonClientFrameViewAsh::~BrowserNonClientFrameViewAsh() { BrowserNonClientFrameViewAsh::~BrowserNonClientFrameViewAsh() {
chromeos::TabletState::Get()->RemoveObserver(this); display::Screen::GetScreen()->RemoveObserver(this);
ImmersiveModeController* immersive_controller = ImmersiveModeController* immersive_controller =
browser_view()->immersive_mode_controller(); browser_view()->immersive_mode_controller();
...@@ -133,7 +135,7 @@ void BrowserNonClientFrameViewAsh::Init() { ...@@ -133,7 +135,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); display::Screen::GetScreen()->AddObserver(this);
if (frame()->ShouldDrawFrameHeader()) if (frame()->ShouldDrawFrameHeader())
frame_header_ = CreateFrameHeader(); frame_header_ = CreateFrameHeader();
...@@ -414,7 +416,7 @@ gfx::ImageSkia BrowserNonClientFrameViewAsh::GetFrameHeaderOverlayImage( ...@@ -414,7 +416,7 @@ gfx::ImageSkia BrowserNonClientFrameViewAsh::GetFrameHeaderOverlayImage(
: BrowserFrameActiveState::kInactive); : BrowserFrameActiveState::kInactive);
} }
void BrowserNonClientFrameViewAsh::OnTabletStateChanged( void BrowserNonClientFrameViewAsh::OnDisplayTabletStateChanged(
display::TabletState state) { display::TabletState state) {
switch (state) { switch (state) {
case display::TabletState::kInTabletMode: case display::TabletState::kInTabletMode:
......
...@@ -15,9 +15,9 @@ ...@@ -15,9 +15,9 @@
#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"
#include "ui/display/display_observer.h"
#include "ui/display/tablet_state.h" #include "ui/display/tablet_state.h"
namespace { namespace {
...@@ -35,7 +35,7 @@ class FrameCaptionButtonContainerView; ...@@ -35,7 +35,7 @@ class FrameCaptionButtonContainerView;
class BrowserNonClientFrameViewAsh class BrowserNonClientFrameViewAsh
: public BrowserNonClientFrameView, : public BrowserNonClientFrameView,
public BrowserFrameHeaderAsh::AppearanceProvider, public BrowserFrameHeaderAsh::AppearanceProvider,
public chromeos::TabletState::Observer, public display::DisplayObserver,
public TabIconViewModel, public TabIconViewModel,
public aura::WindowObserver, public aura::WindowObserver,
public ImmersiveModeController::Observer { public ImmersiveModeController::Observer {
...@@ -82,8 +82,8 @@ class BrowserNonClientFrameViewAsh ...@@ -82,8 +82,8 @@ class BrowserNonClientFrameViewAsh
int GetFrameHeaderImageYInset() override; int GetFrameHeaderImageYInset() override;
gfx::ImageSkia GetFrameHeaderOverlayImage(bool active) override; gfx::ImageSkia GetFrameHeaderOverlayImage(bool active) override;
// chromeos::TabletState::Observer: // display::DisplayObserver:
void OnTabletStateChanged(display::TabletState state) override; void OnDisplayTabletStateChanged(display::TabletState state) override;
void OnTabletModeToggled(bool enabled); void OnTabletModeToggled(bool enabled);
......
...@@ -29,28 +29,13 @@ TabletState::~TabletState() { ...@@ -29,28 +29,13 @@ TabletState::~TabletState() {
display::Screen::GetScreen()->RemoveObserver(this); display::Screen::GetScreen()->RemoveObserver(this);
} }
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_ == display::TabletState::kInTabletMode || return state_ == display::TabletState::kInTabletMode ||
state_ == display::TabletState::kEnteringTabletMode; state_ == display::TabletState::kEnteringTabletMode;
} }
void TabletState::SetState(display::TabletState state) {
state_ = state;
for (auto& observer : observers_)
observer.OnTabletStateChanged(state_);
}
void TabletState::OnDisplayTabletStateChanged(display::TabletState state) { void TabletState::OnDisplayTabletStateChanged(display::TabletState state) {
SetState(state); state_ = state;
} }
} // namespace chromeos } // namespace chromeos
...@@ -6,42 +6,26 @@ ...@@ -6,42 +6,26 @@
#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"
#include "ui/display/display_observer.h" #include "ui/display/display_observer.h"
#include "ui/display/tablet_state.h" #include "ui/display/tablet_state.h"
namespace ash {
class TabletModeController;
}
namespace chromeos { namespace chromeos {
// Class is a singleton and holds the tablet mode state. // Class is a singleton and holds the tablet mode state.
// //
// The idea is that only the creator of this class in Ash or Lacros/Ozone code // TODO(crbug.com/1113900): Move the logic to display::Screen::GetTabletState()
// is able to set the state. // and implement it for Ash and Ozone child classes.
class COMPONENT_EXPORT(CHROMEOS_UI_BASE) TabletState class COMPONENT_EXPORT(CHROMEOS_UI_BASE) TabletState
: public display::DisplayObserver { : public display::DisplayObserver {
public: public:
// Returns the singleton instance. // Returns the singleton instance.
static TabletState* Get(); static TabletState* Get();
class COMPONENT_EXPORT(CHROMEOS_UI_BASE) Observer {
public:
virtual void OnTabletStateChanged(display::TabletState 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() override; ~TabletState() override;
void AddObserver(Observer* observer);
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;
...@@ -51,14 +35,6 @@ class COMPONENT_EXPORT(CHROMEOS_UI_BASE) TabletState ...@@ -51,14 +35,6 @@ class COMPONENT_EXPORT(CHROMEOS_UI_BASE) TabletState
void OnDisplayTabletStateChanged(display::TabletState state) override; void OnDisplayTabletStateChanged(display::TabletState state) override;
private: private:
// The friend class declaration here is used to control classes that can set
// the tablet state.
friend class ash::TabletModeController;
void SetState(display::TabletState state);
base::ObserverList<Observer>::Unchecked observers_;
display::TabletState state_ = display::TabletState::kInClamshellMode; display::TabletState state_ = display::TabletState::kInClamshellMode;
}; };
......
...@@ -36,6 +36,7 @@ ...@@ -36,6 +36,7 @@
#include "ui/display/manager/display_util.h" #include "ui/display/manager/display_util.h"
#include "ui/display/manager/managed_display_info.h" #include "ui/display/manager/managed_display_info.h"
#include "ui/display/screen.h" #include "ui/display/screen.h"
#include "ui/display/tablet_state.h"
#include "ui/display/types/display_snapshot.h" #include "ui/display/types/display_snapshot.h"
#include "ui/gfx/font_render_params.h" #include "ui/gfx/font_render_params.h"
#include "ui/gfx/geometry/rect.h" #include "ui/gfx/geometry/rect.h"
...@@ -2192,6 +2193,12 @@ void DisplayManager::NotifyDisplayRemoved(const Display& display) { ...@@ -2192,6 +2193,12 @@ void DisplayManager::NotifyDisplayRemoved(const Display& display) {
observer.OnDisplayRemoved(display); observer.OnDisplayRemoved(display);
} }
void DisplayManager::NotifyDisplayTabletStateChanged(
const TabletState& tablet_state) {
for (auto& observer : observers_)
observer.OnDisplayTabletStateChanged(tablet_state);
}
void DisplayManager::AddObserver(DisplayObserver* observer) { void DisplayManager::AddObserver(DisplayObserver* observer) {
observers_.AddObserver(observer); observers_.AddObserver(observer);
} }
......
...@@ -50,6 +50,7 @@ class DisplayLayoutStore; ...@@ -50,6 +50,7 @@ class DisplayLayoutStore;
class DisplayObserver; class DisplayObserver;
class NativeDisplayDelegate; class NativeDisplayDelegate;
class Screen; class Screen;
enum class TabletState;
namespace test { namespace test {
class DisplayManagerTestApi; class DisplayManagerTestApi;
...@@ -492,6 +493,7 @@ class DISPLAY_MANAGER_EXPORT DisplayManager ...@@ -492,6 +493,7 @@ class DISPLAY_MANAGER_EXPORT DisplayManager
void NotifyMetricsChanged(const Display& display, uint32_t metrics); void NotifyMetricsChanged(const Display& display, uint32_t metrics);
void NotifyDisplayAdded(const Display& display); void NotifyDisplayAdded(const Display& display);
void NotifyDisplayRemoved(const Display& display); void NotifyDisplayRemoved(const Display& display);
void NotifyDisplayTabletStateChanged(const TabletState& tablet_state);
// Delegated from the Screen implementation. // Delegated from the Screen implementation.
void AddObserver(DisplayObserver* observer); void AddObserver(DisplayObserver* observer);
......
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