Skip to content

Commit

Permalink
Revert "Magnifier button for projector."
Browse files Browse the repository at this point in the history 
This reverts commit a55f503.

Reason for revert: Failing tests on Asan/Msan builders:
https://ci.chromium.org/p/chromium/builders/ci/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29
https://ci.chromium.org/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests

Sample run:
https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20(1)/39987/overview

Original change's description:
> Magnifier button for projector.
>
> This cl adds the magnification tool for projector.
>
> Bug: 1204713
> Change-Id: I14ae21df62df944171f4e75dbadacc62be259e21
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2864483
> Commit-Queue: Yilkal Abe <yilkal@chromium.org>
> Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
> Reviewed-by: Toby Huang <tobyhuang@chromium.org>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#880154}

Bug: 1204713
Change-Id: I094de94120c8b6e884b27a96278f0c61d5c6abcb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2878835
Auto-Submit: Sergey Poromov <poromov@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Oleg Davydov <burunduk@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Reviewed-by: Oleg Davydov <burunduk@chromium.org>
Owners-Override: Sergey Poromov <poromov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#880317}
  • Loading branch information
Sergey Poromov authored and Chromium LUCI CQ committed May 7, 2021
1 parent c091b58 commit 96437f5
Show file tree
Hide file tree
Showing 12 changed files with 12 additions and 172 deletions.
25 changes: 0 additions & 25 deletions ash/accessibility/magnifier/partial_magnification_controller.cc
View file
Expand Up @@ -46,24 +46,9 @@ PartialMagnificationController::~PartialMagnificationController() {
Shell::Get()->RemovePreTargetHandler(this);
}

void PartialMagnificationController::AddObserver(
PartialMagnificationController::Observer* observer) {
observers_.AddObserver(observer);
}

void PartialMagnificationController::RemoveObserver(
PartialMagnificationController::Observer* observer) {
observers_.RemoveObserver(observer);
}

void PartialMagnificationController::SetEnabled(bool enabled) {
if (is_enabled_ == enabled)
return;

is_enabled_ = enabled;
SetActive(false);
for (auto& observer : observers_)
observer.OnPartialMagnificationStateChanged(enabled);
}

