Re-landing "Suppressed screen rotation notifications triggeres by the accelerometer."

Original review: https://codereview.chromium.org/267743010/

BUG=364949
Test=MaximizeModeControllerTest.BlockRotationNotifications
TBR=flackr
TBR=sadrul

Review URL: https://codereview.chromium.org/291233002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@271808 0039d316-1c4b-4281-b951-d872f2087c98

ash/system/chromeos/tray_display.cc

@@ -14,6 +14,7 @@
 #include "ash/system/tray/system_tray_delegate.h"
 #include "ash/system/tray/tray_constants.h"
 #include "ash/system/tray/tray_notification_view.h"
+#include "ash/wm/maximize_mode/maximize_mode_controller.h"
 #include "base/bind.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
@@ -374,6 +375,13 @@ void TrayDisplay::CreateOrUpdateNotification(
   if (message.empty())
     return;
 
+  // Don't display notifications for accelerometer triggered screen rotations.
+  // See http://crbug.com/364949
+  if (Shell::GetInstance()->maximize_mode_controller()->
+      in_set_screen_rotation()) {
+    return;
+  }
+
   ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
   scoped_ptr<Notification> notification(new Notification(
       message_center::NOTIFICATION_TYPE_SIMPLE,
@@ -388,7 +396,8 @@ void TrayDisplay::CreateOrUpdateNotification(
       message_center::RichNotificationData(),
       new message_center::HandleNotificationClickedDelegate(
           base::Bind(&OpenSettings))));
-  message_center::MessageCenter::Get()->AddNotification(notification.Pass());
+
+    message_center::MessageCenter::Get()->AddNotification(notification.Pass());
 }
 
 views::View* TrayDisplay::CreateDefaultView(user::LoginStatus status) {

ash/wm/maximize_mode/maximize_mode_controller.cc

@@ -11,6 +11,7 @@
 #include "ash/display/display_manager.h"
 #include "ash/shell.h"
 #include "ash/wm/maximize_mode/maximize_mode_event_blocker.h"
+#include "base/auto_reset.h"
 #include "base/command_line.h"
 #include "ui/base/accelerators/accelerator.h"
 #include "ui/events/event.h"
@@ -132,7 +133,8 @@ void ScreenshotActionHandler::OnKeyEvent(ui::KeyEvent* event) {
 
 MaximizeModeController::MaximizeModeController()
     : rotation_locked_(false),
-      have_seen_accelerometer_data_(false) {
+      have_seen_accelerometer_data_(false),
+      in_set_screen_rotation_(false) {
   Shell::GetInstance()->accelerometer_controller()->AddObserver(this);
 }
 
@@ -235,8 +237,8 @@ void MaximizeModeController::HandleScreenRotation(const gfx::Vector3dF& lid) {
       // Also, SetDisplayRotation will save the setting to the local store,
       // this should be stored in a way that we can distinguish what the
       // rotation was set by.
-      display_manager->SetDisplayRotation(gfx::Display::InternalDisplayId(),
-                                          gfx::Display::ROTATE_0);
+      SetDisplayRotation(display_manager,
+                         gfx::Display::ROTATE_0);
     }
     rotation_locked_ = false;
     return;
@@ -294,9 +296,18 @@ void MaximizeModeController::HandleScreenRotation(const gfx::Vector3dF& lid) {
   // match screen orientation.
   if (new_rotation == gfx::Display::ROTATE_0 ||
       maximize_mode_engaged) {
-    display_manager->SetDisplayRotation(gfx::Display::InternalDisplayId(),
-                                        new_rotation);
+    SetDisplayRotation(display_manager,
+                       new_rotation);
   }
 }
 
+void MaximizeModeController::SetDisplayRotation(
+    DisplayManager* display_manager,
+    gfx::Display::Rotation rotation) {
+  base::AutoReset<bool> auto_in_set_screen_rotation(
+      &in_set_screen_rotation_, true);
+  display_manager->SetDisplayRotation(gfx::Display::InternalDisplayId(),
+                                      rotation);
+}
+
 }  // namespace ash

ash/wm/maximize_mode/maximize_mode_controller.h

@@ -7,6 +7,7 @@
 
 #include "ash/accelerometer/accelerometer_observer.h"
 #include "ash/ash_export.h"
+#include "ash/display/display_manager.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 
@@ -26,6 +27,10 @@ class ASH_EXPORT MaximizeModeController : public AccelerometerObserver {
   MaximizeModeController();
   virtual ~MaximizeModeController();
 
+  bool in_set_screen_rotation() const {
+    return in_set_screen_rotation_;
+  }
+
   // True if |rotation_lock_| has been set, and OnAccelerometerUpdated will not
   // change the display rotation.
   bool rotation_locked() {
@@ -57,6 +62,10 @@ class ASH_EXPORT MaximizeModeController : public AccelerometerObserver {
   // screen.
   void HandleScreenRotation(const gfx::Vector3dF& lid);
 
+  // Sets the display rotation and suppresses display notifications.
+  void SetDisplayRotation(DisplayManager* display_manager,
+                          gfx::Display::Rotation rotation);
+
   // An event targeter controller which traps mouse and keyboard events while
   // maximize mode is engaged.
   scoped_ptr<MaximizeModeEventBlocker> event_blocker_;
@@ -70,6 +79,9 @@ class ASH_EXPORT MaximizeModeController : public AccelerometerObserver {
   // Whether we have ever seen accelerometer data.
   bool have_seen_accelerometer_data_;
 
+  // True when the screen's orientation is being changed.
+  bool in_set_screen_rotation_;
+
   DISALLOW_COPY_AND_ASSIGN(MaximizeModeController);
 };
 

ash/wm/maximize_mode/maximize_mode_controller_unittest.cc

@@ -12,10 +12,12 @@
 #include "ash/test/display_manager_test_api.h"
 #include "ash/test/test_lock_state_controller_delegate.h"
 #include "ash/test/test_screenshot_delegate.h"
+#include "ash/test/test_system_tray_delegate.h"
 #include "ash/test/test_volume_control_delegate.h"
 #include "ui/aura/test/event_generator.h"
 #include "ui/events/event_handler.h"
 #include "ui/gfx/vector3d_f.h"
+#include "ui/message_center/message_center.h"
 
 namespace ash {
 
@@ -112,6 +114,11 @@ class MaximizeModeControllerTest : public test::AshTestBase {
         gfx::Display::InternalDisplayId()).rotation();
   }
 
+  void SetInternalDisplayRotation(gfx::Display::Rotation rotation) const {
+    Shell::GetInstance()->display_manager()->
+        SetDisplayRotation(gfx::Display::InternalDisplayId(), rotation);
+  }
+
  private:
   DISALLOW_COPY_AND_ASSIGN(MaximizeModeControllerTest);
 };
@@ -152,7 +159,7 @@ TEST_F(MaximizeModeControllerTest, EnterExitThresholds) {
 // Tests that when the hinge is nearly vertically aligned, the current state
 // persists as the computed angle is highly inaccurate in this orientation.
 TEST_F(MaximizeModeControllerTest, HingeAligned) {
-   // Laptop in normal orientation lid open 90 degrees.
+  // Laptop in normal orientation lid open 90 degrees.
   TriggerAccelerometerUpdate(gfx::Vector3dF(0.0f, 0.0f, 1.0f),
                              gfx::Vector3dF(-1.0f, 0.0f, 0.0f));
   EXPECT_FALSE(IsMaximizeModeStarted());
@@ -338,7 +345,7 @@ TEST_F(MaximizeModeControllerTest, BlocksKeyboardAndMouse) {
   EXPECT_EQ(0u, counter.event_count());
   counter.reset();
 
- // Touch should not be blocked.
+  // Touch should not be blocked.
   event_generator.PressTouch();
   event_generator.ReleaseTouch();
   EXPECT_GT(counter.event_count(), 0u);
@@ -503,4 +510,65 @@ TEST_F(MaximizeModeControllerTest, ExitingMaximizeModeClearRotationLock) {
   EXPECT_FALSE(maximize_mode_controller()->rotation_locked());
 }
 
+// The TrayDisplay class that is responsible for adding/updating MessageCenter
+// notifications is only added to the SystemTray on ChromeOS.
+#if defined(OS_CHROMEOS)
+// Tests that the screen rotation notifications are suppressed when
+// triggered by the accelerometer.
+TEST_F(MaximizeModeControllerTest, BlockRotationNotifications) {
+  test::TestSystemTrayDelegate* tray_delegate =
+      static_cast<test::TestSystemTrayDelegate*>(
+          Shell::GetInstance()->system_tray_delegate());
+  tray_delegate->set_should_show_display_notification(true);
+
+  message_center::MessageCenter* message_center =
+      message_center::MessageCenter::Get();
+
+  // Make sure notifications are still displayed when
+  // adjusting the screen rotation directly when not in maximize mode
+  ASSERT_FALSE(IsMaximizeModeStarted());
+  ASSERT_NE(gfx::Display::ROTATE_180, GetInternalDisplayRotation());
+  ASSERT_EQ(0u, message_center->NotificationCount());
+  ASSERT_FALSE(message_center->HasPopupNotifications());
+  SetInternalDisplayRotation(gfx::Display::ROTATE_180);
+  EXPECT_EQ(gfx::Display::ROTATE_180, GetInternalDisplayRotation());
+  EXPECT_EQ(1u, message_center->NotificationCount());
+  EXPECT_TRUE(message_center->HasPopupNotifications());
+
+  // Reset the screen rotation.
+  SetInternalDisplayRotation(gfx::Display::ROTATE_0);
+  // Clear all notifications
+  message_center->RemoveAllNotifications(false);
+  // Trigger maximize mode by opening to 270.
+  TriggerAccelerometerUpdate(gfx::Vector3dF(0.0f, 0.0f, -1.0f),
+                             gfx::Vector3dF(-1.0f, 0.0f, 0.0f));
+  EXPECT_TRUE(IsMaximizeModeStarted());
+  EXPECT_EQ(0u, message_center->NotificationCount());
+  EXPECT_FALSE(message_center->HasPopupNotifications());
+
+  // Make sure notifications are still displayed when
+  // adjusting the screen rotation directly when in maximize mode
+  ASSERT_NE(gfx::Display::ROTATE_270, GetInternalDisplayRotation());
+  SetInternalDisplayRotation(gfx::Display::ROTATE_270);
+  EXPECT_EQ(gfx::Display::ROTATE_270, GetInternalDisplayRotation());
+  EXPECT_EQ(1u, message_center->NotificationCount());
+  EXPECT_TRUE(message_center->HasPopupNotifications());
+
+  // Clear all notifications
+  message_center->RemoveAllNotifications(false);
+  EXPECT_EQ(0u, message_center->NotificationCount());
+  EXPECT_FALSE(message_center->HasPopupNotifications());
+
+  // Make sure notifications are blocked when adjusting the screen rotation
+  // via the accelerometer while in maximize mode
+  // Rotate the screen 90 degrees
+  ASSERT_NE(gfx::Display::ROTATE_90, GetInternalDisplayRotation());
+  TriggerAccelerometerUpdate(gfx::Vector3dF(0.0f, 1.0f, 0.0f),
+                             gfx::Vector3dF(0.0f, 1.0f, 0.0f));
+  ASSERT_EQ(gfx::Display::ROTATE_90, GetInternalDisplayRotation());
+  EXPECT_EQ(0u, message_center->NotificationCount());
+  EXPECT_FALSE(message_center->HasPopupNotifications());
+}
+#endif
+
 }  // namespace ash