Skip to content

Commit

Permalink
Convert /chrome/browser/ash/crosapi to ScopedDisplayObserver
Browse files Browse the repository at this point in the history
This is a refactoring to make it easier for future changes to
DisplayObserver, and to clean up manual Add/Remove calls with RAII.

ScopedDisplayObserver is an RAII class that directly replaces
display::Screen::Add/RemoveObserver.

ScopedOptionalDisplayObserver is the same thing, but handles the case
where display::Screen::GetScreen() is null.  This can happen in tests.

This is a non-functional change, intending to replicate existing
behavior exactly.

This CL was uploaded by git cl split.

R=erikchen@chromium.org

Bug: 1225463
Change-Id: I469ca2770f91d1eb4e2927120ea8c3ab865b9e5b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3004096
Auto-Submit: enne <enne@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#898412}
  • Loading branch information
quisquous authored and Chromium LUCI CQ committed Jul 3, 2021
1 parent 23be389 commit 12259ab
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 15 deletions.
16 changes: 2 additions & 14 deletions chrome/browser/ash/crosapi/system_display_ash.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include "ash/public/ash_interfaces.h"
#include "base/bind.h"
#include "chrome/browser/extensions/system_display/system_display_serialization.h"
#include "ui/display/screen.h"
#include "ui/gfx/geometry/insets.h"
#include "ui/gfx/geometry/mojom/geometry.mojom.h"
#include "ui/gfx/geometry/rect.h"
Expand Down Expand Up @@ -91,13 +90,7 @@ void SystemDisplayAsh::OnDisplayConfigChanged() {

void SystemDisplayAsh::StartDisplayChangedEventSources() {
// Start Source 1.
if (!is_observing_screen_) {
display::Screen* screen = display::Screen::GetScreen();
if (screen) {
screen->AddObserver(this);
is_observing_screen_ = true;
}
}
display_observer_.emplace(this);

// Start Source 2.
if (!is_observing_cros_display_config_) {
Expand All @@ -124,12 +117,7 @@ void SystemDisplayAsh::StopDisplayChangedEventSources() {
}

// Stop Source 1.
if (is_observing_screen_) {
is_observing_screen_ = false;
display::Screen* screen = display::Screen::GetScreen();
DCHECK(screen);
screen->RemoveObserver(this);
}
display_observer_.reset();
}

} // namespace crosapi
2 changes: 1 addition & 1 deletion chrome/browser/ash/crosapi/system_display_ash.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class SystemDisplayAsh : public mojom::SystemDisplay,
mojo::RemoteSet<mojom::DisplayChangeObserver> observers_;

// Source 1 status and objects.
bool is_observing_screen_ = false;
absl::optional<display::ScopedOptionalDisplayObserver> display_observer_;

// Source 2 status and objects.
bool is_observing_cros_display_config_ = false;
Expand Down

0 comments on commit 12259ab

Please sign in to comment.