Suppressed screen rotation notifications triggeres by the accelerometer

ash/wm/maximize_mode/maximize_mode_controller.cc

Index: ash/wm/maximize_mode/maximize_mode_controller.cc
diff --git a/ash/wm/maximize_mode/maximize_mode_controller.cc b/ash/wm/maximize_mode/maximize_mode_controller.cc
index f438d4944b44d52792d28594d0ce0c858e44ec54..ca4b2103a33e7327628360a3cb719bcc977096b5 100644
--- a/ash/wm/maximize_mode/maximize_mode_controller.cc
+++ b/ash/wm/maximize_mode/maximize_mode_controller.cc
@@ -7,8 +7,12 @@
 #include "ash/accelerometer/accelerometer_controller.h"
 #include "ash/display/display_manager.h"
 #include "ash/shell.h"
+#include "ash/system/chromeos/tray_display.h"
+#include "ash/system/system_notifier.h"
 #include "ash/wm/maximize_mode/maximize_mode_event_blocker.h"
 #include "ui/gfx/vector3d_f.h"
+#include "ui/message_center/message_center.h"
+#include "ui/message_center/notification_blocker.h"
 
 namespace ash {
 
@@ -81,6 +85,42 @@ float ClockwiseAngleBetweenVectorsInDegrees(const gfx::Vector3dF& base,
 
 }  // namespace
 
+MaximizeModeController::MaximizeModeNotificationBlocker::

[flackr, 2014/05/08 18:04:00]

Perhaps this should be called DisplayNotificationBlocker

[bruthig, 2014/05/08 21:15:33]

Done.

+    MaximizeModeNotificationBlocker()
+      : message_center::NotificationBlocker(

[flackr, 2014/05/08 18:04:00]

nit: I think the : should align with the M in Maximize above since it's at the
same syntactic/lexical depth as that.

+          message_center::MessageCenter::Get()),

[flackr, 2014/05/08 18:04:00]

nit: this should be indented 4 from the m in "message_center" above.

+        show_notifications_(true) {
+}
+
+MaximizeModeController::MaximizeModeNotificationBlocker::
+    ~MaximizeModeNotificationBlocker() {
+}
+
+void MaximizeModeController::MaximizeModeNotificationBlocker::
+    SetShouldShowNotification(bool should_show) {
+  bool did_change = show_notifications_ != should_show;
+  show_notifications_ = should_show;
+  if (did_change)
+    NotifyBlockingStateChanged();
+}
+
+bool MaximizeModeController::MaximizeModeNotificationBlocker::
+    ShouldShowNotificationAsPopup(
+      const message_center::NotifierId& notifier_id) const {

[flackr, 2014/05/08 18:04:00]

nit: should be indented 4 from the Should above.

+  if (notifier_id.id == ash::system_notifier::kNotifierDisplay)
+    return show_notifications_;
+  return true;

[flackr, 2014/05/08 18:04:00]

Why does this one not defer to the base class where the other one does?

[bruthig, 2014/05/08 21:15:33]

It is a pure virtual function in the base class.

+}
+
+bool MaximizeModeController::MaximizeModeNotificationBlocker::
+    ShouldShowNotificati

[flackr, 2014/05/08 18:04:00]

truncated line?

[bruthig, 2014/05/08 21:15:33]

Done.

+      const message_center::NotifierId& notifier_id) const {
+  if (notifier_id.id == ash::system_notifier::kNotifierDisplay)
+    return show_notifications_;
+  return message_center::NotificationBlocker::ShouldShowNotification(
+      notifier_id);
+}
+
 MaximizeModeController::MaximizeModeController()
     : rotation_locked_(false) {
   Shell::GetInstance()->accelerometer_controller()->AddObserver(this);
@@ -209,11 +249,28 @@ 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,
+                       gfx::Display::InternalDisplayId(),
+                       new_rotation);
   }
 }
 
+// Sets the display's rotation.
+void MaximizeModeController::SetDisplayRotation(DisplayManager* display_manager,
+                        int64 display_id,
+                        gfx::Display::Rotation rotation) {
+  // Suppress message centre notifications for screen rotations caused
+  // by accelerometer events because it should be obvious why the orientation
+  // changed.
+  notification_blocker_.SetShouldShowNotification(false);
+  display_manager->SetDisplayRotation(display_id, rotation);
+  message_center::MessageCenter* message_center =
+      message_center::MessageCenter::Get();
+  if (message_center->HasNotification(TrayDisplay::kNotificationId))
+    message_center->MarkSinglePopupAsShown(TrayDisplay::kNotificationId, true);

[jonross, 2014/05/08 18:12:34]

Since TrayDisplay is responsible for the creation and display of all
notifications related to display configuration, I would rather see a method
added to it for clearing.

It already is working directly with the MessageCenter for injecting events.

This would allow us to not expose the Id.

+  notification_blocker_.SetShouldShowNotification(true);
+}
+
 void MaximizeModeController::EnterMaximizeMode() {
   // TODO(jonross): Create a property on the display to track user rotations.
   // We should lock based on this property. Furthermore we should apply the
@@ -232,8 +289,9 @@ void MaximizeModeController::LeaveMaximizeMode() {
   gfx::Display::Rotation current_rotation = display_manager->
       GetDisplayInfo(gfx::Display::InternalDisplayId()).rotation();
   if (!rotation_locked_ && current_rotation != gfx::Display::ROTATE_0) {
-    display_manager->SetDisplayRotation(gfx::Display::InternalDisplayId(),
-                                        gfx::Display::ROTATE_0);
+    SetDisplayRotation(display_manager,
+                       gfx::Display::InternalDisplayId(),
+                       gfx::Display::ROTATE_0);
   }
   Shell::GetInstance()->EnableMaximizeModeWindowManager(false);
   event_blocker_.reset();