void PartialMagnificationController::SwitchTargetRootWindowIfNeeded(
Expand All @@ -82,10 +67,6 @@ void PartialMagnificationController::OnTouchEvent(ui::TouchEvent* event) {
OnLocatedEvent(event, event->pointer_details());
}

void PartialMagnificationController::OnMouseEvent(ui::MouseEvent* event) {
OnLocatedEvent(event, event->pointer_details());
}

void PartialMagnificationController::SetActive(bool active) {
// Fail if we're trying to activate while disabled.
DCHECK(is_enabled_ || !active);
Expand All @@ -107,12 +88,6 @@ void PartialMagnificationController::OnLocatedEvent(
if (!is_enabled_)
return;

if (allow_mouse_following_ &&
pointer_details.pointer_type == ui::EventPointerType::kMouse) {
SetActive(true);
return;
}

if (pointer_details.pointer_type != ui::EventPointerType::kPen)
return;

Expand Down
24 changes: 0 additions & 24 deletions ash/accessibility/magnifier/partial_magnification_controller.h
View file
Expand Up @@ -9,8 +9,6 @@

#include "ash/ash_export.h"
#include "base/macros.h"
#include "base/observer_list.h"
#include "base/observer_list_types.h"
#include "ui/events/event_handler.h"
#include "ui/gfx/geometry/point.h"
#include "ui/gfx/geometry/size.h"
Expand All @@ -32,18 +30,9 @@ class MagnifierGlass;
// which is zoomed in. The zoomed area follows the mouse cursor when enabled.
class ASH_EXPORT PartialMagnificationController : public ui::EventHandler {
public:
class Observer : public base::CheckedObserver {
public:
// Called when the partial magnification enabled state changes.
virtual void OnPartialMagnificationStateChanged(bool enabled) = 0;
};

PartialMagnificationController();
~PartialMagnificationController() override;

void AddObserver(Observer* observer);
void RemoveObserver(Observer* observer);

// Turns the partial screen magnifier feature on or off. Turning the magnifier
// on does not imply that it will be displayed; the magnifier is only
// displayed when it is both enabled and active.
Expand All @@ -56,18 +45,11 @@ class ASH_EXPORT PartialMagnificationController : public ui::EventHandler {
// - Switch the target window from current window to |new_root_window|.
void SwitchTargetRootWindowIfNeeded(aura::Window* new_root_window);

void set_allow_mouse_following(bool enabled) {
allow_mouse_following_ = enabled;
}

bool is_enabled() const { return is_enabled_; }

private:
friend class PartialMagnificationControllerTestApi;

// ui::EventHandler:
void OnTouchEvent(ui::TouchEvent* event) override;
void OnMouseEvent(ui::MouseEvent* event) override;

// Enables or disables the actual magnifier window.
void SetActive(bool active);
Expand All @@ -79,14 +61,8 @@ class ASH_EXPORT PartialMagnificationController : public ui::EventHandler {
bool is_enabled_ = false;
bool is_active_ = false;

// If enabled, allows the magnifier glass to follow the mouse without the need
// to pressing and holding the mouse.
bool allow_mouse_following_ = false;

std::unique_ptr<MagnifierGlass> magnifier_glass_;

base::ObserverList<Observer> observers_;

DISALLOW_COPY_AND_ASSIGN(PartialMagnificationController);
};

Expand Down
9 changes: 0 additions & 9 deletions ash/accessibility/magnifier/partial_magnification_controller_unittest.cc
View file
Expand Up @@ -134,15 +134,6 @@ TEST_F(PartialMagnificationControllerTest, ActivatesOnlyForPointer) {
EXPECT_FALSE(GetTestApi().is_active());
}

// The magnifier activates for mouse events.
TEST_F(PartialMagnificationControllerTest, ActivatesForMouseEvents) {
GetController()->SetEnabled(true);
GetController()->set_allow_mouse_following(true);
ui::test::EventGenerator* event_generator = GetEventGenerator();
event_generator->MoveMouseBy(1, 1);
EXPECT_TRUE(GetTestApi().is_active());
}

// The magnifier is always located at pointer.
TEST_F(PartialMagnificationControllerTest, MagnifierFollowsPointer) {
ui::test::EventGenerator* event_generator = GetEventGenerator();
Expand Down
4 changes: 0 additions & 4 deletions ash/projector/projector_controller_impl.cc
View file
Expand Up @@ -134,10 +134,6 @@ void ProjectorControllerImpl::OnSelfieCamPressed(bool enabled) {
ui_controller_->OnSelfieCamPressed(enabled);
}

void ProjectorControllerImpl::OnMagnifierButtonPressed(bool enabled) {
ui_controller_->OnMagnifierButtonPressed(enabled);
}

void ProjectorControllerImpl::SetProjectorUiControllerForTest(
std::unique_ptr<ProjectorUiController> ui_controller) {
ui_controller_ = std::move(ui_controller);
Expand Down
2 changes: 0 additions & 2 deletions ash/projector/projector_controller_impl.h
View file
Expand Up @@ -73,8 +73,6 @@ class ASH_EXPORT ProjectorControllerImpl : public ProjectorController {
void OnClearAllMarkersPressed();
// Invoked when selfie cam button is pressed.
void OnSelfieCamPressed(bool enabled);
// Invoked when magnifier button is pressed.
void OnMagnifierButtonPressed(bool enabled);

void SetProjectorUiControllerForTest(
std::unique_ptr<ProjectorUiController> ui_controller);
Expand Down
5 changes: 0 additions & 5 deletions ash/projector/projector_controller_unittest.cc
View file
Expand Up @@ -211,9 +211,4 @@ TEST_F(ProjectorControllerTest, SetCaptionBubbleState) {
controller_->SetCaptionBubbleState(true);
}

TEST_F(ProjectorControllerTest, MagnifierButtonPressed) {
EXPECT_CALL(*mock_ui_controller_, OnMagnifierButtonPressed(true));
controller_->OnMagnifierButtonPressed(true);
}

} // namespace ash
46 changes: 6 additions & 40 deletions ash/projector/projector_ui_controller.cc
View file
Expand Up @@ -4,7 +4,6 @@

#include "ash/projector/projector_ui_controller.h"

#include "ash/accessibility/magnifier/partial_magnification_controller.h"
#include "ash/projector/projector_controller_impl.h"
#include "ash/projector/ui/projector_bar_view.h"
#include "ash/public/cpp/toast_data.h"
Expand Down Expand Up @@ -57,13 +56,6 @@ void EnableMarker(bool enabled) {
MarkerController::Get()->SetEnabled(enabled);
}

void EnableMagnifier(bool enabled) {
auto* magnifier_controller = Shell::Get()->partial_magnification_controller();
DCHECK(magnifier_controller);
magnifier_controller->SetEnabled(enabled);
magnifier_controller->set_allow_mouse_following(enabled);
}

} // namespace

// This class controls the interaction with the caption bubble. It keeps track
Expand Down Expand Up @@ -145,11 +137,6 @@ ProjectorUiController::ProjectorUiController(
DCHECK(marker_controller);
marker_controller_observation_.Observe(marker_controller);

auto* partial_magnification_controller =
Shell::Get()->partial_magnification_controller();
DCHECK(partial_magnification_controller);
partial_magnification_observation_.Observe(partial_magnification_controller);

caption_bubble_ =
std::make_unique<ProjectorUiController::CaptionBubbleController>(this);

Expand Down Expand Up @@ -242,10 +229,6 @@ void ProjectorUiController::OnRecordingStateChanged(bool started) {
caption_bubble_->Close();
}

void ProjectorUiController::OnMagnifierButtonPressed(bool enabled) {
EnableMagnifier(enabled);
}

bool ProjectorUiController::IsToolbarVisible() const {
return model_.bar_enabled();
}
Expand All @@ -260,27 +243,21 @@ void ProjectorUiController::ResetTools() {
EnableLaserPointer(false);
// Reset marker.
EnableMarker(false);
// Reset magnifier.
EnableMagnifier(false);
}

void ProjectorUiController::OnLaserPointerStateChanged(bool enabled) {
// If laser pointer is enabled, disable marker and magnifier.
if (enabled) {
EnableMarker(false);
EnableMagnifier(false);
}
// Disable marker if laser pointer is enabled;
if (enabled)
MarkerController::Get()->SetEnabled(false);

if (projector_bar_view_)
projector_bar_view_->OnLaserPointerStateChanged(enabled);
}

void ProjectorUiController::OnMarkerStateChanged(bool enabled) {
// If marker is enabled, disable laser pointer and magnifier.
if (enabled) {
EnableLaserPointer(false);
EnableMagnifier(false);
}
// Disable laser pointer since marker if enabled;
if (enabled)
Shell::Get()->laser_pointer_controller()->SetEnabled(false);

if (projector_bar_view_)
projector_bar_view_->OnMarkerStateChanged(enabled);
Expand All @@ -291,15 +268,4 @@ void ProjectorUiController::OnProjectorSessionActiveStateChanged(bool active) {
MarkerController::Get()->Clear();
}

void ProjectorUiController::OnPartialMagnificationStateChanged(bool enabled) {
// If magnifier is enabled, disable laser pointer and marker.
if (enabled) {
EnableMarker(false);
EnableLaserPointer(false);
}

if (projector_bar_view_)
projector_bar_view_->OnMagnifierStateChanged(enabled);
}

} // namespace ash
21 changes: 4 additions & 17 deletions ash/projector/projector_ui_controller.h
View file
Expand Up @@ -9,7 +9,6 @@
#include <string>
#include <vector>

#include "ash/accessibility/magnifier/partial_magnification_controller.h"
#include "ash/ash_export.h"
#include "ash/fast_ink/laser/laser_pointer_controller.h"
#include "ash/marker/marker_controller.h"
Expand All @@ -24,11 +23,9 @@ class ProjectorControllerImpl;
class ProjectorBarView;

// The controller in charge of UI.
class ASH_EXPORT ProjectorUiController
: public LaserPointerObserver,
public MarkerObserver,
public ProjectorSessionObserver,
public PartialMagnificationController::Observer {
class ASH_EXPORT ProjectorUiController : public LaserPointerObserver,
public MarkerObserver,
public ProjectorSessionObserver {
public:
explicit ProjectorUiController(ProjectorControllerImpl* projector_controller);
ProjectorUiController(const ProjectorUiController&) = delete;
Expand All @@ -53,14 +50,11 @@ class ASH_EXPORT ProjectorUiController
virtual void OnTranscription(const std::string& transcription, bool is_final);
// Invoked when the selfie cam button is pressed. Virtual for testing.
virtual void OnSelfieCamPressed(bool enabled);
// Invoked when the recording started or stopped. Virtual for testing.
// Called when the recording started or stopped. Virtual for testing.
virtual void OnRecordingStateChanged(bool started);
// Notifies the ProjectorControllerImpl and ProjectorBarView when the caption
// bubble model's state changes.
void OnCaptionBubbleModelStateChanged(bool visible);
// Invoked when magnification is set to be enabled or not. Virtual for
// testing.
virtual void OnMagnifierButtonPressed(bool enabled);

bool IsToolbarVisible() const;

Expand All @@ -86,9 +80,6 @@ class ASH_EXPORT ProjectorUiController
// ProjectorSessionObserver:
void OnProjectorSessionActiveStateChanged(bool active) override;

// PartialMagnificationController::OnPartialMagnificationStateChanged:
void OnPartialMagnificationStateChanged(bool enabled) override;

ProjectorUiModel model_;
views::UniqueWidgetPtr projector_bar_widget_;
ProjectorBarView* projector_bar_view_ = nullptr;
Expand All @@ -105,10 +96,6 @@ class ASH_EXPORT ProjectorUiController

base::ScopedObservation<ProjectorSession, ProjectorSessionObserver>
projector_session_observation_{this};

base::ScopedObservation<PartialMagnificationController,
PartialMagnificationController::Observer>
partial_magnification_observation_{this};
};

} // namespace ash
Expand Down
36 changes: 0 additions & 36 deletions ash/projector/projector_ui_controller_unittest.cc
View file
Expand Up @@ -74,16 +74,6 @@ TEST_F(ProjectorUiControllerTest, EnablingDisablingLaserPointer) {
controller_->OnLaserPointerPressed();
EXPECT_FALSE(marker_controller_->is_enabled());
EXPECT_TRUE(laser_pointer_controller_->is_enabled());

// Verify that toggling laser pointer disables magnifier when it was enabled.
auto* magnification_controller =
Shell::Get()->partial_magnification_controller();
controller_->OnMagnifierButtonPressed(true);
EXPECT_TRUE(magnification_controller->is_enabled());
EXPECT_FALSE(laser_pointer_controller_->is_enabled());
controller_->OnLaserPointerPressed();
EXPECT_TRUE(laser_pointer_controller_->is_enabled());
EXPECT_FALSE(magnification_controller->is_enabled());
}

// Verifies that toggling on the marker on Projector tools propagates to
Expand Down Expand Up @@ -126,15 +116,6 @@ TEST_F(ProjectorUiControllerTest, EnablingDisablingMarker) {
laser_pointer_controller_->SetEnabled(false);
EXPECT_FALSE(marker_controller_->is_enabled());
EXPECT_FALSE(laser_pointer_controller_->is_enabled());

// Verify that toggling marker disables magnifier when it was enabled.
auto* magnification_controller =
Shell::Get()->partial_magnification_controller();
controller_->OnMagnifierButtonPressed(true);
EXPECT_TRUE(magnification_controller->is_enabled());
controller_->OnMarkerPressed();
EXPECT_TRUE(marker_controller_->is_enabled());
EXPECT_FALSE(magnification_controller->is_enabled());
}

// Verifies that clicking the Clear All Markers button and disabling marker mode
Expand Down Expand Up @@ -203,21 +184,4 @@ TEST_F(ProjectorUiControllerTest, CaptionBubbleVisible) {
EXPECT_FALSE(controller_->IsCaptionBubbleModelOpen());
}

TEST_F(ProjectorUiControllerTest, EnablingDisablingMagnifierGlass) {
// Ensure that enabling magnifier disables marker if it was enabled.
controller_->ShowToolbar();
auto* marker_controller_ = MarkerController::Get();
marker_controller_->SetEnabled(true);
EXPECT_TRUE(marker_controller_->is_enabled());
controller_->OnMagnifierButtonPressed(true);
EXPECT_FALSE(marker_controller_->is_enabled());

// Ensures that enabling magnifier disables laser pointer if it was enabled.
auto* laser_pointer_controller_ = Shell::Get()->laser_pointer_controller();
laser_pointer_controller_->SetEnabled(true);
EXPECT_TRUE(laser_pointer_controller_->is_enabled());
controller_->OnMagnifierButtonPressed(true);
EXPECT_FALSE(laser_pointer_controller_->is_enabled());
}

} // namespace ash
1 change: 0 additions & 1 deletion ash/projector/test/mock_projector_ui_controller.h
View file
Expand Up @@ -35,7 +35,6 @@ class ASH_EXPORT MockProjectorUiController : public ProjectorUiController {
MOCK_METHOD1(OnSelfieCamPressed, void(bool enabled));
MOCK_METHOD1(OnRecordingStateChanged, void(bool started));
MOCK_METHOD1(SetCaptionBubbleState, void(bool));
MOCK_METHOD1(OnMagnifierButtonPressed, void(bool));
};

} // namespace ash
Expand Down

0 comments on commit 96437f5

Please sign in to comment.